[PATCH] ASoC: wm8962: Do not clear SYSCLK_ENA owned by DAPM in configure_bclk()

[Maxime Douailin]

wm8962_configure_bclk() provisionally sets SYSCLK_ENA so that the
read-only DSPCLK_DIV field becomes valid, then clears it again whenever
the bias level is below SND_SOC_BIAS_ON. It never checks whether the bit
was already set.

It can be: the "SYSCLK" DAPM supply widget owns that bit, and
wm8962_mic_detect() force-enables the pin. On boards that call it (the
Tegra machine driver, i.e. Microsoft Surface RT and Surface 2) DAPM sets
SYSCLK_ENA once at card init and, because the widget then never changes
power state, never writes it again. The first stream's transition to
SND_SOC_BIAS_PREPARE runs configure_bclk(), which clears the bit behind
DAPM's back. From then on every stream plays with SYSCLK off: the DACs
do not run, CP_ENA refuses to latch, hp_event() reports "DC servo timed
out", and the codec is silent although every register DAPM believes in
looks right.

Remember whether SYSCLK_ENA was set on entry and only undo our own
provisional enable.

The bug was located with the help of an LLM coding assistant, by diffing
the driver's regmap cache against raw I2C reads of the chip during
playback; the assistant also drafted this patch and the changelog. The
analysis was confirmed, and the fix written and tested, on real hardware.

Verified on a Microsoft Surface 2 (Tegra 4, WM8962 rev F): with the bit
forced on over I2C during a stream the DC servo completes in ~56 ms and
audio is heard; with this patch applied the DC servo completes on every
stream and there is no more "DC servo timed out".

Fixes: 75704ecfbb41 ("ASoC: wm8962: Enable SYSCLK provisonally before fetching generated DSPCLK_DIV")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Maxime Douailin <maxime.douailin@xxxxxxxxx>
---
Notes:
 - Found and verified on a Microsoft Surface 2 (Tegra 4); the same silence
   is reported on Microsoft Surface RT, whose machine driver is the same.
   Background and the full register-level analysis (regmap cache vs. raw
   I2C reads during playback):
   https://github.com/grate-driver/linux/issues/141
 - Boards that do not call wm8962_mic_detect() are unaffected: there
   SYSCLK_ENA is never already set below SND_SOC_BIAS_ON, sysclk_was_ena
   is false, and the sequence is bit-identical to the current one.
 - The deeper question is whether tegra_wm8962 should be calling
   wm8962_mic_detect() at all on these boards, but the codec should not be
   clearing a bit DAPM owns regardless, so this fix stands on its own.
 - Tooling, per Documentation/process/generated-content.rst: an LLM coding
   assistant did the register-diff analysis that found the bug and drafted
   this patch and changelog, over an interactive session on the affected
   hardware rather than from a single prompt. Every register claim above
   was checked against the chip over I2C, and the fix was built and
   listened to on the machine before sending. I understand the change and
   stand behind it.

 sound/soc/codecs/wm8962.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index af471605122..c314af2c860 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2472,6 +2472,8 @@ static void wm8962_configure_bclk(struct snd_soc_component *component)
 	int clocking2 = 0;
 	int clocking4 = 0;
 	int aif2 = 0;
+	int sysclk;
+	bool sysclk_was_ena;
 
 	if (!wm8962->sysclk_rate) {
 		dev_dbg(component->dev, "No SYSCLK configured\n");
@@ -2504,8 +2506,19 @@ static void wm8962_configure_bclk(struct snd_soc_component *component)
 	/* DSPCLK_DIV can be only generated correctly after enabling SYSCLK.
 	 * So we here provisionally enable it and then disable it afterward
 	 * if current bias_level hasn't reached SND_SOC_BIAS_ON.
+	 *
+	 * SYSCLK_ENA is owned by the "SYSCLK" DAPM supply widget, which may
+	 * already have it set even below SND_SOC_BIAS_ON: wm8962_mic_detect()
+	 * force-enables that pin, so on boards using it (the Tegra machine
+	 * driver) the bit is set once at card init and DAPM never writes it
+	 * again. If we unconditionally clear it here, SYSCLK stays off for
+	 * every stream, DAPM still believes it is on, and the codec is
+	 * silent with "DC servo timed out" errors. Only undo what we did.
 	 */
-	if (snd_soc_dapm_get_bias_level(dapm) != SND_SOC_BIAS_ON)
+	sysclk = snd_soc_component_read(component, WM8962_CLOCKING2);
+	sysclk_was_ena = sysclk >= 0 && (sysclk & WM8962_SYSCLK_ENA);
+	if (!sysclk_was_ena &&
+	    snd_soc_dapm_get_bias_level(dapm) != SND_SOC_BIAS_ON)
 		snd_soc_component_update_bits(component, WM8962_CLOCKING2,
 				WM8962_SYSCLK_ENA_MASK, WM8962_SYSCLK_ENA);
 
@@ -2519,7 +2532,8 @@ static void wm8962_configure_bclk(struct snd_soc_component *component)
 	usleep_range(500, 1000);
 	dspclk = snd_soc_component_read(component, WM8962_CLOCKING1);
 
-	if (snd_soc_dapm_get_bias_level(dapm) != SND_SOC_BIAS_ON)
+	if (!sysclk_was_ena &&
+	    snd_soc_dapm_get_bias_level(dapm) != SND_SOC_BIAS_ON)
 		snd_soc_component_update_bits(component, WM8962_CLOCKING2,
 				WM8962_SYSCLK_ENA_MASK, 0);
 

base-commit: 0e2bf66c2f6326c0626c9db22530954ef5c64b08
-- 
2.53.0