Re: [PATCH RFC] ASoC: codecs: wcd934x: keep MCLK enabled so init writes reach the codec

[sashiko-bot@kernel.org]

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