Re: [PATCH v2 11/13] ASoC: Intel: mrfld: add the DSP DAPM widgets

[Vinod Koul <vinod.koul@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1387 bytes --]

On Fri, Jul 18, 2014 at 01:19:20PM +0100, Mark Brown wrote:
> On Thu, Jul 10, 2014 at 10:14:55AM +0530, Subhransu S. Prusty wrote:
> 
> > +	pr_debug("%s: widget = %s\n", __func__, w->name);
> > +	for (i = 0; i < w->num_kcontrols; i++) {
> > +		if (dapm_kcontrol_get_value(w->kcontrols[i])) {
> > +			mc = (struct soc_mixer_control *)(w->kcontrols[i])->private_value;
> > +			val |= 1 << mc->shift;
> > +		}
> > +	}
> 
> So, this is the usage of dapm_kcontrol_get_value() (quite a way away
> from the patch exporting it!).  The usage here looks *very* strange.
> We're calling the function but treating the result as a boolean and
> manually decoding the DAPM data structures in order to get the control
> shift...  that's odd to say the least.
> 
> > +	SST_FILL_DESTINATION(2, cmd.output_id,
> > +			     ids->location_id, SST_DEFAULT_MODULE_ID);
> > +	cmd.nb_inputs =	fill_swm_input(&cmd.input[0], val);
> 
> So what we're doing here is parsing the controls to get which inputs are
> enabled...  it's not altogether clear to me that we shouldn't be doing
> this at control update time.  Presumably we'll also need to be sending
> these messages when the controls are updated to account for changes that
> happen while streams are active.
yes thats the idea. Here we send as mixer path is On. Yes the mixer update
if On does send this as well.

-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]