Re: [PATCH] ASoC: cs42l43: leak MCLK when PLL enable fails on PRE_PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: cs42l43: double clk_disable_unprepare() in cs42l43_pll_ev()
--

commit 22be724b77049d9bdd49902edcd96899ee4749da
Author: Tang Bin <tangbin@cmss.chinamobile.com>

ASoC: cs42l43: leak MCLK when PLL enable fails on PRE_PMU

This commit disables and unprepares MCLK in cs42l43_pll_ev() if
cs42l43_enable_pll() fails during SND_SOC_DAPM_PRE_PMU.

> diff --git a/sound/soc/codecs/cs42l43.c b/sound/soc/codecs/cs42l43.c
> index b6238f89c789c..6f308ea227fb6 100644
> --- a/sound/soc/codecs/cs42l43.c
> +++ b/sound/soc/codecs/cs42l43.c
> @@ -1625,6 +1625,8 @@ static int cs42l43_pll_ev(struct snd_soc_dapm_widget *w,
>  		}
>  
>  		ret = cs42l43_enable_pll(priv);
> +		if (ret && priv->refclk_src == CS42L43_SYSCLK_MCLK)
> +			clk_disable_unprepare(priv->mclk);

[Severity: High]
Does this cause a double clk_disable_unprepare(priv->mclk) when the widget
later powers down?

In sound/soc/soc-dapm.c, dapm_seq_run_coalesced() sets w->power before
running SND_SOC_DAPM_PRE_PMU:

sound/soc/soc-dapm.c:dapm_seq_run_coalesced() {
	list_for_each_entry(w, pending, power_list) {
		WARN_ON(reg != w->reg || dapm != w->dapm);
		w->power = w->new_power;
		...
		/* Check for events */
		dapm_seq_check_event(card, w, SND_SOC_DAPM_PRE_PMU);

And dapm_seq_check_event() only logs an error when w->event() returns a
negative value, without rolling back w->power:

sound/soc/soc-dapm.c:dapm_seq_check_event() {
		ret = w->event(w, NULL, event);
		trace_snd_soc_dapm_widget_event_done(w, event);
		if (ret < 0)
			dev_err(dev, "ASoC: %s: %s event failed: %d\n",
			       ev_name, w->name, ret);

Because w->power remains 1 even when cs42l43_enable_pll() fails, DAPM will
still run the SND_SOC_DAPM_POST_PMD event when the audio path powers down:

sound/soc/codecs/cs42l43.c:cs42l43_pll_ev() {
	case SND_SOC_DAPM_POST_PMD:
		ret = cs42l43_disable_pll(priv);

		if (priv->refclk_src == CS42L43_SYSCLK_MCLK)
			clk_disable_unprepare(priv->mclk);
		break;

Wouldn't disabling priv->mclk in the SND_SOC_DAPM_PRE_PMU error path cause
SND_SOC_DAPM_POST_PMD to call clk_disable_unprepare(priv->mclk) a second
time and underflow the clock enable and prepare reference counts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104080257.1144-1-tangbin@cmss.chinamobile.com?part=1