Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH RFC] ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec
@ 2026-11-09 22:33 Gaurav Suman via B4 Relay
  2026-11-09 22:53 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Gaurav Suman via B4 Relay @ 2026-11-09 22:33 UTC (permalink / raw)
  To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai
  Cc: linux-sound, linux-arm-msm, linux-kernel, Gaurav Suman

From: Gaurav Suman <gauravsuman007@gmail.com>

On an LG G8x (SM8150, WCD9340 over SLIMbus) the microphones record
digital silence on roughly half of all boots. Register dumps look
correct because they are served from the regmap cache, but the
writes never reached the hardware: the WCD934x digital core ignores
register writes while its MCLK/sysclk is off.

wcd934x_comp_init() runs with MCLK off unless the SoundWire master
happened to probe first and enabled the clock through
wcd934x_swrm_clock(); __wcd934x_cdc_mclk_enable(false) then leaves it
running because the SWR clock bit is set. Probe ordering is not
deterministic, which matches the random good and bad boots.

Enable MCLK before wcd934x_comp_init() and leave it enabled, which is
what the good boots already do. With this change 8 of 8 boots had a
working microphone (before: about 3 in 5 silent).

Enabling MCLK only around wcd934x_comp_init() (and disabling it
again afterwards, as wcd934x_enable_efuse_sensing() does) was not
enough: 2 of 4 boots were silent again, so later writes made while
the clock is off are lost as well. An alternative would be to mark
the cache dirty and resync whenever MCLK goes from off to on; this
patch takes the simpler route at the cost of keeping MCLK running.

The fault was found, the change written and the boot tests run with the
help of an AI coding assistant (Claude Opus 5.5, Anthropic); I reviewed
the change and the test results.

Signed-off-by: Gaurav Suman <gauravsuman007@gmail.com>
---
Tested on LG G8x ThinQ (SM8150).
---
 sound/soc/codecs/wcd934x.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
index 0ed24cca088f839263f015778e8281027392e51f..6b6ce5ba5d30da194db1b1570a58f0324956f96f 100644
--- a/sound/soc/codecs/wcd934x.c
+++ b/sound/soc/codecs/wcd934x.c
@@ -2995,6 +2995,12 @@ static int wcd934x_comp_probe(struct snd_soc_component *component)
 	/* Default HPH Mode to Class-H Low HiFi */
 	wcd->hph_mode = CLS_H_LOHIFI;
 
+	/*
+	 * The digital core drops register writes while MCLK is off, but the
+	 * regmap cache still records them. Keep MCLK running so the init
+	 * sequence below and later mixer/DAPM writes actually reach the codec.
+	 */
+	__wcd934x_cdc_mclk_enable(wcd, true);
 	wcd934x_comp_init(component);
 
 	for (i = 0; i < NUM_CODEC_DAIS; i++)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b
change-id: 20261109-wcd934x-mclk-de59e1646807

Best regards,
-- 
Gaurav Suman <gauravsuman007@gmail.com>



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH RFC] ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec
  2026-11-09 22:33 [PATCH RFC] ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec Gaurav Suman via B4 Relay
@ 2026-11-09 22:53 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-09 22:53 UTC (permalink / raw)
  To: Gaurav Suman; +Cc: linux-sound

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] ASoC: wcd934x: unchecked, leaked MCLK in wcd934x_comp_probe()
  and remove()
- [Medium] ASoC: wcd934x: always-on MCLK in wcd934x_comp_probe() breaks
  clock gating
- [Low] ASoC: wcd934x: missing Fixes trailer on wcd934x_comp_probe()
  MCLK fix
--

commit 4995c3fc995e247626bf6a941f670d9b5cfc1f8c
Author: Gaurav Suman <gauravsuman007@gmail.com>

> ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec
>
> On an LG G8x (SM8150, WCD9340 over SLIMbus) the microphones record
> digital silence on roughly half of all boots. Register dumps look
> correct because they are served from the regmap cache, but the
> writes never reached the hardware: the WCD934x digital core ignores
> register writes while its MCLK/sysclk is off.
>
> wcd934x_comp_init() runs with MCLK off unless the SoundWire master
> happened to probe first and enabled the clock through
> wcd934x_swrm_clock(); __wcd934x_cdc_mclk_enable(false) then leaves it
> running because the SWR clock bit is set. Probe ordering is not
> deterministic, which matches the random good and bad boots.
>
> Enable MCLK before wcd934x_comp_init() and leave it enabled, which is
> what the good boots already do. With this change 8 of 8 boots had a
> working microphone (before: about 3 in 5 silent).
>
> Enabling MCLK only around wcd934x_comp_init() (and disabling it
> again afterwards, as wcd934x_enable_efuse_sensing() does) was not
> enough: 2 of 4 boots were silent again, so later writes made while
> the clock is off are lost as well. An alternative would be to mark
> the cache dirty and resync whenever MCLK goes from off to on; this
> patch takes the simpler route at the cost of keeping MCLK running.
>
> The fault was found, the change written and the boot tests run with the
> help of an AI coding assistant (Claude Opus 5.5, Anthropic); I reviewed
> the change and the test results.
>
> Signed-off-by: Gaurav Suman <gauravsuman007@gmail.com>

[Severity: Low]
Should this bug fix include a Fixes: tag?

  Fixes: a61f3b4f476e ("ASoC: wcd934x: add support to wcd9340/wcd9341 codec")

> diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
> index 0ed24cca088f8..6b6ce5ba5d30d 100644
> --- a/sound/soc/codecs/wcd934x.c
> +++ b/sound/soc/codecs/wcd934x.c
> @@ -2995,6 +2995,12 @@ static int wcd934x_comp_probe(struct snd_soc_component *component)
>  	/* Default HPH Mode to Class-H Low HiFi */
>  	wcd->hph_mode = CLS_H_LOHIFI;
>  
> +	/*
> +	 * The digital core drops register writes while MCLK is off, but the
> +	 * regmap cache still records them. Keep MCLK running so the init
> +	 * sequence below and later mixer/DAPM writes actually reach the codec.
> +	 */
> +	__wcd934x_cdc_mclk_enable(wcd, true);
>  	wcd934x_comp_init(component);
>  
>  	for (i = 0; i < NUM_CODEC_DAIS; i++)

[Severity: Medium]
Does this leak wcd->extclk and wcd->sysclk_users when wcd934x_init_dmic()
fails or when wcd934x_comp_remove() runs?

__wcd934x_cdc_mclk_enable(wcd, true) prepares and enables wcd->extclk (which
is acquired via devm_clk_get() in the parent MFD driver) and increments
wcd->sysclk_users, but neither the wcd934x_init_dmic() error return in
wcd934x_comp_probe() nor wcd934x_comp_remove() calls
__wcd934x_cdc_mclk_enable(wcd, false).  Each component probe deferral or
card unbind leaks a clock prepare/enable count and sysclk_users reference,
and unbinding the parent MFD device triggers a clock framework WARN_ON for
freeing a prepared and enabled clock.

In addition, the return value of __wcd934x_cdc_mclk_enable(wcd, true) is
not checked, so if clk_prepare_enable(wcd->extclk) fails,
wcd934x_comp_init() still runs with MCLK disabled.

[Severity: Medium]
Does holding an unreleased __wcd934x_cdc_mclk_enable(wcd, true) reference
from wcd934x_comp_probe() prevent the driver from ever gating MCLK or
analog bias?

Because wcd->sysclk_users stays >= 1 for the lifetime of the component,
wcd934x_disable_ana_bias_and_syclk() can never reach zero when the DAPM
MCLK supply widget (wcd934x_codec_enable_mclk()), wcd934x_swrm_clock(), or
wcd934x_mbhc_request_micbias() disable MCLK:

sound/soc/codecs/wcd934x.c:wcd934x_disable_ana_bias_and_syclk() {
	scoped_guard(mutex, &wcd->sysclk_mutex) {
		if (--wcd->sysclk_users != 0)
			return 0;
	}
    ...
}

This keeps WCD934X_CLK_SYS_MCLK_PRG, WCD934X_ANA_BIAS, and wcd->extclk
active across audio idle and system suspend.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-wcd934x-mclk-v1-1-ab86342e7ead@gmail.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-09 22:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 22:33 [PATCH RFC] ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec Gaurav Suman via B4 Relay
2026-11-09 22:53 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox