[alsa-devel] [PATCH] ASoC: dapm: Fix snd_soc_dapm_put_volsw() connect

[Takashi Iwai]

At Mon, 18 Jun 2012 22:41:28 +0200 (CEST),
Benoît Thébaudeau wrote:
> 
> snd_soc_dapm_put_volsw() sets connect incorrectly in the case max > 1 with
> invert. In that case, the raw disconnect value should be max, which corresponds
> to the userspace value 0.
> 
> This use case currently does not appear upstream, but it could break
> SOC_DAPM_SINGLE() or SOC_DAPM_SINGLE_TLV() elsewhere or in the future.
> 
> Cc: Liam Girdwood <lrg at ti.com>
> Cc: Mark Brown <broonie at opensource.wolfsonmicro.com>
> Cc: <alsa-devel at alsa-project.org>
> Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> ---
>  .../sound/soc/soc-dapm.c                           |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git linux-next-HEAD-6c86b58.orig/sound/soc/soc-dapm.c linux-next-HEAD-6c86b58/sound/soc/soc-dapm.c
> index 405841c..5ef082f 100644
> --- linux-next-HEAD-6c86b58.orig/sound/soc/soc-dapm.c
> +++ linux-next-HEAD-6c86b58/sound/soc/soc-dapm.c
> @@ -2515,19 +2515,13 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
>  	int wi;
>  
>  	val = (ucontrol->value.integer.value[0] & mask);
> +	connect = !!val;
>  
>  	if (invert)
>  		val = max - val;
>  	mask = mask << shift;
>  	val = val << shift;
>  
> -	if (val)
> -		/* new connection */
> -		connect = invert ? 0 : 1;
> -	else
> -		/* old connection must be powered down */
> -		connect = invert ? 1 : 0;
> -

Doesn't this result in the same value of connect?

(given value, invert) --> (raw value, connect)

old code:  (0, 0) --> (0, 0)
           (0, 1) --> (max, 0)
           (max, 0) -> (max, 1)
           (max, 1) -> (0, 1)

new code:  (0, 0) --> (0, 0)
           (0, 1) --> (max, 0)
           (max, 0) --> (max, 1)
           (max, 1) --> (0, 1)

I'd understand if the line "connect = !!val;" were after the invert
conversion...

  	if (invert)
  		val = max - val;
	connect = !!val;


thanks,

Takashi