[PATCH v1 0/4] ASoC: ops: Fix stereo change notifications
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
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);
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; }
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;}
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/85b667d190953231ef3...
On Mon, Apr 18, 2022 at 04:33:21PM -0500, Pierre-Louis Bossart wrote:
if (ret == 0 || err < 0) {ret = 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.
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;
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);
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart