[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1069077: es8316 driver causes kernel oops / panic on rockpro64



Control: retitle -1 es8316 driver causes kernel oops / panic on rockpro64

On Tuesday, 16 April 2024 01:37:57 CEST Forest wrote:
> The current debian unstable kernel causes a variety of failures that are not
> present in the bookworm kernel, on the RockPro64 single board computer.
> (This is an arm64 machine built upon the Rockchip rk3399 SoC.)

On Tuesday, 16 April 2024 05:21:13 CEST Forest wrote:
> Blacklisting the snd_soc_es8316 module in /etc/modprobe.d seems to restore
> kernel stability, as far as I have seen from half a dozen reboots.

Can you try the Debian Testing kernel, which is at version 6.6.15?

If the 6.6.15 kernel does work properly, then the 3 commits for
the es8316 driver in kernel 6.7 are the most likely suspects:
869f30782cda ASoC: es8316: Enable support for MCLK div by 2
a43c0dc1004c ASoC: es8316: Replace NR_SUPPORTED_MCLK_LRCK_RATIOS with ARRAY_SIZE()
2f06f231f0bf ASoC: es8316: Enable support for S32 LE format

Attached you'll find 3 patches which revert those commits.
https://kernel-team.pages.debian.net/kernel-handbook/ch-common-tasks.html#id-1.6.6.4
describes a procedure with which you can apply patches to the
(6.7) kernel. If that makes the 6.7 kernel work properly again, we
likely have found the culprit for the kernel oops/panic.

Can you first try the Testing (6.6.15) kernel and if that works try
applying the attached patches to the 6.7 kernel?
>From 407672343a738ede6f5e955e3afa57d16b37f4e6 Mon Sep 17 00:00:00 2001
From: Diederik de Haas <didi.debian@cknow.org>
Date: Tue, 16 Apr 2024 10:24:39 +0200
Subject: [PATCH 1/3] Revert "ASoC: es8316: Enable support for MCLK div by 2"

This reverts commit 869f30782cdad0a86598a700a864e4a2bf44f8cc.
---
 sound/soc/codecs/es8316.c | 45 ++++++++++-----------------------------
 sound/soc/codecs/es8316.h |  3 ---
 2 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index e53b2856d625..a1c3e10c3cf1 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -469,42 +469,19 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream *substream,
 	u8 bclk_divider;
 	u16 lrck_divider;
 	int i;
-	unsigned int clk = es8316->sysclk / 2;
-	bool clk_valid = false;
-
-	/* We will start with halved sysclk and see if we can use it
-	 * for proper clocking. This is to minimise the risk of running
-	 * the CODEC with a too high frequency. We have an SKU where
-	 * the sysclk frequency is 48Mhz and this causes the sound to be
-	 * sped up. If we can run with a halved sysclk, we will use it,
-	 * if we can't use it, then full sysclk will be used.
-	 */
-	do {
-		/* Validate supported sample rates that are autodetected from MCLK */
-		for (i = 0; i < ARRAY_SIZE(supported_mclk_lrck_ratios); i++) {
-			const unsigned int ratio = supported_mclk_lrck_ratios[i];
-
-			if (clk % ratio != 0)
-				continue;
-			if (clk / ratio == params_rate(params))
-				break;
-		}
-		if (i == ARRAY_SIZE(supported_mclk_lrck_ratios)) {
-			if (clk == es8316->sysclk)
-				return -EINVAL;
-			clk = es8316->sysclk;
-		} else {
-			clk_valid = true;
-		}
-	} while (!clk_valid);
 
-	if (clk != es8316->sysclk) {
-		snd_soc_component_update_bits(component, ES8316_CLKMGR_CLKSW,
-					      ES8316_CLKMGR_CLKSW_MCLK_DIV,
-					      ES8316_CLKMGR_CLKSW_MCLK_DIV);
-	}
+	/* Validate supported sample rates that are autodetected from MCLK */
+	for (i = 0; i < ARRAY_SIZE(supported_mclk_lrck_ratios); i++) {
+		const unsigned int ratio = supported_mclk_lrck_ratios[i];
 
-	lrck_divider = clk / params_rate(params);
+		if (es8316->sysclk % ratio != 0)
+			continue;
+		if (es8316->sysclk / ratio == params_rate(params))
+			break;
+	}
+	if (i == ARRAY_SIZE(supported_mclk_lrck_ratios))
+		return -EINVAL;
+	lrck_divider = es8316->sysclk / params_rate(params);
 	bclk_divider = lrck_divider / 4;
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S16_LE:
diff --git a/sound/soc/codecs/es8316.h b/sound/soc/codecs/es8316.h
index 0ff16f948690..c335138e2837 100644
--- a/sound/soc/codecs/es8316.h
+++ b/sound/soc/codecs/es8316.h
@@ -129,7 +129,4 @@
 #define ES8316_GPIO_FLAG_GM_NOT_SHORTED		0x02
 #define ES8316_GPIO_FLAG_HP_NOT_INSERTED	0x04
 
-/* ES8316_CLKMGR_CLKSW */
-#define ES8316_CLKMGR_CLKSW_MCLK_DIV	0x80
-
 #endif
-- 
2.43.0

>From c309d8cf7e3c192683beacb3781458a2f8bfef81 Mon Sep 17 00:00:00 2001
From: Diederik de Haas <didi.debian@cknow.org>
Date: Tue, 16 Apr 2024 10:24:55 +0200
Subject: [PATCH 2/3] Revert "ASoC: es8316: Replace
 NR_SUPPORTED_MCLK_LRCK_RATIOS with ARRAY_SIZE()"

This reverts commit a43c0dc1004cbe2edbae9b6e6793db71f6896449.
---
 sound/soc/codecs/es8316.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index a1c3e10c3cf1..09fc0b25f600 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -27,6 +27,7 @@
  * MCLK/LRCK ratios, but we also add ratio 400, which is commonly used on
  * Intel Cherry Trail platforms (19.2MHz MCLK, 48kHz LRCK).
  */
+#define NR_SUPPORTED_MCLK_LRCK_RATIOS ARRAY_SIZE(supported_mclk_lrck_ratios)
 static const unsigned int supported_mclk_lrck_ratios[] = {
 	256, 384, 400, 500, 512, 768, 1024
 };
@@ -39,7 +40,7 @@ struct es8316_priv {
 	struct snd_soc_jack *jack;
 	int irq;
 	unsigned int sysclk;
-	unsigned int allowed_rates[ARRAY_SIZE(supported_mclk_lrck_ratios)];
+	unsigned int allowed_rates[NR_SUPPORTED_MCLK_LRCK_RATIOS];
 	struct snd_pcm_hw_constraint_list sysclk_constraints;
 	bool jd_inverted;
 };
@@ -381,7 +382,7 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	/* Limit supported sample rates to ones that can be autodetected
 	 * by the codec running in slave mode.
 	 */
-	for (i = 0; i < ARRAY_SIZE(supported_mclk_lrck_ratios); i++) {
+	for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
 		const unsigned int ratio = supported_mclk_lrck_ratios[i];
 
 		if (freq % ratio == 0)
@@ -471,7 +472,7 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream *substream,
 	int i;
 
 	/* Validate supported sample rates that are autodetected from MCLK */
-	for (i = 0; i < ARRAY_SIZE(supported_mclk_lrck_ratios); i++) {
+	for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
 		const unsigned int ratio = supported_mclk_lrck_ratios[i];
 
 		if (es8316->sysclk % ratio != 0)
@@ -479,7 +480,7 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream *substream,
 		if (es8316->sysclk / ratio == params_rate(params))
 			break;
 	}
-	if (i == ARRAY_SIZE(supported_mclk_lrck_ratios))
+	if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS)
 		return -EINVAL;
 	lrck_divider = es8316->sysclk / params_rate(params);
 	bclk_divider = lrck_divider / 4;
-- 
2.43.0

>From 041909b470536bf5acd00dba5fe8d19e2da9bef9 Mon Sep 17 00:00:00 2001
From: Diederik de Haas <didi.debian@cknow.org>
Date: Tue, 16 Apr 2024 10:25:08 +0200
Subject: [PATCH 3/3] Revert "ASoC: es8316: Enable support for S32 LE format"

This reverts commit 2f06f231f0bfe74711fee14e28a8789a3de9bc36.
---
 sound/soc/codecs/es8316.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 09fc0b25f600..a8f347f1affb 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -526,7 +526,7 @@ static int es8316_mute(struct snd_soc_dai *dai, int mute, int direction)
 }
 
 #define ES8316_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
-			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
+			SNDRV_PCM_FMTBIT_S24_LE)
 
 static const struct snd_soc_dai_ops es8316_ops = {
 	.startup = es8316_pcm_startup,
-- 
2.43.0

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: