Re: [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable

Hi Val,

On Mon May 18, 2026 at 11:44 PM CEST, Val Packett wrote:
> - Invert the value to match userspace expectations (in the hardware,
>   positive numbers represent negative dB attenuation)
> - Provide TLV metadata for the dB scale (and divide the raw values by 2
>   as the excessive precision used by HW is not representable in TLV)
> - Do not unnecessarily reset the volume while switching profiles
> - Simplify aw88261_dev_set_volume using regmap_update_bits
> - Do not add the initial volume from the profile to the requested volume
>   as that would throw off the dB mapping (if a lower max limit is
>   desired, it can be set in the UCM profile in userspace)
>
> With this change, it's actually possible to use this hardware volume
> control as PlaybackVolume in an ALSA UCM profile.
>
> Fixes: 028a2ae25691 ("ASoC: codecs: Add aw88261 amplifier driver")
> Signed-off-by: Val Packett <[email protected]>
> ---
>  sound/soc/codecs/aw88261.c | 52 +++++++++++++++-----------------------
>  sound/soc/codecs/aw88261.h |  1 -
>  2 files changed, 21 insertions(+), 32 deletions(-)
>
> diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
> index adc728e45f57..0e6b2dfe5db9 100644
> --- a/sound/soc/codecs/aw88261.c
> +++ b/sound/soc/codecs/aw88261.c
> @@ -15,6 +15,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <sound/soc.h>
>  #include <sound/pcm_params.h>
> +#include <sound/tlv.h>
>  #include "aw88261.h"
>  #include "aw88395/aw88395_data_type.h"
>  #include "aw88395/aw88395_device.h"
> @@ -29,20 +30,10 @@ static const struct regmap_config aw88261_remap_config = {
>  
>  static void aw88261_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
>  {
> -	struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
> -	unsigned int real_value, volume;
> -	unsigned int reg_value;
> +	unsigned int volume = min(value, (unsigned int)AW88261_MUTE_VOL);
>  
> -	volume = min((value + vol_desc->init_volume), (unsigned int)AW88261_MUTE_VOL);
> -	real_value = DB_TO_REG_VAL(volume);
> -
> -	regmap_read(aw_dev->regmap, AW88261_SYSCTRL2_REG, &reg_value);
> -
> -	real_value = (real_value | (reg_value & AW88261_VOL_START_MASK));
> -
> -	dev_dbg(aw_dev->dev, "value 0x%x , real_value:0x%x", value, real_value);
> -
> -	regmap_write(aw_dev->regmap, AW88261_SYSCTRL2_REG, real_value);
> +	regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL2_REG,
> +		~AW88261_VOL_MASK, DB_TO_REG_VAL(volume));
>  }
>  
>  static void aw88261_dev_i2s_tx_enable(struct aw_device *aw_dev, bool flag)
> @@ -424,17 +415,7 @@ static int aw88261_dev_reg_update(struct aw88261 *aw88261,
>  			break;
>  	}
>  
> -	ret = aw88261_dev_set_vcalb(aw_dev);
> -	if (ret)
> -		return ret;
> -
> -	if (aw_dev->prof_cur != aw_dev->prof_index)
> -		vol_desc->ctl_volume = 0;
> -
> -	/* keep min volume */
> -	aw88261_dev_set_volume(aw_dev, vol_desc->mute_volume);
> -
> -	return ret;
> +	return aw88261_dev_set_vcalb(aw_dev);

These removals cause the following compile warning, when applied on
linux-next:

sound/soc/codecs/aw88261.c: In function 'aw88261_dev_reg_update':
sound/soc/codecs/aw88261.c:347:32: warning: unused variable 'vol_desc' [-Wunused-variable]
  347 |         struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
      |                                ^~~~~~~~

Regards
Luca