Re: [PATCH] ASoC: DAPM: Cover regression by kctl change notification fix

[Takashi Iwai <tiwai@suse.de>]

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@chromium.org>
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Takashi Iwai <tiwai@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.


thanks,

Takashi