On Wed, Mar 11, 2015 at 07:07:12AM +0000, Lu, Han wrote:
Again, please fix your mailer to word wrap within paragraphs - this mail is very difficult to read.
On Tue, Mar 10, 2015 at 10:41:23AM +0800, han.lu@intel.com wrote:
+static int hsw_waves_switch_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol) {- struct snd_soc_platform *platform =
snd_soc_kcontrol_platform(kcontrol);
- struct hsw_priv_data *pdata =
snd_soc_platform_get_drvdata(platform);
- struct sst_hsw *hsw = pdata->hsw;
- int ret = 0;
- enum sst_hsw_module_id id = SST_HSW_MODULE_WAVES;
- bool switch_on = (bool)ucontrol->value.integer.value[0];
- if (sst_hsw_is_module_loaded(hsw, id)) {
if (switch_on == sst_hsw_is_module_active(hsw, id))return 0;/* ipc is valid only when module is loaded */if (switch_on)ret = sst_hsw_module_enable(hsw, id, 0);elseret = sst_hsw_module_disable(hsw, id, 0);- }
I'm not clear how this setting is saved when the module is not in RAM on the DSP. If the module enable/disable calls do that that's fine but I'd expect the function to return an error if the module isn't loaded (or better yet for the control not to be made visible to userspace at all) - we shouldn't be silently ignoring what the user is setting.
[Han] if module is not in RAM on the DSP, sst_hsw_is_module_loaded() will return false. Also sst_hsw_module_enable() and sst_hsw_module_disable() will check the module status in DSP before send IPC commands to DSP. In case sst_hsw_is_module_loaded() return false, (eg. System go RTD3 and modules in DSP RAM be unloaded) the user setting will be stored in variant u32 enabled_modules_rtd3, which is implemented in patch 0005. So which is better for next version of patch, to add error return here, merge patch 0005 with 0004, or do not change this part? Thanks.
It's better to merge the two patches so that it's clear that everything is working fine - in general the idea is that things should work after every patch and people shouldn't be finding problems in the patch series which are fixed later on in the same series.