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

["Ding, Shenghao" <shenghao-ding@xxxxxx>]

Thanks for your ref code
> > +/*
> > + * The order of calibrated-data writing is a bit different from the
> > +order
> > + * in UEFI. Here is the conversion to match the order of
> > +calibrated-data
> > + * writing.
> > + */
> > +static void cali_cnv(unsigned char *data, unsigned int base, int
> > +offset) {
> > +	__be32 bedata[TASDEV_CALIB_N];
> > +	int i;
> > +
> > +	/* r0_reg */
> > +	bedata[0] = cpu_to_be32(*(uint32_t *)&data[base]);
> > +	/* r0_low_reg */
> > +	bedata[1] = cpu_to_be32(*(uint32_t *)&data[base + 8]);
> > +	/* invr0_reg */
> > +	bedata[2] = cpu_to_be32(*(uint32_t *)&data[base + 4]);
> > +	/* pow_reg */
> > +	bedata[3] = cpu_to_be32(*(uint32_t *)&data[base + 12]);
> > +	/* tlimit_reg */
> > +	bedata[4] = cpu_to_be32(*(uint32_t *)&data[base + 16]);
> > +
> > +	for (i = 0; i < TASDEV_CALIB_N; i++)
> > +		memcpy(&data[offset + i * 4 + 1], &bedata[i],
> > +			sizeof(bedata[i]));
> > +}
> 
> IMO, this can be more readable when you use struct calidata, e.g.
> 
> static void cali_cnv(unsigned char *data, unsigned int base, int offset) {
> 	struct calidata reg;
> 
> 	reg.r0_reg = *(u32 *)&data[base]
> 	reg.r0_low_reg = *(u32 *)&data[base + 8]
> 	reg.invr0_reg = *(u32 *)&data[base + 4]
> 	reg.pow_reg = *(u32 *)&data[base + 12];
> 	reg.tlimit_reg = *(u32 *)&data[base + 16]);
> 
> 	cpu_to_be32_array((__force __be32 *)(data + offset + 1), &reg,
> 			  TASDEV_CALIB_N);
> }
> 
> ... or even simpler like:
> 
> static void cali_cnv(unsigned char *data, unsigned int base, int offset) {
> 	struct calidata reg;
> 
> 	memcpy(&reg, data, sizeof(reg));
> 	/* the data order has to be swapped between r0_low_reg and inv0_reg
> */
> 	swap(reg.r0_low_reg, reg.invr0_reg);
> 
> 	cpu_to_be32_array((__force __be32 *)(data + offset + 1), &reg,
> 			  TASDEV_CALIB_N);
> }
I like this code so much. It's elegant simplicity.

Thanks,
Shenghao Ding