On Fri, Jun 28, 2024 at 12:18:43PM +0800, Shenghao Ding wrote: > -static int tas2781_force_fwload_get(struct snd_kcontrol *kcontrol, > +static int tasdev_force_fwload_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > struct snd_soc_component *component = > @@ -118,7 +119,7 @@ static int tas2781_force_fwload_get(struct snd_kcontrol *kcontrol, > return 0; > } > > -static int tas2781_force_fwload_put(struct snd_kcontrol *kcontrol, > +static int tasdev_force_fwload_put(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) Are these fwload changes really related to the rest of the change? They feel like they should be a separate patch, even if it's a peparatory one for this. > +static int tas2563_digital_gain_put( > + struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + for (i = 0; i < tas_dev->ndev; i++) { > + ret = tasdevice_dev_bulk_write(tas_dev, i, reg, > + (unsigned char *)tas2563_dvc_table[vol], 4); > + if (ret) > + dev_err(tas_dev->dev, > + "%s, set digital vol error in device %d\n", > + __func__, i); > + } > + > +out: > + mutex_unlock(&tas_dev->codec_lock); > + return ret; > +} This needs to return 1 if the value is being changed, the mixer-test selftest will tell you about this and other issues. > +static const struct snd_kcontrol_new tas2563_snd_controls[] = { > + SOC_SINGLE_RANGE_EXT_TLV("Speaker Digital Gain", TAS2563_DVC_LVL, 0, > + 0, ARRAY_SIZE(tas2563_dvc_table) - 1, 0, > + tas2563_digital_gain_get, tas2563_digital_gain_put, > + tas2563_dvc_tlv), > }; Volume controls should end in Volume, again mixer-test will tell you this - please run mixer-test. > static int tasdevice_codec_probe(struct snd_soc_component *codec) > { > struct tasdevice_priv *tas_priv = snd_soc_component_get_drvdata(codec); > + int rc; > + > + if (tas_priv->chip_id == TAS2781) { > + } else { If you write this as a switch statement it'll be more extensible.
Attachment:
signature.asc
Description: PGP signature