Re: [PATCH v4 2/6] ASoC: ops: add snd_soc_get_volsw_clamped

Re: [PATCH v4 2/6] ASoC: ops: add snd_soc_get_volsw_clamped

[Holalu Yogendra, Niranjan]

> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, September 12, 2025 12:02 AM
> Subject: Re: [EXTERNAL] Re: [PATCH v4 2/6] ASoC: ops: add
> snd_soc_get_volsw_clamped
> 
> On Thu, Sep 11, 2025 at 05:21:14PM +0000, Holalu Yogendra, Niranjan wrote:
> > > From: Mark Brown <broonie@kernel.org>
> 
> >   Since few of the drivers as still using the original API, and was not sure if
> > everyone wanted to clamp the register value, did not want to disturb the
> > original API snd_soc_get_volsw. Per my analysis, if register read fails due to
> In what situation would anything want to report an invalid value?
I am not sure if I fully follow. But in my case, for one of the cases,
valid amp volume is 0x0 to 0x14 with mask 0x1f - so if the register value read is
0x15 to 0x1f, it is invalid. But in the next patch, I will add the logic to clamp the value.

> > Are you thinking that this patch could be dropped and clamp the value to max in  
> > 'soc_get_volsw' (as in 1st patch) and use the same API ? Please suggest.
> Yes, everything should report valid values.  I/O errors should be
> propagated as errors.
I will update the next patch accordingly. 

Thanks 
Niranjan H Y

[PATCH v4 1/6] ASoC: ops: improve snd_soc_get_volsw

[Niranjan H Y]

* add error handling in case register read fails
* add support for clamping the values if the register
  value read is greater than max value

Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
---
 sound/soc/soc-ops.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index a629e0eac..59e91741b 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -111,10 +111,14 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
 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 clamp)
 {
 	int val = (reg_val >> shift) & mask;
 
+	if (clamp)
+		val = clamp(val, 0, mc->max);
+
 	if (mc->sign_bit)
 		val = sign_extend32(val, mc->sign_bit);
 
@@ -245,29 +249,44 @@ 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 clamp)
 {
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	unsigned int reg_val;
-	int val;
+	int val, ret = 0;
 
 	reg_val = snd_soc_component_read(component, mc->reg);
-	val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max);
+	val = reg_val;
+	if (val < 0) {
+		ret = val;
+		goto get_volsw_done;
+	}
+
+	val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max, clamp);
 
 	ucontrol->value.integer.value[0] = val;
 
 	if (snd_soc_volsw_is_stereo(mc)) {
 		if (mc->reg == mc->rreg) {
-			val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, max);
+			val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift,
+						   max, clamp);
 		} else {
 			reg_val = snd_soc_component_read(component, mc->rreg);
-			val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max);
+			val = reg_val;
+			if (val < 0) {
+				ret = val;
+				goto get_volsw_done;
+			}
+			val = soc_mixer_reg_to_ctl(mc, reg_val, mask,
+						   mc->shift, max, clamp);
 		}
 
 		ucontrol->value.integer.value[1] = val;
 	}
 
-	return 0;
+get_volsw_done:
+	return ret;
 }
 
 /**
@@ -330,7 +349,8 @@ 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);
 
@@ -372,7 +392,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, false);
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx);
 
-- 
2.45.2

[PATCH v4 2/6] ASoC: ops: add snd_soc_get_volsw_clamped

[Niranjan H Y]

  Add API snd_soc_get_volsw_clamped to read
  the volume from device. This similar to
  existing API snd_soc_get_volsw. But has
  additional step to clamp the value to max value
  if the register value is larger than max value.

Signed-off-by: Niranjan H Y <niranjan.hy@ti.com>
---
 include/sound/soc.h |  2 ++
 sound/soc/soc-ops.c | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 1fffef311..34a490ece 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -553,6 +553,8 @@ int snd_soc_info_volsw_sx(struct snd_kcontrol *kcontrol,
 #define snd_soc_info_bool_ext		snd_ctl_boolean_mono_info
 int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_get_volsw_clamped(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_put_volsw(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
 #define snd_soc_get_volsw_2r snd_soc_get_volsw
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 59e91741b..4d1ee5cd2 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -354,6 +354,28 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_volsw);
 
+/**
+ * snd_soc_get_volsw_clamped - single mixer get callback with range
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Similar to snd_soc_get_volsw, with additional operation to
+ * clamp the register value to its maximum.
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_get_volsw_clamped(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc =
+		(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, true);
+}
+EXPORT_SYMBOL_GPL(snd_soc_get_volsw_clamped);
+
 /**
  * snd_soc_put_volsw - single mixer put callback with range
  * @kcontrol: mixer control
-- 
2.45.2

Re: [PATCH v4 2/6] ASoC: ops: add snd_soc_get_volsw_clamped

[Mark Brown]

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

On Thu, Sep 11, 2025 at 09:26:59PM +0530, Niranjan H Y wrote:
>   Add API snd_soc_get_volsw_clamped to read
>   the volume from device. This similar to
>   existing API snd_soc_get_volsw. But has
>   additional step to clamp the value to max value
>   if the register value is larger than max value.

Why is this a separate API, what is the situation where we would want to
report an invalid value from a control?  I was thinking of just adding
this to the existing operations rather than adding separate ones that
need to be explicitly set.

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