On Wed, Sep 10, 2025 at 05:49:14PM +0530, Niranjan H Y wrote:

new file mode 100644 index 000000000..ddadfb7c5 --- /dev/null +++ b/sound/soc/codecs/tas2783-sdw.c @@ -0,0 +1,1417 @@

+#if !IS_ENABLED(CONFIG_SND_SOC_TAS2783_UTIL) +static void tas25xx_register_misc(struct sdw_slave *peripheral) {} +static void tas25xx_deregister_misc(void) {} +#endif

This config symbol doesn't exist already and isn't defined by the patch. If it did exist I'd expect these stubs to be in a header file rather than a C file. What's going on here?

+static s32 tas2783_digital_getvol(struct snd_kcontrol *kcontrol,

• 		  struct snd_ctl_elem_value *ucontrol)
  

+{

• struct snd_soc_component *component =

• snd_soc_kcontrol_component(kcontrol);
  

• struct tas2783_prv *tas_dev =

• snd_soc_component_get_drvdata(component);
  

• struct soc_mixer_control *mc =

• (struct soc_mixer_control *)kcontrol->private_value;
  

• s32 val, ret;
• ret = regmap_read(tas_dev->regmap, mc->reg, &val);
• if (ret < 0)

• return ret;
  

• ucontrol->value.integer.value[0] =

• tas_clamp(val, mc->max, mc->invert);
  

It feels like rather than define a custom read operation it'd be better to make this clamping part of the generic ASoC ops if it's needed - the code is totally generic and you could see something similar for any device that has a limited set of values that are valid. Presumably your device happens to trigger this in some common situation but anything could be affected and we should handle it generically.

+static s32 tas2783_digital_putvol(struct snd_kcontrol *kcontrol,

• 		  struct snd_ctl_elem_value *ucontrol)
  

+{

• val = tas_clamp(ucontrol->value.integer.value[0],

• 	mc->max, mc->invert);
  

This appears to be the only non-standard part of this function but the generic implementation should already deal with invalid inputs.

• ret = regmap_write(tas_dev->regmap, mc->reg, val);
• if (ret)

• dev_dbg(tas_dev->dev, "put vol %d into %x %x\n",
  

• 	val, mc->reg, ret);
  

• return ret;

This won't return 1 on change, mixer-test will tell you that (and spot a bunch of other errors).

+static const struct snd_kcontrol_new tas2783_snd_controls[] = {

• SOC_SINGLE_RANGE_EXT_TLV("Amp Gain Volume", TAS2783_AMP_LEVEL,

• 		 1, 0, 20, 0, tas2783_amp_getvol,
  

• 		 tas2783_amp_putvol, tas2781_amp_tlv),
  

Just Amp Volume.