[PATCH v1] ASoC: ops: fix snd_soc_get_volsw for sx controls

[PATCH v1] ASoC: ops: fix snd_soc_get_volsw for sx controls

[Stefan Binding]

SX controls are currently broken, since the clamp introduced in
commit a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw") does not
handle SX controls, for example where the min value in the clamp is
greater than the max value in the clamp.

Add clamp parameter to prevent clamping in SX controls.
The nature of SX controls mean that it wraps around 0, with a variable
number of bits, therefore clamping the value becomes complicated and
prone to error.

Fixes 35 kunit tests for soc_ops_test_access.

Fixes: a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw")

Co-developed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/soc/soc-ops.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index ce86978c158d..624e9269fc25 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -111,7 +111,8 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
 static int sdca_soc_q78_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_val,
-				unsigned int mask, unsigned int shift, int max)
+				   unsigned int mask, unsigned int shift, int max,
+				   bool sx)
 {
 	int val = reg_val;
 
@@ -141,20 +142,26 @@ static unsigned int sdca_soc_q78_ctl_to_reg(struct soc_mixer_control *mc, int va
 }
 
 static int soc_mixer_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_val,
-				unsigned int mask, unsigned int shift, int max)
+				unsigned int mask, unsigned int shift, int max,
+				bool sx)
 {
 	int val = (reg_val >> shift) & mask;
 
 	if (mc->sign_bit)
 		val = sign_extend32(val, mc->sign_bit);
 
-	val = clamp(val, mc->min, mc->max);
-	val -= mc->min;
+	if (sx) {
+		val -= mc->min; // SX controls intentionally can overflow here
+		val = min_t(unsigned int, val & mask, max);
+	} else {
+		val = clamp(val, mc->min, mc->max);
+		val -= mc->min;
+	}
 
 	if (mc->invert)
 		val = max - val;
 
-	return val & mask;
+	return val;
 }
 
 static unsigned int soc_mixer_ctl_to_reg(struct soc_mixer_control *mc, int val,
@@ -280,9 +287,10 @@ static int soc_put_volsw(struct snd_kcontrol *kcontrol,
 
 static int soc_get_volsw(struct snd_kcontrol *kcontrol,
 			 struct snd_ctl_elem_value *ucontrol,
-			 struct soc_mixer_control *mc, int mask, int max)
+			 struct soc_mixer_control *mc, int mask, int max, bool sx)
 {
-	int (*reg_to_ctl)(struct soc_mixer_control *, unsigned int, unsigned int, unsigned int, int);
+	int (*reg_to_ctl)(struct soc_mixer_control *, unsigned int, unsigned int,
+			  unsigned int, int, bool);
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	unsigned int reg_val;
 	int val;
@@ -293,16 +301,16 @@ static int soc_get_volsw(struct snd_kcontrol *kcontrol,
 		reg_to_ctl = soc_mixer_reg_to_ctl;
 
 	reg_val = snd_soc_component_read(component, mc->reg);
-	val = reg_to_ctl(mc, reg_val, mask, mc->shift, max);
+	val = reg_to_ctl(mc, reg_val, mask, mc->shift, max, sx);
 
 	ucontrol->value.integer.value[0] = val;
 
 	if (snd_soc_volsw_is_stereo(mc)) {
 		if (mc->reg == mc->rreg) {
-			val = reg_to_ctl(mc, reg_val, mask, mc->rshift, max);
+			val = reg_to_ctl(mc, reg_val, mask, mc->rshift, max, sx);
 		} else {
 			reg_val = snd_soc_component_read(component, mc->rreg);
-			val = reg_to_ctl(mc, reg_val, mask, mc->shift, max);
+			val = reg_to_ctl(mc, reg_val, mask, mc->shift, max, sx);
 		}
 
 		ucontrol->value.integer.value[1] = val;
@@ -371,7 +379,7 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
 		(struct soc_mixer_control *)kcontrol->private_value;
 	unsigned int mask = soc_mixer_mask(mc);
 
-	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min);
+	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min, false);
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_volsw);
 
@@ -413,7 +421,7 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol,
 		(struct soc_mixer_control *)kcontrol->private_value;
 	unsigned int mask = soc_mixer_sx_mask(mc);
 
-	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max);
+	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max, true);
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx);
 
-- 
2.43.0

Re: [PATCH v1] ASoC: ops: fix snd_soc_get_volsw for sx controls

[Péter Ujfalusi]

On 16/12/2025 15:49, Stefan Binding wrote:
> SX controls are currently broken, since the clamp introduced in
> commit a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw") does not
> handle SX controls, for example where the min value in the clamp is
> greater than the max value in the clamp.
> 
> Add clamp parameter to prevent clamping in SX controls.
> The nature of SX controls mean that it wraps around 0, with a variable
> number of bits, therefore clamping the value becomes complicated and
> prone to error.
> 
> Fixes 35 kunit tests for soc_ops_test_access.
> 
> Fixes: a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw")
> 
> Co-developed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Tested-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

> ---
>  sound/soc/soc-ops.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index ce86978c158d..624e9269fc25 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c
> @@ -111,7 +111,8 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
>  EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
>  
>  static int sdca_soc_q78_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_val,
> -				unsigned int mask, unsigned int shift, int max)
> +				   unsigned int mask, unsigned int shift, int max,
> +				   bool sx)
>  {
>  	int val = reg_val;
>  
> @@ -141,20 +142,26 @@ static unsigned int sdca_soc_q78_ctl_to_reg(struct soc_mixer_control *mc, int va
>  }
>  
>  static int soc_mixer_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_val,
> -				unsigned int mask, unsigned int shift, int max)
> +				unsigned int mask, unsigned int shift, int max,
> +				bool sx)
>  {
>  	int val = (reg_val >> shift) & mask;
>  
>  	if (mc->sign_bit)
>  		val = sign_extend32(val, mc->sign_bit);
>  
> -	val = clamp(val, mc->min, mc->max);
> -	val -= mc->min;
> +	if (sx) {
> +		val -= mc->min; // SX controls intentionally can overflow here
> +		val = min_t(unsigned int, val & mask, max);
> +	} else {
> +		val = clamp(val, mc->min, mc->max);
> +		val -= mc->min;
> +	}
>  
>  	if (mc->invert)
>  		val = max - val;
>  
> -	return val & mask;
> +	return val;
>  }
>  
>  static unsigned int soc_mixer_ctl_to_reg(struct soc_mixer_control *mc, int val,
> @@ -280,9 +287,10 @@ static int soc_put_volsw(struct snd_kcontrol *kcontrol,
>  
>  static int soc_get_volsw(struct snd_kcontrol *kcontrol,
>  			 struct snd_ctl_elem_value *ucontrol,
> -			 struct soc_mixer_control *mc, int mask, int max)
> +			 struct soc_mixer_control *mc, int mask, int max, bool sx)
>  {
> -	int (*reg_to_ctl)(struct soc_mixer_control *, unsigned int, unsigned int, unsigned int, int);
> +	int (*reg_to_ctl)(struct soc_mixer_control *, unsigned int, unsigned int,
> +			  unsigned int, int, bool);
>  	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
>  	unsigned int reg_val;
>  	int val;
> @@ -293,16 +301,16 @@ static int soc_get_volsw(struct snd_kcontrol *kcontrol,
>  		reg_to_ctl = soc_mixer_reg_to_ctl;
>  
>  	reg_val = snd_soc_component_read(component, mc->reg);
> -	val = reg_to_ctl(mc, reg_val, mask, mc->shift, max);
> +	val = reg_to_ctl(mc, reg_val, mask, mc->shift, max, sx);
>  
>  	ucontrol->value.integer.value[0] = val;
>  
>  	if (snd_soc_volsw_is_stereo(mc)) {
>  		if (mc->reg == mc->rreg) {
> -			val = reg_to_ctl(mc, reg_val, mask, mc->rshift, max);
> +			val = reg_to_ctl(mc, reg_val, mask, mc->rshift, max, sx);
>  		} else {
>  			reg_val = snd_soc_component_read(component, mc->rreg);
> -			val = reg_to_ctl(mc, reg_val, mask, mc->shift, max);
> +			val = reg_to_ctl(mc, reg_val, mask, mc->shift, max, sx);
>  		}
>  
>  		ucontrol->value.integer.value[1] = val;
> @@ -371,7 +379,7 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
>  		(struct soc_mixer_control *)kcontrol->private_value;
>  	unsigned int mask = soc_mixer_mask(mc);
>  
> -	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min);
> +	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min, false);
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_get_volsw);
>  
> @@ -413,7 +421,7 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol,
>  		(struct soc_mixer_control *)kcontrol->private_value;
>  	unsigned int mask = soc_mixer_sx_mask(mc);
>  
> -	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max);
> +	return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max, true);
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx);
>  

-- 
Péter

Re: [PATCH v1] ASoC: ops: fix snd_soc_get_volsw for sx controls

[David Gow]

Le 16/12/2025 à 9:49 PM, Stefan Binding a écrit :
> SX controls are currently broken, since the clamp introduced in
> commit a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw") does not
> handle SX controls, for example where the min value in the clamp is
> greater than the max value in the clamp.
> 
> Add clamp parameter to prevent clamping in SX controls.
> The nature of SX controls mean that it wraps around 0, with a variable
> number of bits, therefore clamping the value becomes complicated and
> prone to error.
> 
> Fixes 35 kunit tests for soc_ops_test_access.
> 
> Fixes: a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw")
> 
> Co-developed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---

Thanks very much: I've just bisected those failures to the same patch, 
so am definitely keen to see this fixed.

Tested-by: David Gow <david@davidgow.net>

Cheers,
-- David

Re: [PATCH v1] ASoC: ops: fix snd_soc_get_volsw for sx controls

[Mark Brown]

On Tue, 16 Dec 2025 13:49:20 +0000, Stefan Binding wrote:
> SX controls are currently broken, since the clamp introduced in
> commit a0ce874cfaaa ("ASoC: ops: improve snd_soc_get_volsw") does not
> handle SX controls, for example where the min value in the clamp is
> greater than the max value in the clamp.
> 
> Add clamp parameter to prevent clamping in SX controls.
> The nature of SX controls mean that it wraps around 0, with a variable
> number of bits, therefore clamping the value becomes complicated and
> prone to error.
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: ops: fix snd_soc_get_volsw for sx controls
      commit: 095d621141826a2841dae85b52c784c147ea99d3

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