[PATCH v2] ALSA: hda/tas2781: Fix EFI name for calibration beginning with 1 instead of 0
A bug reported by one of my customers that EFI name beginning with 0 instead of 1.
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
--- v2: - remove unrelated change v1: - Fix EFI name beginning with 1 instead of 0 - Add extra comments on EFI name for calibration - Remove an extra space --- sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c index ed7771ab9475..635cbd8820ac 100644 --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c @@ -340,7 +340,8 @@ static int tas2563_save_calibration(struct tas2781_hda *h) data[offset] = i; offset++; for (j = 0; j < TASDEV_CALIB_N; ++j) { - ret = snprintf(var8, sizeof(var8), vars[j], i); + /* EFI name for calibration started with 1, not 0 */ + ret = snprintf(var8, sizeof(var8), vars[j], i + 1);
if (ret < 0 || ret >= sizeof(var8) - 1) { dev_err(p->dev, "%s: Read %s failed\n", @@ -349,7 +350,7 @@ static int tas2563_save_calibration(struct tas2781_hda *h) } /* * Our variable names are ASCII by construction, but - * EFI names are wide chars. Convert and zero-pad. + * EFI names are wide chars. Convert and zero-pad. */ memset(efi_name, 0, sizeof(efi_name)); for (k = 0; k < sizeof(var8) && var8[k]; k++)
On Tue, 26 Aug 2025 11:41:05 +0200, Shenghao Ding wrote:
A bug reported by one of my customers that EFI name beginning with 0 instead of 1.
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
v2:
- remove unrelated change
v1:
- Fix EFI name beginning with 1 instead of 0
- Add extra comments on EFI name for calibration
- Remove an extra space
sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c index ed7771ab9475..635cbd8820ac 100644 --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c @@ -340,7 +340,8 @@ static int tas2563_save_calibration(struct tas2781_hda *h) data[offset] = i; offset++; for (j = 0; j < TASDEV_CALIB_N; ++j) {
ret = snprintf(var8, sizeof(var8), vars[j], i);
/* EFI name for calibration started with 1, not 0 */ret = snprintf(var8, sizeof(var8), vars[j], i + 1); if (ret < 0 || ret >= sizeof(var8) - 1) { dev_err(p->dev, "%s: Read %s failed\n",@@ -349,7 +350,7 @@ static int tas2563_save_calibration(struct tas2781_hda *h) } /* * Our variable names are ASCII by construction, but
* EFI names are wide chars. Convert and zero-pad.
* EFI names are wide chars. Convert and zero-pad. */ memset(efi_name, 0, sizeof(efi_name)); for (k = 0; k < sizeof(var8) && var8[k]; k++)
Please drop unrelated change and concentrate on the real issue.
thanks,
Takashi
Hi Shenghao,
On Tue, 2025-08-26 at 17:41 +0800, Shenghao Ding wrote:
A bug reported by one of my customers that EFI name beginning with 0 instead of 1.
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
v2:
- remove unrelated change
v1:
- Fix EFI name beginning with 1 instead of 0
- Add extra comments on EFI name for calibration
- Remove an extra space
sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c index ed7771ab9475..635cbd8820ac 100644 --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c @@ -340,7 +340,8 @@ static int tas2563_save_calibration(struct tas2781_hda *h) data[offset] = i; offset++; for (j = 0; j < TASDEV_CALIB_N; ++j) {
ret = snprintf(var8, sizeof(var8), vars[j], i);
/* EFI name for calibration started with 1, not 0 */ret = snprintf(var8, sizeof(var8), vars[j], i + 1); if (ret < 0 || ret >= sizeof(var8) - 1) { dev_err(p->dev, "%s: Read %s failed\n",@@ -349,7 +350,7 @@ static int tas2563_save_calibration(struct tas2781_hda *h) } /* * Our variable names are ASCII by construction, but
* EFI names are wide chars. Convert and zero-pad.
* EFI names are wide chars. Convert and zero-pad. */ memset(efi_name, 0, sizeof(efi_name)); for (k = 0; k < sizeof(var8) && var8[k]; k++)
The previous tas2781_apply_calib() and tas2563_apply_calib() functions performed a big endian conversion on the data readed from the EFI variables.
I couldn't find this in either fmwlib or this file. Could you please recheck if this happens somewhere?
Thanks, Gergo
Hi Shenghao,
On Tue, 2025-08-26 at 17:41 +0800, Shenghao Ding wrote:
A bug reported by one of my customers that EFI name beginning with 0 instead of 1.
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
v2:
- remove unrelated change
v1:
- Fix EFI name beginning with 1 instead of 0
- Add extra comments on EFI name for calibration
- Remove an extra space
sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c index ed7771ab9475..635cbd8820ac 100644 --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c @@ -340,7 +340,8 @@ static int tas2563_save_calibration(struct tas2781_hda *h) data[offset] = i; offset++; for (j = 0; j < TASDEV_CALIB_N; ++j) {
ret = snprintf(var8, sizeof(var8), vars[j], i);
/* EFI name for calibration started with 1, not 0 */ret = snprintf(var8, sizeof(var8), vars[j], i + 1); if (ret < 0 || ret >= sizeof(var8) - 1) { dev_err(p->dev, "%s: Read %s failed\n",@@ -349,7 +350,7 @@ static int tas2563_save_calibration(struct tas2781_hda *h) } /* * Our variable names are ASCII by construction, but
* EFI names are wide chars. Convert and zero-pad.
* EFI names are wide chars. Convert and zero-pad. */ memset(efi_name, 0, sizeof(efi_name)); for (k = 0; k < sizeof(var8) && var8[k]; k++)
In the tas2563_save_calibration() function the variables are read in the following order: R0, InvR0, R0_Low, Power, TLim. They are also included in cali_data in this order.
But the tasdev_load_calibrated_data() function reads them from cali_data as R0, R0_Low, InvR0, Power, TLim.
And this may be true for tas2781 as well.
Could you check this also?
Thanks, Gergo
Hi GK Long time no see. Thanks for pointing that out. I'm preparing a new patch to correct it.
-----Original Message----- From: Gergo Koteles soyer@irl.hu Sent: Wednesday, August 27, 2025 9:34 AM 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; Ji, Jesse jesse-ji@ti.com Subject: [EXTERNAL] Re: [PATCH v2] ALSA: hda/tas2781: Fix EFI name for calibration beginning with 1 instead of 0
Hi Shenghao, On Tue, 2025-08-26 at 17: 41 +0800, Shenghao Ding wrote: > A bug reported by one of my customers that EFI name beginning with 0 > instead of 1. > > Fixes: 4fe238513407 ("ALSA: hda/tas2781: Move and unified the calibrated-data 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!uldrHTaPnO07KyzOfDOJHMHGipcjY2REMQ tg2sq6EsrNqo3BvxwK7HK1Pa6IoEqp0AsXy_amOqGIF7xIufCxaZfP5b4W5MYIwV xonydOKJA$ Report Suspicious
ZjQcmQRYFpfptBannerEnd Hi Shenghao,
On Tue, 2025-08-26 at 17:41 +0800, Shenghao Ding wrote:
A bug reported by one of my customers that EFI name beginning with 0 instead of 1.
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
v2:
- remove unrelated change
v1:
- Fix EFI name beginning with 1 instead of 0
- Add extra comments on EFI name for calibration
- Remove an extra space
sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c index ed7771ab9475..635cbd8820ac 100644 --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c @@ -340,7 +340,8 @@ static int tas2563_save_calibration(struct
tas2781_hda *h)
data[offset] = i; offset++; for (j = 0; j < TASDEV_CALIB_N; ++j) {
ret = snprintf(var8, sizeof(var8), vars[j], i);
/* EFI name for calibration started with 1, not 0 */ret = snprintf(var8, sizeof(var8), vars[j], i + 1); if (ret < 0 || ret >= sizeof(var8) - 1) { dev_err(p->dev, "%s: Read %s failed\n", @@ -349,7 +350,7 @@
static int tas2563_save_calibration(struct tas2781_hda *h) } /* * Our variable names are ASCII by construction, but
* EFI names are wide chars. Convert and zero-pad.
* EFI names are wide chars. Convert and zero-pad. */ memset(efi_name, 0, sizeof(efi_name)); for (k = 0; k < sizeof(var8) && var8[k]; k++)In the tas2563_save_calibration() function the variables are read in the following order: R0, InvR0, R0_Low, Power, TLim. They are also included in cali_data in this order.
But the tasdev_load_calibrated_data() function reads them from cali_data as R0, R0_Low, InvR0, Power, TLim.
And this may be true for tas2781 as well.
Could you check this also?
Thanks, Gergo
Hi Shenghao,
On Wed, 2025-08-27 at 14:15 +0000, Ding, Shenghao wrote:
Long time no see. Thanks for pointing that out. I'm preparing a new patch to correct it.
Yeah, the amps worked well, so no need to write :)
Rolling distros are moving to kernel 6.16 soon, so I'm worried about the speakers.
Can the amplifier protect the speaker even with completely wrong calibration values?
Thanks, Gergo
participants (4)
-
Ding, Shenghao -
Gergo Koteles -
Shenghao Ding
-
Takashi Iwai