[alsa-devel] [PATCH v2 4/8] ASoC: Intel: add kcontrol to enable/disable sound effect module waves

[Lu, Han]

> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Wednesday, March 11, 2015 4:38 AM
> To: Lu, Han
> Cc: alsa-devel at alsa-project.org; Girdwood, Liam R
> Subject: Re: [PATCH v2 4/8] ASoC: Intel: add kcontrol to enable/disable
> sound effect module waves
> 
> On Tue, Mar 10, 2015 at 10:41:23AM +0800, han.lu at 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);
> > +		else
> > +			ret = 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.