Re: [PATCH v1] ALSA: hda/tas2781: Support dsp firmware Alpha and Beta seaies

[Takashi Iwai <tiwai@suse.de>]

On Tue, 25 Mar 2025 11:36:12 +0100,
Shenghao Ding wrote:
> 
> For calibration data, basic version firmware does not contain any
> calibration addresses, it depends on calibration tool to convey the
> addresses to the driver. Since Alpha and Beta firmware, all the
> calibration addresses are saved into the firmware.

The description is same with the other changes in ASoC and that's
*very* confusing.

Please give the proper description that matches with the actual
change.  Also consider a better subject line, too.

The same problem happened already a few times from your submitted
series in the past.  Please try to improve the process.


thanks,

Takashi

> Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
> 
> ---
> v1:
>  - Add updating calibration addresses code into tas2781_apply_calib in
>    case of Alpha and Beta firmware.
> ---
>  sound/pci/hda/tas2781_hda_i2c.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
> index 50c5e5f26589..cb3d683013d9 100644
> --- a/sound/pci/hda/tas2781_hda_i2c.c
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> @@ -558,28 +558,38 @@ static int tas2563_save_calibration(struct tasdevice_priv *tas_priv)
>  
>  static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
>  {
> -	static const unsigned char page_array[CALIB_MAX] = {
> -		0x17, 0x18, 0x18, 0x13, 0x18,
> +	struct calidata *cali_data = &tas_priv->cali_data;
> +	struct cali_reg *r = &cali_data->cali_reg_array;
> +	unsigned int cali_reg[CALIB_MAX] = {
> +		TASDEVICE_REG(0, 0x17, 0x74),
> +		TASDEVICE_REG(0, 0x18, 0x0c),
> +		TASDEVICE_REG(0, 0x18, 0x14),
> +		TASDEVICE_REG(0, 0x13, 0x70),
> +		TASDEVICE_REG(0, 0x18, 0x7c),
>  	};
> -	static const unsigned char rgno_array[CALIB_MAX] = {
> -		0x74, 0x0c, 0x14, 0x70, 0x7c,
> -	};
> -	int offset = 0;
>  	int i, j, rc;
> +	int oft = 0;
>  	__be32 data;
>  
> +	if (tas_priv->dspbin_typ != TASDEV_BASIC) {
> +		cali_reg[0] = r->r0_reg;
> +		cali_reg[1] = r->invr0_reg;
> +		cali_reg[2] = r->r0_low_reg;
> +		cali_reg[3] = r->pow_reg;
> +		cali_reg[4] = r->tlimit_reg;
> +	}
> +
>  	for (i = 0; i < tas_priv->ndev; i++) {
>  		for (j = 0; j < CALIB_MAX; j++) {
>  			data = cpu_to_be32(
> -				*(uint32_t *)&tas_priv->cali_data.data[offset]);
> +				*(uint32_t *)&tas_priv->cali_data.data[oft]);
>  			rc = tasdevice_dev_bulk_write(tas_priv, i,
> -				TASDEVICE_REG(0, page_array[j], rgno_array[j]),
> -				(unsigned char *)&data, 4);
> +				cali_reg[j], (unsigned char *)&data, 4);
>  			if (rc < 0)
>  				dev_err(tas_priv->dev,
>  					"chn %d calib %d bulk_wr err = %d\n",
>  					i, j, rc);
> -			offset += 4;
> +			oft += 4;
>  		}
>  	}
>  }
> -- 
> 2.34.1
>