[PATCH v3] ALSA: hda/tas2781: Fix the order of TAS2781 calibrated-data

[PATCH v3] ALSA: hda/tas2781: Fix the order of TAS2781 calibrated-data

[Shenghao Ding]

A bug reported by one of my customers that the order of TAS2781
calibrated-data is incorrect, the correct way is to move R0_Low
and insert it between R0 and InvR0.

Fixes: 4fe238513407 ("ALSA: hda/tas2781: Move and unified the calibrated-data getting function for SPI and I2C into the tas2781_hda lib")
Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
v3:
 - Take Tiwai's advice on cali_cnv() to make it more simpler.
v2:
 - Submit to sound branch maintianed by Tiwai instead of linux-next branch
 - Drop other fix
---
 sound/hda/codecs/side-codecs/tas2781_hda.c | 25 +++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/sound/hda/codecs/side-codecs/tas2781_hda.c b/sound/hda/codecs/side-codecs/tas2781_hda.c
index f46d2e06c64f..f4a44c6b0234 100644
--- a/sound/hda/codecs/side-codecs/tas2781_hda.c
+++ b/sound/hda/codecs/side-codecs/tas2781_hda.c
@@ -33,6 +33,23 @@ const efi_guid_t tasdev_fct_efi_guid[] = {
 };
 EXPORT_SYMBOL_NS_GPL(tasdev_fct_efi_guid, "SND_HDA_SCODEC_TAS2781");
 
+/*
+ * The order of calibrated-data writing function is a bit different from the
+ * order in UEFI. Here is the conversion to match the order of calibrated-data
+ * writing function.
+ */
+static void cali_cnv(unsigned char *data, unsigned int base, int offset)
+{
+	struct cali_reg reg_data;
+
+	memcpy(&reg_data, data, sizeof(reg_data));
+	/* the data order has to be swapped between r0_low_reg and inv0_reg */
+	swap(reg_data.r0_low_reg, reg_data.invr0_reg);
+
+	cpu_to_be32_array((__force __be32 *)(data + offset + 1),
+		(u32 *)&reg_data, TASDEV_CALIB_N);
+}
+
 static void tas2781_apply_calib(struct tasdevice_priv *p)
 {
 	struct calidata *cali_data = &p->cali_data;
@@ -103,8 +120,7 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
 
 				data[l] = k;
 				oft++;
-				for (i = 0; i < TASDEV_CALIB_N * 4; i++)
-					data[l + i + 1] = data[4 * oft + i];
+				cali_cnv(data, 4 * oft, l);
 				k++;
 			}
 		}
@@ -130,9 +146,8 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
 
 		for (j = p->ndev - 1; j >= 0; j--) {
 			l = j * (cali_data->cali_dat_sz_per_dev + 1);
-			for (i = TASDEV_CALIB_N * 4; i > 0 ; i--)
-				data[l + i] = data[p->index * 5 + i];
-			data[l+i] = j;
+			cali_cnv(data, cali_data->cali_dat_sz_per_dev * j, l);
+			data[l] = j;
 		}
 	}
 
-- 
2.43.0

Re: [PATCH v3] ALSA: hda/tas2781: Fix the order of TAS2781 calibrated-data

[Gergo Koteles]

Hi Shenghao,

On Sat, 2025-09-06 at 17:19 +0800, Shenghao Ding wrote:
> A bug reported by one of my customers that the order of TAS2781
> calibrated-data is incorrect, the correct way is to move R0_Low
> and insert it between R0 and InvR0.
> 
> Fixes: 4fe238513407 ("ALSA: hda/tas2781: Move and unified the calibrated-data getting function for SPI and I2C into the tas2781_hda lib")
> Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
> 
> ---
> v3:
>  - Take Tiwai's advice on cali_cnv() to make it more simpler.
> v2:
>  - Submit to sound branch maintianed by Tiwai instead of linux-next branch
>  - Drop other fix
> ---
>  sound/hda/codecs/side-codecs/tas2781_hda.c | 25 +++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/tas2781_hda.c b/sound/hda/codecs/side-codecs/tas2781_hda.c
> index f46d2e06c64f..f4a44c6b0234 100644
> --- a/sound/hda/codecs/side-codecs/tas2781_hda.c
> +++ b/sound/hda/codecs/side-codecs/tas2781_hda.c
> @@ -33,6 +33,23 @@ const efi_guid_t tasdev_fct_efi_guid[] = {
>  };
>  EXPORT_SYMBOL_NS_GPL(tasdev_fct_efi_guid, "SND_HDA_SCODEC_TAS2781");
>  
> +/*
> + * The order of calibrated-data writing function is a bit different from the
> + * order in UEFI. Here is the conversion to match the order of calibrated-data
> + * writing function.
> + */
> +static void cali_cnv(unsigned char *data, unsigned int base, int offset)

unused base?

> +{
> +	struct cali_reg reg_data;
> +
> +	memcpy(&reg_data, data, sizeof(reg_data));

data points to cali_data?

> +	/* the data order has to be swapped between r0_low_reg and inv0_reg */
> +	swap(reg_data.r0_low_reg, reg_data.invr0_reg);
> +
> +	cpu_to_be32_array((__force __be32 *)(data + offset + 1),
> +		(u32 *)&reg_data, TASDEV_CALIB_N);
> +}
> +
>  static void tas2781_apply_calib(struct tasdevice_priv *p)
>  {
>  	struct calidata *cali_data = &p->cali_data;
> @@ -103,8 +120,7 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
>  
>  				data[l] = k;
>  				oft++;
> -				for (i = 0; i < TASDEV_CALIB_N * 4; i++)
> -					data[l + i + 1] = data[4 * oft + i];
> +				cali_cnv(data, 4 * oft, l);
>  				k++;
>  			}
>  		}
> @@ -130,9 +146,8 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
>  
>  		for (j = p->ndev - 1; j >= 0; j--) {
>  			l = j * (cali_data->cali_dat_sz_per_dev + 1);
> -			for (i = TASDEV_CALIB_N * 4; i > 0 ; i--)
> -				data[l + i] = data[p->index * 5 + i];
> -			data[l+i] = j;
> +			cali_cnv(data, cali_data->cali_dat_sz_per_dev * j, l);
> +			data[l] = j;
>  		}
>  	}
>  

Gergo

RE: [EXTERNAL] Re: [PATCH v3] ALSA: hda/tas2781: Fix the order of TAS2781 calibrated-data

[Ding, Shenghao]

Thanks for your comment, I will prepare a new patch soon.

> -----Original Message-----
> From: Gergo Koteles <soyer@irl.hu>
> Sent: Saturday, September 6, 2025 8:13 PM
> To: Ding, Shenghao <shenghao-ding@ti.com>; tiwai@suse.de
> Cc: broonie@kernel.org; andriy.shevchenko@linux.intel.com;
> 13564923607@139.com; 13916275206@139.com; alsa-devel@alsa-
> project.org; linux-kernel@vger.kernel.org; Xu, Baojun <baojun.xu@ti.com>;
> Baojun.Xu@fpt.com
> Subject: [EXTERNAL] Re: [PATCH v3] ALSA: hda/tas2781: Fix the order of
> TAS2781 calibrated-data
> 
> Hi Shenghao, On Sat, 2025-09-06 at 17: 19 +0800, Shenghao Ding wrote: > A bug
> reported by one of my customers that the order of TAS2781 > calibrated-data is
> incorrect, the correct way is to move R0_Low > and insert it between R0 and
> ZjQcmQRYFpfptBannerStart This message was sent from outside of Texas
> Instruments.
> Do not click links or open attachments unless you recognize the source of this
> email and know the content is safe.
> <https://us-phishalarm-
> ewt.proofpoint.com/EWT/v1/G3vK!uldgPTePPA3xaizufNmp9kStdEQS36lcAc9Q
> wr41j6O4ieidIV-4ba5O3e2zdceeW_86IkqVuODKMJ11ZYqjt-
> Y4PnTTtX6uNRJ38dLuTVI$>
> Report Suspicious
> 
> ZjQcmQRYFpfptBannerEnd
> Hi Shenghao,
> 
> On Sat, 2025-09-06 at 17:19 +0800, Shenghao Ding wrote:
> > A bug reported by one of my customers that the order of TAS2781
> > calibrated-data is incorrect, the correct way is to move R0_Low and
> > insert it between R0 and InvR0.
> >
> > Fixes: 4fe238513407 ("ALSA: hda/tas2781: Move and unified the
> > calibrated-data getting function for SPI and I2C into the tas2781_hda
> > lib")
> > Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
> >
> > ---
> > v3:
> >  - Take Tiwai's advice on cali_cnv() to make it more simpler.
> > v2:
> >  - Submit to the sound branch maintained by Tiwai instead of linux-next
> > branch
> >  - Drop other fix
> > ---
> >  sound/hda/codecs/side-codecs/tas2781_hda.c | 25
> > +++++++++++++++++-----
> >  1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/sound/hda/codecs/side-codecs/tas2781_hda.c
> > b/sound/hda/codecs/side-codecs/tas2781_hda.c
> > index f46d2e06c64f..f4a44c6b0234 100644
> > --- a/sound/hda/codecs/side-codecs/tas2781_hda.c
> > +++ b/sound/hda/codecs/side-codecs/tas2781_hda.c
> > @@ -33,6 +33,23 @@ const efi_guid_t tasdev_fct_efi_guid[] = {  };
> > EXPORT_SYMBOL_NS_GPL(tasdev_fct_efi_guid,
> "SND_HDA_SCODEC_TAS2781");
> >
> > +/*
> > + * The order of calibrated-data writing function is a bit different
> > +from the
> > + * order in UEFI. Here is the conversion to match the order of
> > +calibrated-data
> > + * writing function.
> > + */
> > +static void cali_cnv(unsigned char *data, unsigned int base, int
> > +offset)
> 
> unused base?
> 
> > +{
> > +	struct cali_reg reg_data;
> > +
> > +	memcpy(&reg_data, data, sizeof(reg_data));
> 
> data points to cali_data?
it should add base
memcpy(&reg_data, &data[base], sizeof(reg_data));
> 
> > +	/* the data order has to be swapped between r0_low_reg and inv0_reg
> */
> > +	swap(reg_data.r0_low_reg, reg_data.invr0_reg);
> > +
> > +	cpu_to_be32_array((__force __be32 *)(data + offset + 1),
> > +		(u32 *)&reg_data, TASDEV_CALIB_N);
> > +}
> > +
> >  static void tas2781_apply_calib(struct tasdevice_priv *p)  {
> >  	struct calidata *cali_data = &p->cali_data; @@ -103,8 +120,7 @@
> > static void tas2781_apply_calib(struct tasdevice_priv *p)
> >
> >  				data[l] = k;
> >  				oft++;
> > -				for (i = 0; i < TASDEV_CALIB_N * 4; i++)
> > -					data[l + i + 1] = data[4 * oft + i];
> > +				cali_cnv(data, 4 * oft, l);
> >  				k++;
> >  			}
> >  		}
> > @@ -130,9 +146,8 @@ static void tas2781_apply_calib(struct
> > tasdevice_priv *p)
> >
> >  		for (j = p->ndev - 1; j >= 0; j--) {
> >  			l = j * (cali_data->cali_dat_sz_per_dev + 1);
> > -			for (i = TASDEV_CALIB_N * 4; i > 0 ; i--)
> > -				data[l + i] = data[p->index * 5 + i];
> > -			data[l+i] = j;
> > +			cali_cnv(data, cali_data->cali_dat_sz_per_dev * j, l);
> > +			data[l] = j;
> >  		}
> >  	}
> >
> 
> Gergo