[PATCH v1 0/4] ASoC: ops: Fix stereo change notifications

[PATCH v1 0/4] ASoC: ops: Fix stereo change notifications

[Mark Brown]

The event generation coverage I just wrote shows that the generic ASoC
ops fail to generate events for stereo controls when only the first
channel is changed, we just return the status for the second channel and
discard that for the first.

Mark Brown (4):
  ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw()
  ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()
  ASoC: ops: Fix stereo change notifications in
    snd_soc_put_volsw_range()
  ASoC: ops: Fix stereo change notifications in snd_soc_put_xr_sx()

 sound/soc/soc-ops.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)


base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
-- 
2.30.2

[PATCH v1 1/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw()

[Mark Brown]

When writing out a stereo control we discard the change notification from
the first channel, meaning that events are only generated based on changes
to the second channel. Ensure that we report a change if either channel
has changed.

Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-ops.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 08eaa9ddf191..73c9d53de25b 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -308,7 +308,7 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol,
 	unsigned int sign_bit = mc->sign_bit;
 	unsigned int mask = (1 << fls(max)) - 1;
 	unsigned int invert = mc->invert;
-	int err;
+	int err, ret;
 	bool type_2r = false;
 	unsigned int val2 = 0;
 	unsigned int val, val_mask;
@@ -336,12 +336,18 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol,
 	err = snd_soc_component_update_bits(component, reg, val_mask, val);
 	if (err < 0)
 		return err;
+	ret = err;
 
-	if (type_2r)
+	if (type_2r) {
 		err = snd_soc_component_update_bits(component, reg2, val_mask,
-			val2);
+						    val2);
+		/* Don't discard any error code or drop change flag */
+		if (ret == 0 || err < 0) {
+			ret = err;
+		}
+	}
 
-	return err;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_volsw);
 
-- 
2.30.2

[PATCH v1 2/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()

[Mark Brown]

When writing out a stereo control we discard the change notification from
the first channel, meaning that events are only generated based on changes
to the second channel. Ensure that we report a change if either channel
has changed.

Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-ops.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 73c9d53de25b..f0d1aeb38346 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -413,6 +413,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 	int min = mc->min;
 	unsigned int mask = (1U << (fls(min + max) - 1)) - 1;
 	int err = 0;
+	int ret;
 	unsigned int val, val_mask;
 
 	val_mask = mask << shift;
@@ -422,6 +423,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 	err = snd_soc_component_update_bits(component, reg, val_mask, val);
 	if (err < 0)
 		return err;
+	ret = err;
 
 	if (snd_soc_volsw_is_stereo(mc)) {
 		unsigned int val2;
@@ -432,6 +434,11 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 
 		err = snd_soc_component_update_bits(component, reg2, val_mask,
 			val2);
+
+		/* Don't discard any error code or drop change flag */
+		if (ret == 0 || err < 0) {
+			ret = err;
+		}
 	}
 	return err;
 }
-- 
2.30.2

Re: [PATCH v1 2/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()

[Pierre-Louis Bossart]

On 2/1/22 09:56, Mark Brown wrote:
> When writing out a stereo control we discard the change notification from
> the first channel, meaning that events are only generated based on changes
> to the second channel. Ensure that we report a change if either channel
> has changed.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  sound/soc/soc-ops.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index 73c9d53de25b..f0d1aeb38346 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c
> @@ -413,6 +413,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
>  	int min = mc->min;
>  	unsigned int mask = (1U << (fls(min + max) - 1)) - 1;
>  	int err = 0;
> +	int ret;
>  	unsigned int val, val_mask;
>  
>  	val_mask = mask << shift;
> @@ -422,6 +423,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
>  	err = snd_soc_component_update_bits(component, reg, val_mask, val);
>  	if (err < 0)
>  		return err;
> +	ret = err;
>  
>  	if (snd_soc_volsw_is_stereo(mc)) {
>  		unsigned int val2;
> @@ -432,6 +434,11 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
>  
>  		err = snd_soc_component_update_bits(component, reg2, val_mask,
>  			val2);
> +
> +		/* Don't discard any error code or drop change flag */
> +		if (ret == 0 || err < 0) {
> +			ret = err;
> +		}
>  	}
>  	return err;

cppcheck flags a warning on this patch, I believe we should use "return ret;" here, as done in the other patches of this series?

https://github.com/thesofproject/linux/pull/3597/commits/85b667d190953231ef314ac429019a011596f6d7

Re: [PATCH v1 2/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

On Mon, Apr 18, 2022 at 04:33:21PM -0500, Pierre-Louis Bossart wrote:
> > +		if (ret == 0 || err < 0) {
> > +			ret = err;
> > +		}
> >  	}
> >  	return err;

> cppcheck flags a warning on this patch, I believe we should use "return ret;" here, as done in the other patches of this series?

Yes, I think so.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

On Mon, Apr 18, 2022 at 04:33:21PM -0500, Pierre-Louis Bossart wrote:
> > +		if (ret == 0 || err < 0) {
> > +			ret = err;
> > +		}
> >  	}
> >  	return err;

> cppcheck flags a warning on this patch, I believe we should use "return ret;" here, as done in the other patches of this series?

Yes, I think so.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v1 3/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_range()

[Mark Brown]

When writing out a stereo control we discard the change notification from
the first channel, meaning that events are only generated based on changes
to the second channel. Ensure that we report a change if either channel
has changed.

Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-ops.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index f0d1aeb38346..fefd4f34cbc1 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -498,7 +498,7 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 	unsigned int mask = (1 << fls(max)) - 1;
 	unsigned int invert = mc->invert;
 	unsigned int val, val_mask;
-	int ret;
+	int err, ret;
 
 	if (invert)
 		val = (max - ucontrol->value.integer.value[0]) & mask;
@@ -507,9 +507,10 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 	val_mask = mask << shift;
 	val = val << shift;
 
-	ret = snd_soc_component_update_bits(component, reg, val_mask, val);
-	if (ret < 0)
-		return ret;
+	err = snd_soc_component_update_bits(component, reg, val_mask, val);
+	if (err < 0)
+		return err;
+	ret = err;
 
 	if (snd_soc_volsw_is_stereo(mc)) {
 		if (invert)
@@ -519,8 +520,12 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 		val_mask = mask << shift;
 		val = val << shift;
 
-		ret = snd_soc_component_update_bits(component, rreg, val_mask,
+		err = snd_soc_component_update_bits(component, rreg, val_mask,
 			val);
+		/* Don't discard any error code or drop change flag */
+		if (ret == 0 || err < 0) {
+			ret = err;
+		}
 	}
 
 	return ret;
-- 
2.30.2

[PATCH v1 4/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_xr_sx()

[Mark Brown]

When writing out a stereo control we discard the change notification from
the first channel, meaning that events are only generated based on changes
to the second channel. Ensure that we report a change if either channel
has changed.

Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/soc-ops.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index fefd4f34cbc1..6b922d12afb5 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -874,6 +874,7 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol,
 	unsigned long mask = (1UL<<mc->nbits)-1;
 	long max = mc->max;
 	long val = ucontrol->value.integer.value[0];
+	int ret = 0;
 	unsigned int i;
 
 	if (invert)
@@ -886,9 +887,11 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol,
 							regmask, regval);
 		if (err < 0)
 			return err;
+		if (err > 0)
+			ret = err;
 	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_xr_sx);
 
-- 
2.30.2

Re: [PATCH v1 0/4] ASoC: ops: Fix stereo change notifications

[Mark Brown]

On Tue, 1 Feb 2022 15:56:25 +0000, Mark Brown wrote:
> The event generation coverage I just wrote shows that the generic ASoC
> ops fail to generate events for stereo controls when only the first
> channel is changed, we just return the status for the second channel and
> discard that for the first.
> 
> Mark Brown (4):
>   ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw()
>   ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()
>   ASoC: ops: Fix stereo change notifications in
>     snd_soc_put_volsw_range()
>   ASoC: ops: Fix stereo change notifications in snd_soc_put_xr_sx()
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus

Thanks!

[1/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw()
      commit: 564778d7b1ea465f9487eedeece7527a033549c5
[2/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_sx()
      commit: 7f3d90a3519680dfa23e750f80bfdefc0f5eda4a
[3/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_volsw_range()
      commit: 650204ded3703b5817bd4b6a77fa47d333c4f902
[4/4] ASoC: ops: Fix stereo change notifications in snd_soc_put_xr_sx()
      commit: 2b7c46369f09c358164d31d17e5695185403185e

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark