[v1] ALSA: ASoC/tas2781: fix wrong calibrated data order

[PATCH v1] ALSA: ASoC/tas2781: fix wrong calibrated data order

Posted by Shenghao Ding 1 year, 4 months ago

From: Baojun Xu <baojun.xu@ti.com>

Wrong calibration data order cause sound too low in some device.
Fix wrong calibrated data order, add calibration data converssion
by get_unaligned_be32() after reading from UEFI.

Fixes: 5be27f1e3ec9 ("ALSA: hda/tas2781: Add tas2781 HDA driver")
Signed-off-by: Baojun Xu <baojun.xu@ti.com>

---
v1:
 - Change copyright date, and add new maintainer.
 - Add unaligned.h included for get_unaligned_be32().
 - In tas2781_apply_calib(), change data address transfer directly to
   get data by get_unaligned_be32(), and send address to device.
---
 sound/pci/hda/tas2781_hda_i2c.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 49bd7097d..4dc3350d5 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -2,10 +2,12 @@
 //
 // TAS2781 HDA I2C driver
 //
-// Copyright 2023 Texas Instruments, Inc.
+// Copyright 2023 - 2024 Texas Instruments, Inc.
 //
 // Author: Shenghao Ding <shenghao-ding@ti.com>
+// Current maintainer: Baojun Xu <baojun.xu@ti.com>
 
+#include <asm/unaligned.h>
 #include <linux/acpi.h>
 #include <linux/crc8.h>
 #include <linux/crc32.h>
@@ -519,20 +521,21 @@ static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
 	static const unsigned char rgno_array[CALIB_MAX] = {
 		0x74, 0x0c, 0x14, 0x70, 0x7c,
 	};
-	unsigned char *data;
-	int i, j, rc;
+	int i, j, rc, data;
+	int offset = 0;
 
 	for (i = 0; i < tas_priv->ndev; i++) {
-		data = tas_priv->cali_data.data +
-			i * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
 		for (j = 0; j < CALIB_MAX; j++) {
+			data = get_unaligned_be32(
+				&tas_priv->cali_data.data[offset]);
 			rc = tasdevice_dev_bulk_write(tas_priv, i,
 				TASDEVICE_REG(0, page_array[j], rgno_array[j]),
-				&(data[4 * j]), 4);
+				(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;
 		}
 	}
 }
-- 
2.40.1

Re: [PATCH v1] ALSA: ASoC/tas2781: fix wrong calibrated data order

Posted by Takashi Iwai 1 year, 4 months ago

On Wed, 07 Aug 2024 09:55:40 +0200,
Shenghao Ding wrote:
> 
> From: Baojun Xu <baojun.xu@ti.com>
> 
> Wrong calibration data order cause sound too low in some device.
> Fix wrong calibrated data order, add calibration data converssion
> by get_unaligned_be32() after reading from UEFI.
> 
> Fixes: 5be27f1e3ec9 ("ALSA: hda/tas2781: Add tas2781 HDA driver")
> Signed-off-by: Baojun Xu <baojun.xu@ti.com>
> 
> ---
> v1:
>  - Change copyright date, and add new maintainer.
>  - Add unaligned.h included for get_unaligned_be32().
>  - In tas2781_apply_calib(), change data address transfer directly to
>    get data by get_unaligned_be32(), and send address to device.
> ---
>  sound/pci/hda/tas2781_hda_i2c.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
> index 49bd7097d..4dc3350d5 100644
> --- a/sound/pci/hda/tas2781_hda_i2c.c
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> @@ -2,10 +2,12 @@
>  //
>  // TAS2781 HDA I2C driver
>  //
> -// Copyright 2023 Texas Instruments, Inc.
> +// Copyright 2023 - 2024 Texas Instruments, Inc.
>  //
>  // Author: Shenghao Ding <shenghao-ding@ti.com>
> +// Current maintainer: Baojun Xu <baojun.xu@ti.com>
>  
> +#include <asm/unaligned.h>
>  #include <linux/acpi.h>
>  #include <linux/crc8.h>
>  #include <linux/crc32.h>
> @@ -519,20 +521,21 @@ static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
>  	static const unsigned char rgno_array[CALIB_MAX] = {
>  		0x74, 0x0c, 0x14, 0x70, 0x7c,
>  	};
> -	unsigned char *data;
> -	int i, j, rc;
> +	int i, j, rc, data;
> +	int offset = 0;

data should be __be32 type, to be more explicit.


Takashi

>  
>  	for (i = 0; i < tas_priv->ndev; i++) {
> -		data = tas_priv->cali_data.data +
> -			i * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
>  		for (j = 0; j < CALIB_MAX; j++) {
> +			data = get_unaligned_be32(
> +				&tas_priv->cali_data.data[offset]);
>  			rc = tasdevice_dev_bulk_write(tas_priv, i,
>  				TASDEVICE_REG(0, page_array[j], rgno_array[j]),
> -				&(data[4 * j]), 4);
> +				(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;
>  		}
>  	}
>  }
> -- 
> 2.40.1
>