[PATCH v4 2/4] ASoC: Intel: bytcr_rt5651: Fix MCLK leak on platform_clock_control error

aravindanilraj0702@gmail.com posted 4 patches 5 days, 8 hours ago
[PATCH v4 2/4] ASoC: Intel: bytcr_rt5651: Fix MCLK leak on platform_clock_control error
Posted by aravindanilraj0702@gmail.com 5 days, 8 hours ago
From: Aravind Anilraj <aravindanilraj0702@gmail.com>

If byt_rt5651_prepare_and_enable_pll1() fails, the function returns
without calling clk_disable_unprepare() on priv->mclk, which was
already enabled earlier in the same code path. Add the missing
clk_disable_unprepare() call before returning the error.

Signed-off-by: Aravind Anilraj <aravindanilraj0702@gmail.com>
---
 sound/soc/intel/boards/bytcr_rt5651.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 68cf463f1d50..65944eb4be16 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -205,7 +205,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
 		ret = clk_prepare_enable(priv->mclk);
 		if (ret < 0) {
-			dev_err(card->dev, "could not configure MCLK state");
+			dev_err(card->dev, "could not configure MCLK state\n");
 			return ret;
 		}
 		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
@@ -224,6 +224,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 
 	if (ret < 0) {
 		dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+		if (SND_SOC_DAPM_EVENT_ON(event))
+			clk_disable_unprepare(priv->mclk);
 		return ret;
 	}
 
-- 
2.47.3
Re: [PATCH v4 2/4] ASoC: Intel: bytcr_rt5651: Fix MCLK leak on platform_clock_control error
Posted by Mark Brown 2 days, 21 hours ago
On Sat, Mar 28, 2026 at 01:25:53AM -0400, aravindanilraj0702@gmail.com wrote:

> If byt_rt5651_prepare_and_enable_pll1() fails, the function returns
> without calling clk_disable_unprepare() on priv->mclk, which was
> already enabled earlier in the same code path. Add the missing
> clk_disable_unprepare() call before returning the error.

>  	if (SND_SOC_DAPM_EVENT_ON(event)) {
>  		ret = clk_prepare_enable(priv->mclk);
>  		if (ret < 0) {
> -			dev_err(card->dev, "could not configure MCLK state");
> +			dev_err(card->dev, "could not configure MCLK state\n");
>  			return ret;
>  		}
>  		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);

As I said on prior versions this has an unrelated change in this print.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.
Re: [PATCH v4 2/4] ASoC: Intel: bytcr_rt5651: Fix MCLK leak on platform_clock_control error
Posted by Cezary Rojewski 3 days, 5 hours ago
On 2026-03-28 6:25 AM, aravindanilraj0702@gmail.com wrote:
> From: Aravind Anilraj <aravindanilraj0702@gmail.com>
> 
> If byt_rt5651_prepare_and_enable_pll1() fails, the function returns
> without calling clk_disable_unprepare() on priv->mclk, which was
> already enabled earlier in the same code path. Add the missing
> clk_disable_unprepare() call before returning the error.
> 
> Signed-off-by: Aravind Anilraj <aravindanilraj0702@gmail.com>
> ---
>   sound/soc/intel/boards/bytcr_rt5651.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
> index 68cf463f1d50..65944eb4be16 100644
> --- a/sound/soc/intel/boards/bytcr_rt5651.c
> +++ b/sound/soc/intel/boards/bytcr_rt5651.c
> @@ -205,7 +205,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
>   	if (SND_SOC_DAPM_EVENT_ON(event)) {
>   		ret = clk_prepare_enable(priv->mclk);
>   		if (ret < 0) {
> -			dev_err(card->dev, "could not configure MCLK state");
> +			dev_err(card->dev, "could not configure MCLK state\n");
>   			return ret;
>   		}
>   		ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
> @@ -224,6 +224,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
>   
>   	if (ret < 0) {
>   		dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
> +		if (SND_SOC_DAPM_EVENT_ON(event))
> +			clk_disable_unprepare(priv->mclk);

Same as for bytcr_rt5640:

This if-statement complicates the situation unnecessarily.  Please do
the check in scope of "if (SND_SOC_DAPM_EVENT_ON(event))" found earlier
in this function.

>   		return ret;
>   	}
>