[PATCH v2 2/2] ASoC: adau1372: Fix clock leak on PLL lock failure

Jihed Chaibi posted 2 patches 1 week, 1 day ago
[PATCH v2 2/2] ASoC: adau1372: Fix clock leak on PLL lock failure
Posted by Jihed Chaibi 1 week, 1 day ago
adau1372_enable_pll() was a void function that logged a dev_err() on
PLL lock timeout but did not propagate the error. As a result,
adau1372_set_power() would continue with adau1372->enabled set to true
despite the PLL being unlocked, and the mclk left enabled with no
corresponding disable on the error path.

Convert adau1372_enable_pll() to return int, using -ETIMEDOUT on lock
timeout and propagating regmap errors directly. In adau1372_set_power(),
check the return value and unwind in reverse order: restore regcache to
cache-only mode, reassert GPIO power-down, and disable the clock before
returning the error.

Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
---
Changes in v2:
- Also unwind regcache and GPIO power-down state on PLL lock failure,
  as noted by Mark Brown.

 sound/soc/codecs/adau1372.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/adau1372.c b/sound/soc/codecs/adau1372.c
index 6345342218d6..e3eba241bdf7 100644
--- a/sound/soc/codecs/adau1372.c
+++ b/sound/soc/codecs/adau1372.c
@@ -762,7 +762,7 @@ static int adau1372_startup(struct snd_pcm_substream *substream, struct snd_soc_
 	return 0;
 }

-static void adau1372_enable_pll(struct adau1372 *adau1372)
+static int adau1372_enable_pll(struct adau1372 *adau1372)
 {
 	unsigned int val, timeout = 0;
 	int ret;
@@ -778,8 +778,12 @@ static void adau1372_enable_pll(struct adau1372 *adau1372)
 		timeout++;
 	} while (!(val & 1) && timeout < 3);

-	if (ret < 0 || !(val & 1))
+	if (ret < 0 || !(val & 1)) {
 		dev_err(adau1372->dev, "Failed to lock PLL\n");
+		return ret < 0 ? ret : -ETIMEDOUT;
+	}
+
+	return 0;
 }

 static int adau1372_set_power(struct adau1372 *adau1372, bool enable)
@@ -807,7 +811,14 @@ static int adau1372_set_power(struct adau1372 *adau1372, bool enable)
 		 * accessed.
 		 */
 		if (adau1372->use_pll) {
-			adau1372_enable_pll(adau1372);
+			ret = adau1372_enable_pll(adau1372);
+			if (ret) {
+				regcache_cache_only(adau1372->regmap, true);
+				if (adau1372->pd_gpio)
+					gpiod_set_value(adau1372->pd_gpio, 1);
+				clk_disable_unprepare(adau1372->mclk);
+				return ret;
+			}
 			clk_ctrl |= ADAU1372_CLK_CTRL_CLKSRC;
 		}

--
2.47.3
Re: [PATCH v2 2/2] ASoC: adau1372: Fix clock leak on PLL lock failure
Posted by Nuno Sá 1 week ago
On Wed, 2026-03-25 at 22:07 +0100, Jihed Chaibi wrote:
> adau1372_enable_pll() was a void function that logged a dev_err() on
> PLL lock timeout but did not propagate the error. As a result,
> adau1372_set_power() would continue with adau1372->enabled set to true
> despite the PLL being unlocked, and the mclk left enabled with no
> corresponding disable on the error path.
> 
> Convert adau1372_enable_pll() to return int, using -ETIMEDOUT on lock
> timeout and propagating regmap errors directly. In adau1372_set_power(),
> check the return value and unwind in reverse order: restore regcache to
> cache-only mode, reassert GPIO power-down, and disable the clock before
> returning the error.
> 
> Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
> ---

Also deserves a Fixes tag IMO.

> Changes in v2:
> - Also unwind regcache and GPIO power-down state on PLL lock failure,
>   as noted by Mark Brown.
> 
>  sound/soc/codecs/adau1372.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/codecs/adau1372.c b/sound/soc/codecs/adau1372.c
> index 6345342218d6..e3eba241bdf7 100644
> --- a/sound/soc/codecs/adau1372.c
> +++ b/sound/soc/codecs/adau1372.c
> @@ -762,7 +762,7 @@ static int adau1372_startup(struct snd_pcm_substream *substream, struct
> snd_soc_
>  	return 0;
>  }
> 
> -static void adau1372_enable_pll(struct adau1372 *adau1372)
> +static int adau1372_enable_pll(struct adau1372 *adau1372)
>  {
>  	unsigned int val, timeout = 0;
>  	int ret;
> @@ -778,8 +778,12 @@ static void adau1372_enable_pll(struct adau1372 *adau1372)
>  		timeout++;
>  	} while (!(val & 1) && timeout < 3);
> 
> -	if (ret < 0 || !(val & 1))
> +	if (ret < 0 || !(val & 1)) {
>  		dev_err(adau1372->dev, "Failed to lock PLL\n");
> +		return ret < 0 ? ret : -ETIMEDOUT;
> +	}
> +
> +	return 0;
>  }
> 
>  static int adau1372_set_power(struct adau1372 *adau1372, bool enable)
> @@ -807,7 +811,14 @@ static int adau1372_set_power(struct adau1372 *adau1372, bool enable)
>  		 * accessed.
>  		 */
>  		if (adau1372->use_pll) {
> -			adau1372_enable_pll(adau1372);
> +			ret = adau1372_enable_pll(adau1372);
> +			if (ret) {
> +				regcache_cache_only(adau1372->regmap, true);
> +				if (adau1372->pd_gpio)
> +					gpiod_set_value(adau1372->pd_gpio, 1);
> +				clk_disable_unprepare(adau1372->mclk);
> +				return ret;
> +			}
>  			clk_ctrl |= ADAU1372_CLK_CTRL_CLKSRC;
>  		}
> 
> --
> 2.47.3