[PATCH 0/3] ASoC: tas2562: Volume setting fixes

[PATCH 0/3] ASoC: tas2562: Volume setting fixes

[Mark Brown]

While reviewing another fix for the tas2562 volume control I noticed a
few issues with the put() operation, this series fixes them.

It's also a bit weird that the volume control is defined with twice as
many values as can actually be set, probably the best fix there is to
regnerate the table of volume values with the intermediate values.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (3):
      ASoC: tas2562: Validate values for volume writes
      ASoC: tas2562: Fix event generation for volume control
      ASoC: tas2562: Fix default digital volume

 sound/soc/codecs/tas2562.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260715-asoc-tas2562-put-retval-a5dd3e90fdb2

Best regards,
--  
Mark Brown <broonie@kernel.org>

[PATCH 1/3] ASoC: tas2562: Validate values for volume writes

[Mark Brown]

tas2562_volume_control_put() does not do any validation of the control
value written by userspace, it uses it to look up a value in a fixed
size array which can easily be overflowed and then writes whatever value
it gets back to the device.  Add validation that we are loading a value
we have in the array.

Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 sound/soc/codecs/tas2562.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/tas2562.c b/sound/soc/codecs/tas2562.c
index 2f7cfc2be970..82bc0078a5c7 100644
--- a/sound/soc/codecs/tas2562.c
+++ b/sound/soc/codecs/tas2562.c
@@ -471,10 +471,14 @@ static int tas2562_volume_control_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct tas2562_data *tas2562 = snd_soc_component_get_drvdata(component);
-	int ret;
+	int ret, index;
 	u32 reg_val;
 
-	reg_val = float_vol_db_lookup[ucontrol->value.integer.value[0]/2];
+	index = ucontrol->value.integer.value[0] / 2;
+	if (index < 0 || index >= ARRAY_SIZE(float_vol_db_lookup))
+		return -EINVAL;
+
+	reg_val = float_vol_db_lookup[index];
 	ret = snd_soc_component_write(component, TAS2562_DVC_CFG4,
 				      (reg_val & 0xff));
 	if (ret)

-- 
2.47.3

[PATCH 2/3] ASoC: tas2562: Fix event generation for volume control

[Mark Brown]

ALSA put() operations should return 0 for noop updates and 1 if the
value of the control changed, this is used by the ALSA core to generate
events to userspace.  tas2562_volume_control_put() does not implement
this, it just writes whatever value userspace wrote to the device and
returns 0 regardless of what the previous value was.  Fix this by
suppressing writes if the value is unchanged and returning 1 if the
writes succeed.

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

diff --git a/sound/soc/codecs/tas2562.c b/sound/soc/codecs/tas2562.c
index 82bc0078a5c7..9b863b000470 100644
--- a/sound/soc/codecs/tas2562.c
+++ b/sound/soc/codecs/tas2562.c
@@ -474,6 +474,9 @@ static int tas2562_volume_control_put(struct snd_kcontrol *kcontrol,
 	int ret, index;
 	u32 reg_val;
 
+	if (tas2562->volume_lvl == ucontrol->value.integer.value[0])
+		return 0;
+
 	index = ucontrol->value.integer.value[0] / 2;
 	if (index < 0 || index >= ARRAY_SIZE(float_vol_db_lookup))
 		return -EINVAL;
@@ -498,7 +501,7 @@ static int tas2562_volume_control_put(struct snd_kcontrol *kcontrol,
 
 	tas2562->volume_lvl = ucontrol->value.integer.value[0];
 
-	return 0;
+	return 1;
 }
 
 /* Digital Volume Control. From 0 dB to -110 dB in 1 dB steps */

-- 
2.47.3

[PATCH 3/3] ASoC: tas2562: Fix default digital volume

[Mark Brown]

The tas2562 digital volume is spread over four registers and is implemented
as lookups into a table so the driver stores the value for the userspace
control in the driver data. This defaults to 0 due to kzalloc() but the
register default is 0x40400000 which maps onto something a bit over the
largest value defined in the lookup table. While it's not an exact match
update the default to the largest value, avoiding user surprise due to a
sudden change on first write.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/tas2562.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/codecs/tas2562.c b/sound/soc/codecs/tas2562.c
index 9b863b000470..5fdbfec8ce3f 100644
--- a/sound/soc/codecs/tas2562.c
+++ b/sound/soc/codecs/tas2562.c
@@ -738,6 +738,8 @@ static int tas2562_probe(struct i2c_client *client)
 	data->client = client;
 	data->dev = &client->dev;
 	data->model_id = (uintptr_t)i2c_get_match_data(client);
+	/* Register default is 0x40400000, this is closest */
+	data->volume_lvl = (ARRAY_SIZE(float_vol_db_lookup) - 1) * 2;
 
 	tas2562_parse_dt(data);
 

-- 
2.47.3

Re: [PATCH 0/3] ASoC: tas2562: Volume setting fixes

[Cezary Rojewski]

On 7/15/2026 10:18 PM, Mark Brown wrote:
> While reviewing another fix for the tas2562 volume control I noticed a
> few issues with the put() operation, this series fixes them.
> 
> It's also a bit weird that the volume control is defined with twice as
> many values as can actually be set, probably the best fix there is to
> regnerate the table of volume values with the intermediate values.
For the series:

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

Re: [PATCH 3/3] ASoC: tas2562: Fix default digital volume

[Cezary Rojewski]

On 7/15/2026 10:18 PM, Mark Brown wrote:
> The tas2562 digital volume is spread over four registers and is implemented
> as lookups into a table so the driver stores the value for the userspace
> control in the driver data. This defaults to 0 due to kzalloc() but the
> register default is 0x40400000 which maps onto something a bit over the
> largest value defined in the lookup table. While it's not an exact match
> update the default to the largest value, avoiding user surprise due to a
> sudden change on first write.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   sound/soc/codecs/tas2562.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/codecs/tas2562.c b/sound/soc/codecs/tas2562.c
> index 9b863b000470..5fdbfec8ce3f 100644
> --- a/sound/soc/codecs/tas2562.c
> +++ b/sound/soc/codecs/tas2562.c
> @@ -738,6 +738,8 @@ static int tas2562_probe(struct i2c_client *client)
>   	data->client = client;
>   	data->dev = &client->dev;
>   	data->model_id = (uintptr_t)i2c_get_match_data(client);
> +	/* Register default is 0x40400000, this is closest */
> +	data->volume_lvl = (ARRAY_SIZE(float_vol_db_lookup) - 1) * 2;
Sounds like bug fix to me and a Fixes: candidate.  That's a nitpick 
though and not a blocker.