[PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[Wentao Liang]

When the FLL is not already enabled, madera_fllhj_enable() takes a
runtime PM reference and relies on madera_fllhj_disable() to drop it
later. If madera_fllhj_apply() fails, the ENA bit is never set, so a
subsequent disable sees no change and does not drop the reference,
leaking it along with the device's active state.

Drop the reference on the failure path and skip waiting for a lock that
will never be asserted.

Fixes: 6535e831b4c8 ("ASoC: cs47l92: Add codec driver for Cirrus Logic CS47L92")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 sound/soc/codecs/madera.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c
index 58255844b4df..2f6256f8e87b 100644
--- a/sound/soc/codecs/madera.c
+++ b/sound/soc/codecs/madera.c
@@ -4608,8 +4608,17 @@ static int madera_fllhj_enable(struct madera_fll *fll)
 			   MADERA_FLL1_HOLD_MASK,
 			   0);
 
-	if (!already_enabled)
-		madera_wait_for_fll(fll, true);
+	/*
+	 * If configuring the FLL failed it is left disabled, so a later
+	 * madera_fllhj_disable() will not see the ENA bit set and will not
+	 * drop the runtime PM reference taken above.
+	 */
+	if (!already_enabled) {
+		if (ret)
+			pm_runtime_put_autosuspend(madera->dev);
+		else
+			madera_wait_for_fll(fll, true);
+	}
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: madera: broken error handling and return value in madera_fllhj_enable()
--

commit 0527a87963a29c49669e1565c1faf7d3cc1ad756
Author: Wentao Liang <vulab@iscas.ac.cn>

ASoC: codecs: madera: release runtime PM ref on FLL config failure

This commit releases the runtime PM reference and skips waiting for FLL lock
when madera_fllhj_apply() fails in madera_fllhj_enable() while the FLL was
previously disabled.

> diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c
> index 58255844b4df3..2f6256f8e87b2 100644
> --- a/sound/soc/codecs/madera.c
> +++ b/sound/soc/codecs/madera.c
> @@ -4608,8 +4608,17 @@ static int madera_fllhj_enable(struct madera_fll *fll)
>  			   MADERA_FLL1_HOLD_MASK,
>  			   0);
>  
> -	if (!already_enabled)
> -		madera_wait_for_fll(fll, true);
> +	/*
> +	 * If configuring the FLL failed it is left disabled, so a later
> +	 * madera_fllhj_disable() will not see the ENA bit set and will not
> +	 * drop the runtime PM reference taken above.
> +	 */
> +	if (!already_enabled) {
> +		if (ret)
> +			pm_runtime_put_autosuspend(madera->dev);
> +		else
> +			madera_wait_for_fll(fll, true);
> +	}
>  
>  	return 0;
>  }

[Severity: High]
When madera_fllhj_apply() fails in sound/soc/codecs/madera.c, execution
jumps to the out label before calling pm_runtime_put_autosuspend() and
returning:

sound/soc/codecs/madera.c:madera_fllhj_enable() {
	...
	ret = madera_fllhj_apply(fll, fll->ref_freq);
	if (ret) {
		madera_fll_err(fll, "Failed to set FLL: %d\n", ret);
		goto out;
	}
	...
out:
	regmap_update_bits(madera->regmap,
			   fll->base + MADERA_FLL_CONTROL_11_OFFS,
			   MADERA_FLL1_LOCKDET_MASK,
			   MADERA_FLL1_LOCKDET_MASK);

	regmap_update_bits(madera->regmap,
			   fll->base + MADERA_FLL_CONTROL_2_OFFS,
			   MADERA_FLL1_CTRL_UPD_MASK,
			   MADERA_FLL1_CTRL_UPD_MASK);

	/* Release the hold so that flln locks to external frequency */
	regmap_update_bits(madera->regmap,
			   fll->base + MADERA_FLL_CONTROL_1_OFFS,
			   MADERA_FLL1_HOLD_MASK,
			   0);
}

As documented in madera_fllhj_disable(), a disabled FLL requires
MADERA_FLL1_LOCKDET_MASK = 0, MADERA_FLL1_HOLD_MASK = 1, and
MADERA_FLL1_CTRL_UPD_MASK = 0 across runtime suspend and resume so that
regcache_sync() does not latch CTRL_UPD before writing out all FLL
registers:

sound/soc/codecs/madera.c:madera_fllhj_disable() {
	...
	/* ctrl_up gates the writes to all the fll's registers, setting it to 0
	 * here ensures that after a runtime suspend/resume cycle when one
	 * enables the fll then ctrl_up is the last bit that is configured
	 * by the fll enable code rather than the cache sync operation which
	 * would have updated it much earlier before writing out all fll
	 * registers
	 */
	regmap_update_bits(madera->regmap,
			   fll->base + MADERA_FLL_CONTROL_2_OFFS,
			   MADERA_FLL1_CTRL_UPD_MASK, 0);
}

Does dropping the runtime PM reference at the end of madera_fllhj_enable()
allow the device to runtime-suspend while MADERA_FLL1_CTRL_UPD_MASK = 1,
MADERA_FLL1_LOCKDET_MASK = 1, and MADERA_FLL1_HOLD_MASK = 0 remain set in
the regcache for a disabled FLL?

Additionally, madera_fllhj_enable() still returns 0 instead of ret at the
end of the function. Because cs47l92_set_fll() -> madera_fllhj_set_refclk()
stores fll->fout before calling madera_fllhj_enable():

sound/soc/codecs/madera.c:madera_fllhj_set_refclk() {
	...
	fll->ref_src = source;
	fll->ref_freq = fin;
	fll->fout = fout;

	if (fout)
		ret = madera_fllhj_enable(fll);
	else
		madera_fllhj_disable(fll);

	return ret;
}

shouldn't madera_fllhj_enable() avoid setting LOCKDET/CTRL_UPD or clearing
HOLD on error and return ret so the caller does not see success with
fll->fout left non-zero while the FLL hardware is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104022814.1484284-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[Charles Keepax]

On Wed, Nov 04, 2026 at 02:28:14AM +0000, Wentao Liang wrote:
> When the FLL is not already enabled, madera_fllhj_enable() takes a
> runtime PM reference and relies on madera_fllhj_disable() to drop it
> later. If madera_fllhj_apply() fails, the ENA bit is never set, so a
> subsequent disable sees no change and does not drop the reference,
> leaking it along with the device's active state.
> 
> Drop the reference on the failure path and skip waiting for a lock that
> will never be asserted.
> 
> Fixes: 6535e831b4c8 ("ASoC: cs47l92: Add codec driver for Cirrus Logic CS47L92")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[Mark Brown]

On Wed, 04 Nov 2026 02:28:14 +0000, Wentao Liang wrote:
> ASoC: codecs: madera: release runtime PM ref on FLL config failure

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.5

Thanks!

[1/1] ASoC: codecs: madera: release runtime PM ref on FLL config failure
      https://git.kernel.org/broonie/sound/c/1ba6a1d89d11

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] ASoC: codecs: madera: release runtime PM ref on FLL config failure

[Richard Fitzgerald]

On 04/11/2026 2:42 am, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ASoC: madera: broken error handling and return value in madera_fllhj_enable()
> --
> 
> commit 0527a87963a29c49669e1565c1faf7d3cc1ad756
> Author: Wentao Liang <vulab@iscas.ac.cn>
> 
> ASoC: codecs: madera: release runtime PM ref on FLL config failure

Sashiko doesn't send its reports to everyone on the email it is replying
to. So I'm forwarding its findings.

There are some potential issues with register state introduced by this
patch. Can you check this?

> 
> This commit releases the runtime PM reference and skips waiting for FLL lock
> when madera_fllhj_apply() fails in madera_fllhj_enable() while the FLL was
> previously disabled.
> 
>> diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c
>> index 58255844b4df3..2f6256f8e87b2 100644
>> --- a/sound/soc/codecs/madera.c
>> +++ b/sound/soc/codecs/madera.c
>> @@ -4608,8 +4608,17 @@ static int madera_fllhj_enable(struct madera_fll *fll)
>>   			   MADERA_FLL1_HOLD_MASK,
>>   			   0);
>>   
>> -	if (!already_enabled)
>> -		madera_wait_for_fll(fll, true);
>> +	/*
>> +	 * If configuring the FLL failed it is left disabled, so a later
>> +	 * madera_fllhj_disable() will not see the ENA bit set and will not
>> +	 * drop the runtime PM reference taken above.
>> +	 */
>> +	if (!already_enabled) {
>> +		if (ret)
>> +			pm_runtime_put_autosuspend(madera->dev);
>> +		else
>> +			madera_wait_for_fll(fll, true);
>> +	}
>>   
>>   	return 0;
>>   }
> 
> [Severity: High]
> When madera_fllhj_apply() fails in sound/soc/codecs/madera.c, execution
> jumps to the out label before calling pm_runtime_put_autosuspend() and
> returning:
> 
> sound/soc/codecs/madera.c:madera_fllhj_enable() {
> 	...
> 	ret = madera_fllhj_apply(fll, fll->ref_freq);
> 	if (ret) {
> 		madera_fll_err(fll, "Failed to set FLL: %d\n", ret);
> 		goto out;
> 	}
> 	...
> out:
> 	regmap_update_bits(madera->regmap,
> 			   fll->base + MADERA_FLL_CONTROL_11_OFFS,
> 			   MADERA_FLL1_LOCKDET_MASK,
> 			   MADERA_FLL1_LOCKDET_MASK);
> 
> 	regmap_update_bits(madera->regmap,
> 			   fll->base + MADERA_FLL_CONTROL_2_OFFS,
> 			   MADERA_FLL1_CTRL_UPD_MASK,
> 			   MADERA_FLL1_CTRL_UPD_MASK);
> 
> 	/* Release the hold so that flln locks to external frequency */
> 	regmap_update_bits(madera->regmap,
> 			   fll->base + MADERA_FLL_CONTROL_1_OFFS,
> 			   MADERA_FLL1_HOLD_MASK,
> 			   0);
> }
> 
> As documented in madera_fllhj_disable(), a disabled FLL requires
> MADERA_FLL1_LOCKDET_MASK = 0, MADERA_FLL1_HOLD_MASK = 1, and
> MADERA_FLL1_CTRL_UPD_MASK = 0 across runtime suspend and resume so that
> regcache_sync() does not latch CTRL_UPD before writing out all FLL
> registers:
> 
> sound/soc/codecs/madera.c:madera_fllhj_disable() {
> 	...
> 	/* ctrl_up gates the writes to all the fll's registers, setting it to 0
> 	 * here ensures that after a runtime suspend/resume cycle when one
> 	 * enables the fll then ctrl_up is the last bit that is configured
> 	 * by the fll enable code rather than the cache sync operation which
> 	 * would have updated it much earlier before writing out all fll
> 	 * registers
> 	 */
> 	regmap_update_bits(madera->regmap,
> 			   fll->base + MADERA_FLL_CONTROL_2_OFFS,
> 			   MADERA_FLL1_CTRL_UPD_MASK, 0);
> }
> 
> Does dropping the runtime PM reference at the end of madera_fllhj_enable()
> allow the device to runtime-suspend while MADERA_FLL1_CTRL_UPD_MASK = 1,
> MADERA_FLL1_LOCKDET_MASK = 1, and MADERA_FLL1_HOLD_MASK = 0 remain set in
> the regcache for a disabled FLL?
> 
> Additionally, madera_fllhj_enable() still returns 0 instead of ret at the
> end of the function. Because cs47l92_set_fll() -> madera_fllhj_set_refclk()
> stores fll->fout before calling madera_fllhj_enable():
> 
> sound/soc/codecs/madera.c:madera_fllhj_set_refclk() {
> 	...
> 	fll->ref_src = source;
> 	fll->ref_freq = fin;
> 	fll->fout = fout;
> 
> 	if (fout)
> 		ret = madera_fllhj_enable(fll);
> 	else
> 		madera_fllhj_disable(fll);
> 
> 	return ret;
> }
> 
> shouldn't madera_fllhj_enable() avoid setting LOCKDET/CTRL_UPD or clearing
> HOLD on error and return ret so the caller does not see success with
> fll->fout left non-zero while the FLL hardware is disabled?
>