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

["Ding, Shenghao" <shenghao-ding@ti.com>]

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