[alsa-devel] [v4 10/12] ASoC: Intel: mrfld: add the DSP DAPM widgets

[Subhransu S. Prusty]

On Thu, Aug 14, 2014 at 02:07:08PM +0100, Mark Brown wrote:
> On Thu, Aug 14, 2014 at 03:51:56PM +0530, Subhransu S. Prusty wrote:
> > On Wed, Aug 13, 2014 at 09:14:54PM +0100, Mark Brown wrote:
> > > On Mon, Aug 04, 2014 at 03:16:01PM +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;
> > > > +		}
> > > > +	}
> 
> > > All my concerns about this still stand - using something called
> > > _get_value() to do something other than read the control value (which
> > > this does - it uses it as a boolean then does something else to read the
> > > value) which doesn't seem good.
> 
> > I think this can be optimized. This need a bit of rework in the code, but
> > need to check. But this would need an assumption that the controls are
> > created in order.
> 
> > for (i = 0; i < w->num_kcontrols; i++) {
> > 	if (dapm_kcontrol_get_value(w->kcontrols[i])) {
> > 		val |= 1 << i;
> > }
> 
> > Is this ok?
> 
> No, that doesn't seem safe.  Why not read the data from the control - if
> we were calling a function which returned the value that'd be much
> clearer.
So lets step back and see what is required here.

I get a mixer update for one of the inputs of the mixer. This needs to be
communicated to the DSP via the IPC. But the IPC expects that we send all
the values of all the inputs of a mixer to the DSP.

That is why here we are reading the all mixer inputs to find the values and
send the bitmap of all inputs.

So from the driver if I have to find out what is the value of the inputs in
mixer, how do I do that, is there a simpler way? Or should we push
this/<something else> into framework to query

--



--