Re: [PATCH 2/2] ASoC: tlv320aic3x: Fix Left/Rigth PGA mixer widgets/kcontrols

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

On 05/27/2014 02:18 PM, Lars-Peter Clausen wrote:
> On 05/27/2014 12:53 PM, Peter Ujfalusi wrote:
>> Commit "cf7c1de20c576 ASoC: dapm: Move 'value' field from widget to
>> control" changed the way how the 'value' has been stored for a widget.
>> Since the driver is not updated following the mentioned commit it is mostly
>> broken when we try to change mixers for Left/Right PGA since the value is
>> not going to be updated correctly.
> 
> I don't think the mentioned commit is the cause. The driver didn't set
> widget->value either, so there is no change in behavior here. The commit only
> changed where the value is stored not how it is handled.

Yes, exactly. The driver has not been updated after the place where the value
is stored, so it is not going to store it at all, which causes issues along
the way.

> This driver uses a custom put handler and the generic get handler.
> dapm_kcontrol_get_value() is used in the generic get handler, but only if reg
> equal to SND_SOC_NOPM. Which is not the case for this driver.

The relevant code is this:
	if (dapm_kcontrol_is_powered(kcontrol) && reg != SND_SOC_NOPM)
		val = (snd_soc_read(codec, reg) >> shift) & mask;
	else
		val = dapm_kcontrol_get_value(kcontrol);

and when the widget is not powered the code will try to get the value from the
dapm_kcontrol_data, which is not even touched by the custom code in the driver.

> And that's the
> only place where the value stored for the kcontrol is used. I don't see how
> calling dapm_kcontrol_set_value() or not calling dapm_kcontrol_set_value()
> changes anything.

When the widget is not powered the stored value is going to be used. The
custom code in the driver should mimic the code from the core which it is set
to replace/customize.

> 
> I might be missing something, but I think the issue is somewhere else.
> 
> - Lars


-- 
Péter