[PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Mohamed Yassine Jebabli via B4 Relay]

From: Mohamed Yassine Jebabli <med.jebabli@gmail.com>

Commit 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data
without CRC") only checked the first device entry. On machines where
the BIOS populates fewer entries than p->ndev, the remaining all-zero
entries are applied and tas2781_cali_preproc() divides by zero on
playback open.

Accept CRC-less V1 data only if every expected entry is non-zero and
self-consistent (R0 * 1/R0 close to 1.0), otherwise reject it as
before.

Fixes: 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data without CRC")
Reported-by: Cristian Castellano <cristian.castellano@icloud.com>
Closes: https://lore.kernel.org/all/4e500063-d985-46b6-a0a1-3feaa77c4b2f@leemhuis.info/
Cc: stable@vger.kernel.org
Signed-off-by: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
---
Cristian, could you test this on the Yoga Pro 9 16IRP8? With the fix,
the driver should log "V1 CRC error" again and fall back to default
parameters, as it did before 408249089c48. If it still logs
"accepting", p->ndev is 2 on your machine and the root cause is
elsewhere, so please send the full "dmesg | grep -i tas2781".

Takashi's re_cal check in tas2781_cali_preproc() is complementary
and still worth applying as a safeguard.

To: Takashi Iwai <tiwai@suse.com>
To: Jaroslav Kysela <perex@perex.cz>
Cc: regressions@lists.linux.dev
Cc: linux-sound@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Cristian Castellano <cristian.castellano@icloud.com>
Cc: Thorsten Leemhuis <regressions@leemhuis.info>
Cc: Shenghao Ding <shenghao-ding@ti.com>
Cc: Kevin Lu <kevin-lu@ti.com>
Cc: Baojun Xu <baojun.xu@ti.com>
Cc: Sen Wang <sen@ti.com> 
---
 sound/hda/codecs/side-codecs/tas2781_hda.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/sound/hda/codecs/side-codecs/tas2781_hda.c b/sound/hda/codecs/side-codecs/tas2781_hda.c
index e0da299df780..9162173ccb10 100644
--- a/sound/hda/codecs/side-codecs/tas2781_hda.c
+++ b/sound/hda/codecs/side-codecs/tas2781_hda.c
@@ -50,6 +50,31 @@ static void cali_cnv(unsigned char *data, unsigned int base, int offset)
 		(u32 *)&reg_data, TASDEV_CALIB_N);
 }
 
+/*
+ * V1 data without CRC is only trusted if every expected device has a
+ * populated, self-consistent entry: R0 (Q27) * 1/R0 (Q31) must be
+ * close to 1.0 in Q58.
+ */
+static bool tas2781_v1_nocrc_valid(const unsigned int *v, int ndev)
+{
+	const u64 one = 1ULL << 58;
+	int i;
+
+	if (ndev <= 0 || ndev > 4)
+		return false;
+
+	for (i = 0; i < ndev; i++) {
+		const unsigned int *e = &v[i * 5];
+		u64 prod = (u64)e[0] * e[1];
+
+		if (!e[0] || !e[1] ||
+		    prod < one - one / 10 || prod > one + one / 10)
+			return false;
+	}
+
+	return true;
+}
+
 static void tas2781_apply_calib(struct tasdevice_priv *p)
 {
 	struct calidata *cali_data = &p->cali_data;
@@ -145,7 +170,7 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
 			 * but leave TimeStamp and CRC zeroed.
 			 */
 			if (!tmp_val[20] && !tmp_val[21] &&
-			    tmp_val[0] && tmp_val[1]) {
+			    tas2781_v1_nocrc_valid(tmp_val, p->ndev)) {
 				dev_warn(p->dev,
 					 "%s: V1 data without CRC, accepting\n",
 					 __func__);

---
base-commit: d73cab4a427f924efd5b5d8769c80e0ca5bdcadf
change-id: 20261103-tas2781-v1-nocrc-fix-cecb33deea8f

Best regards,
-- 
Mohamed Yassine Jebabli <med.jebabli@gmail.com>

Re: [PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Takashi Iwai]

On Tue, 03 Nov 2026 10:27:12 +0100,
Mohamed Yassine Jebabli via B4 Relay wrote:
> 
> From: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
> 
> Commit 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data
> without CRC") only checked the first device entry. On machines where
> the BIOS populates fewer entries than p->ndev, the remaining all-zero
> entries are applied and tas2781_cali_preproc() divides by zero on
> playback open.
> 
> Accept CRC-less V1 data only if every expected entry is non-zero and
> self-consistent (R0 * 1/R0 close to 1.0), otherwise reject it as
> before.
> 
> Fixes: 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data without CRC")
> Reported-by: Cristian Castellano <cristian.castellano@icloud.com>
> Closes: https://lore.kernel.org/all/4e500063-d985-46b6-a0a1-3feaa77c4b2f@leemhuis.info/
> Cc: stable@vger.kernel.org
> Signed-off-by: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
> ---
> Cristian, could you test this on the Yoga Pro 9 16IRP8? With the fix,
> the driver should log "V1 CRC error" again and fall back to default
> parameters, as it did before 408249089c48. If it still logs
> "accepting", p->ndev is 2 on your machine and the root cause is
> elsewhere, so please send the full "dmesg | grep -i tas2781".
> 
> Takashi's re_cal check in tas2781_cali_preproc() is complementary
> and still worth applying as a safeguard.

As the code looks safe enough, I applied now.
Let me know if the problem still happens.

I'm going to submit my zero-check patch as a further hardening, too.


thanks,

Takashi

Re: [PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Gergo Koteles]

Hi Mohamed,

On Tue, 2026-11-03 at 10:27 +0100, Mohamed Yassine Jebabli via B4 Relay
wrote:
> From: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
> 
> Commit 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data
> without CRC") only checked the first device entry. On machines where
> the BIOS populates fewer entries than p->ndev, the remaining all-zero
> entries are applied and tas2781_cali_preproc() divides by zero on
> playback open.
> 
> Accept CRC-less V1 data only if every expected entry is non-zero and
> self-consistent (R0 * 1/R0 close to 1.0), otherwise reject it as
> before.
> 

...

>  
> +/*
> + * V1 data without CRC is only trusted if every expected device has a
> + * populated, self-consistent entry: R0 (Q27) * 1/R0 (Q31) must be
> + * close to 1.0 in Q58.
> + */

Just out of curiosity, where do Q27, Q31, Q58 come from?
I couldn't find anything about these.

Thanks,
Gergo

Re: [PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Gergo Koteles]

Hi,

On Wed, 2026-11-04 at 16:55 +0100, Gergo Koteles wrote:
> Hi Mohamed,
> 
> On Tue, 2026-11-03 at 10:27 +0100, Mohamed Yassine Jebabli via B4 Relay
> wrote:
> > From: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
> > 
> > Commit 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data
> > without CRC") only checked the first device entry. On machines where
> > the BIOS populates fewer entries than p->ndev, the remaining all-zero
> > entries are applied and tas2781_cali_preproc() divides by zero on
> > playback open.
> > 
> > Accept CRC-less V1 data only if every expected entry is non-zero and
> > self-consistent (R0 * 1/R0 close to 1.0), otherwise reject it as
> > before.
> > 
> 
> ...
> 
> >  
> > +/*
> > + * V1 data without CRC is only trusted if every expected device has a
> > + * populated, self-consistent entry: R0 (Q27) * 1/R0 (Q31) must be
> > + * close to 1.0 in Q58.
> > + */
> 
> Just out of curiosity, where do Q27, Q31, Q58 come from?
> I couldn't find anything about these.
> 
> 

I think it would be good to be able to see which commits were generated
by an LLM. This way, people wouldn't have to scour the internet trying
to verify hallucinations. Furthermore, it would be possible to see what
percentage of the kernel consists of public domain/LLM code.

Best regards,
Gergo

Re: [PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Gergo Koteles]

Hi Mohamed,

On Thu, 2026-11-12 at 16:09 +0100, Mohamed Yassine JEBABLI wrote:
> Hi Gergo,
> 
> Regarding the Q27/Q31/Q58 formats: they are not from any documentation; I
> inferred them from the data. Reading the first word of each entry as Q27
> gives a plausible speaker resistance (3.82 ohm on my Legion Pro 7, 3.64 ohm
> on Cristian's Yoga Pro 9), and reading the second word as Q31 gives exactly
> its reciprocal, within 1e-4, on both machines. So, for a consistent entry,
> R0 × InvR0 ~= 2^27 × 2^31 = 2^58.
> 
> The V2 dump from the ASUS ROG Xbox Ally X thread (with a valid CRC) fits
> the same pattern: r0 = 0x3c5f7222 (~7.55 ohm in Q27) and invr0 = 0x10f615d5
> (~0.1325 in Q31, i.e. 1/7.55).
> 
> My guess is Q5.27 and Q1.31, but I have no TI reference to confirm it.
> 
> Finally, all testing was done on my own hardware: the original patch was
> tested on my Legion Pro 7 16IRX9H, and I reviewed and own the changes.
> 

Ah okey, these are TI fixed point formats.

Thanks,
Gergo

Re: [PATCH] ALSA: hda/tas2781: Only accept CRC-less V1 data if all entries are valid

[Mohamed Yassine JEBABLI]

Hi Gergo,

Regarding the Q27/Q31/Q58 formats: they are not from any
documentation; I inferred them from the data. Reading the first word
of each entry as Q27 gives a plausible speaker resistance (3.82 ohm on
my Legion Pro 7, 3.64 ohm on Cristian's Yoga Pro 9), and reading the
second word as Q31 gives exactly its reciprocal, within 1e-4, on both
machines. So, for a consistent entry, R0 × InvR0 ~= 2^27 × 2^31 =
2^58.

The V2 dump from the ASUS ROG Xbox Ally X thread (with a valid CRC)
fits the same pattern: r0 = 0x3c5f7222 (~7.55 ohm in Q27) and invr0 =
0x10f615d5 (~0.1325 in Q31, i.e. 1/7.55).

My guess is Q5.27 and Q1.31, but I have no TI reference to confirm it.

Finally, all testing was done on my own hardware: the original patch
was tested on my Legion Pro 7 16IRX9H, and I reviewed and own the
changes.

Thanks,
Mohamed Yassine JEBABLI


Le jeu. 12 nov. 2026 à 16:09, Mohamed Yassine JEBABLI
<med.jebabli@gmail.com> a écrit :
>
> Hi Gergo,
>
> Regarding the Q27/Q31/Q58 formats: they are not from any documentation; I inferred them from the data. Reading the first word of each entry as Q27 gives a plausible speaker resistance (3.82 ohm on my Legion Pro 7, 3.64 ohm on Cristian's Yoga Pro 9), and reading the second word as Q31 gives exactly its reciprocal, within 1e-4, on both machines. So, for a consistent entry, R0 × InvR0 ~= 2^27 × 2^31 = 2^58.
>
> The V2 dump from the ASUS ROG Xbox Ally X thread (with a valid CRC) fits the same pattern: r0 = 0x3c5f7222 (~7.55 ohm in Q27) and invr0 = 0x10f615d5 (~0.1325 in Q31, i.e. 1/7.55).
>
> My guess is Q5.27 and Q1.31, but I have no TI reference to confirm it.
>
> Finally, all testing was done on my own hardware: the original patch was tested on my Legion Pro 7 16IRX9H, and I reviewed and own the changes.
>
> Thanks,
> Mohamed Yassine JEBABLI
>
>
> Le jeu. 12 nov. 2026, 15:43, Gergo Koteles <soyer@irl.hu> a écrit :
>>
>> Hi,
>>
>> On Wed, 2026-11-04 at 16:55 +0100, Gergo Koteles wrote:
>> > Hi Mohamed,
>> >
>> > On Tue, 2026-11-03 at 10:27 +0100, Mohamed Yassine Jebabli via B4 Relay
>> > wrote:
>> > > From: Mohamed Yassine Jebabli <med.jebabli@gmail.com>
>> > >
>> > > Commit 408249089c48 ("ALSA: hda/tas2781: Accept V1 calibration data
>> > > without CRC") only checked the first device entry. On machines where
>> > > the BIOS populates fewer entries than p->ndev, the remaining all-zero
>> > > entries are applied and tas2781_cali_preproc() divides by zero on
>> > > playback open.
>> > >
>> > > Accept CRC-less V1 data only if every expected entry is non-zero and
>> > > self-consistent (R0 * 1/R0 close to 1.0), otherwise reject it as
>> > > before.
>> > >
>> >
>> > ...
>> >
>> > >
>> > > +/*
>> > > + * V1 data without CRC is only trusted if every expected device has a
>> > > + * populated, self-consistent entry: R0 (Q27) * 1/R0 (Q31) must be
>> > > + * close to 1.0 in Q58.
>> > > + */
>> >
>> > Just out of curiosity, where do Q27, Q31, Q58 come from?
>> > I couldn't find anything about these.
>> >
>> >
>>
>> I think it would be good to be able to see which commits were generated
>> by an LLM. This way, people wouldn't have to scour the internet trying
>> to verify hallucinations. Furthermore, it would be possible to see what
>> percentage of the kernel consists of public domain/LLM code.
>>
>> Best regards,
>> Gergo