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

[Lars-Peter Clausen <lars@metafoo.de>]

On 05/27/2014 01:41 PM, Peter Ujfalusi wrote:
> 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.

Before the patch the value was stored in widget->value, after the patch the 
value is stored in the kcontrol. The tlv320aic3x driver did not store 
anything in widget->value before the patch and neither does it store 
anything in the kcontrol after the patch.

>
>> 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.

dapm_kcontrol_is_powered() always returns true if the control is not a 
auto-muted control. And there are no auto-muted controls in the tlv320aix3c 
driver.

There are two cases where dapm_kcontrol_get_value() can be called
a) The control is a virtual control
b) The control is a auto-muted control

Neither is the case here, which is why I expect that the real issue is 
something else and it is just shadowed by this patch.

- Lars