[PATCH] ASoC: DAPM: Cover regression by kctl change notification fix
Cezary Rojewski
cezary.rojewski at intel.com
Fri Nov 5 13:03:06 CET 2021
On 2021-11-05 12:36 PM, Takashi Iwai wrote:
> On Fri, 05 Nov 2021 12:30:48 +0100,
> Cezary Rojewski wrote:
>>
>> On 2021-11-05 10:09 AM, Takashi Iwai wrote:
>>> The recent fix for DAPM to correct the kctl change notification by the
>>> commit 5af82c81b2c4 ("ASoC: DAPM: Fix missing kctl change
>>> notifications") caused other regressions since it changed the behavior
>>> of snd_soc_dapm_set_pin() that is called from several API functions.
>>> Formerly it returned always 0 for success, but now it returns 0 or 1.
>>>
>>> This patch addresses it, restoring the old behavior of
>>> snd_soc_dapm_set_pin() while keeping the fix in
>>> snd_soc_dapm_put_pin_switch().
>>>
>>> Fixes: 5af82c81b2c4 ("ASoC: DAPM: Fix missing kctl change notifications")
>>> Reported-by: Yu-Hsuan Hsu <yuhsuan at chromium.org>
>>> Cc: <stable at vger.kernel.org>
>>> Signed-off-by: Takashi Iwai <tiwai at suse.de>
>>
>> Hello,
>>
>> From my research I've found very few places which actually check the
>> returned value of functions mentioned by you. And thus we could use
>> this opportunity to adjust the kcontrol-put behavior according to
>> documentation for all users without adding any additional functions
>> which are part of this patch.
>>
>> Board:
>> sound/soc/intel/boards/kbl_da7219_max98927.c
>>
>> seems to be the offending user. We could update its code instead,
>> leaving ASoC unchanged. What do you think?
>
> Well, if we're going to that direction, we have to update the
> documentation and properly mention about the positive return value.
> So the changes in ASoC core would be needed nevertheless.
>
> I find it good to keep the old existing behavior; those API functions
> are only for enabling/disabling, so returning 0 or a negative error is
> more natural than tri-state, less complex for callers.
Hmm, indeed ASoC API doc would need an update, ASoC DAPM api != kcontrol
api after all.
My impression of original patch was the desire to align the behaviour of
snd_soc_dapm_enable/disable_pin() and similar functions with
kcontrol-put equivalents - in terms of the returned value. Perhaps
that's not the case.
While it's preferred the keep the existing behaviour, if we were to
align DAPM functions with kcontrol-put ones, fact that vast majority of
users ignore the returned value, helps us out.
Regards,
Czarek
More information about the Alsa-devel
mailing list