linux-kernel - Re: [PATCH v1] ASoc: tas2781: Add new Kontrol to set tas2563 digital gain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 28 Jun 2024 13:27:36 +0100
From: Mark Brown <broonie@...nel.org>
To: Shenghao Ding <shenghao-ding@...com>
Cc: andriy.shevchenko@...ux.intel.com, lgirdwood@...il.com, perex@...ex.cz,
	pierre-louis.bossart@...ux.intel.com, 13916275206@....com,
	zhourui@...qin.com, alsa-devel@...a-project.org, i-salazar@...com,
	linux-kernel@...r.kernel.org, j-chadha@...com,
	liam.r.girdwood@...el.com, jaden-yue@...com,
	yung-chuan.liao@...ux.intel.com, dipa@...com, yuhsuan@...gle.com,
	henry.lo@...com, tiwai@...e.de, baojun.xu@...com, soyer@....hu,
	Baojun.Xu@....com, judyhsiao@...gle.com, navada@...com,
	cujomalainey@...gle.com, aanya@...com, nayeem.mahmud@...com,
	savyasanchi.shukla@...radyne.com, flaviopr@...rosoft.com,
	jesse-ji@...com, darren.ye@...iatek.com
Subject: Re: [PATCH v1] ASoc: tas2781: Add new Kontrol to set tas2563 digital
 gain

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)