[PATCH] pwm: fsl-ftm: Unwind resume failures

[PATCH] pwm: fsl-ftm: Unwind resume failures

[Pengpeng Hou]

fsl_pwm_resume() ignores all clock-enable and register-cache replay
failures.  A failure can leave a prefix of requested PWM channels
holding clocks while runtime PM reports the chip resumed.

Check each acquisition and regcache_sync(), then unwind completed
channels in the reverse of the suspend order.  Restore cache-only and
dirty state when the register replay fails.

The issue was identified via static analysis and manually reviewed.

Fixes: 97d0b42e39a7 ("pwm: ftm: Add Power Management support for FTM PWM")

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/pwm/pwm-fsl-ftm.c | 44 +++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index 35406b2e1925..7a61ccb75963 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -501,7 +501,7 @@ static int fsl_pwm_resume(struct device *dev)
 {
 	struct pwm_chip *chip = dev_get_drvdata(dev);
 	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
-	int i;
+	int i, ret;
 
 	for (i = 0; i < chip->npwm; i++) {
 		struct pwm_device *pwm = &chip->pwms[i];
@@ -509,20 +509,52 @@ static int fsl_pwm_resume(struct device *dev)
 		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
 			continue;
 
-		clk_prepare_enable(fpc->ipg_clk);
+		ret = clk_prepare_enable(fpc->ipg_clk);
+		if (ret)
+			goto unwind;
 
 		if (!pwm_is_enabled(pwm))
 			continue;
 
-		clk_prepare_enable(fpc->clk[fpc->period.clk_select]);
-		clk_prepare_enable(fpc->clk[FSL_PWM_CLK_CNTEN]);
+		ret = clk_prepare_enable(fpc->clk[fpc->period.clk_select]);
+		if (ret) {
+			clk_disable_unprepare(fpc->ipg_clk);
+			goto unwind;
+		}
+
+		ret = clk_prepare_enable(fpc->clk[FSL_PWM_CLK_CNTEN]);
+		if (ret) {
+			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
+			clk_disable_unprepare(fpc->ipg_clk);
+			goto unwind;
+		}
 	}
 
 	/* restore all registers from cache */
 	regcache_cache_only(fpc->regmap, false);
-	regcache_sync(fpc->regmap);
+	ret = regcache_sync(fpc->regmap);
+	if (!ret)
+		return 0;
 
-	return 0;
+	regcache_cache_only(fpc->regmap, true);
+	regcache_mark_dirty(fpc->regmap);
+
+unwind:
+	while (i--) {
+		struct pwm_device *pwm = &chip->pwms[i];
+
+		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
+			continue;
+
+		if (pwm_is_enabled(pwm)) {
+			clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_CNTEN]);
+			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
+		}
+
+		clk_disable_unprepare(fpc->ipg_clk);
+	}
+
+	return ret;
 }
 #endif
 
-- 
2.43.0

Re: [PATCH] pwm: fsl-ftm: Unwind resume failures

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3008 bytes --]

On Fri, Aug 28, 2026 at 05:20:14PM +0800, Pengpeng Hou wrote:
> fsl_pwm_resume() ignores all clock-enable and register-cache replay
> failures.  A failure can leave a prefix of requested PWM channels
> holding clocks while runtime PM reports the chip resumed.
> 
> Check each acquisition and regcache_sync(), then unwind completed
> channels in the reverse of the suspend order.  Restore cache-only and
> dirty state when the register replay fails.
> 
> The issue was identified via static analysis and manually reviewed.
> 
> Fixes: 97d0b42e39a7 ("pwm: ftm: Add Power Management support for FTM PWM")
> 

This empty line should be dropped.

> Assisted-by: LLM
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/pwm/pwm-fsl-ftm.c | 44 +++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
> index 35406b2e1925..7a61ccb75963 100644
> --- a/drivers/pwm/pwm-fsl-ftm.c
> +++ b/drivers/pwm/pwm-fsl-ftm.c
> @@ -501,7 +501,7 @@ static int fsl_pwm_resume(struct device *dev)
>  {
>  	struct pwm_chip *chip = dev_get_drvdata(dev);
>  	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> -	int i;
> +	int i, ret;
>  
>  	for (i = 0; i < chip->npwm; i++) {
>  		struct pwm_device *pwm = &chip->pwms[i];
> @@ -509,20 +509,52 @@ static int fsl_pwm_resume(struct device *dev)
>  		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
>  			continue;
>  
> -		clk_prepare_enable(fpc->ipg_clk);
> +		ret = clk_prepare_enable(fpc->ipg_clk);
> +		if (ret)
> +			goto unwind;
>  
>  		if (!pwm_is_enabled(pwm))
>  			continue;
>  
> -		clk_prepare_enable(fpc->clk[fpc->period.clk_select]);
> -		clk_prepare_enable(fpc->clk[FSL_PWM_CLK_CNTEN]);
> +		ret = clk_prepare_enable(fpc->clk[fpc->period.clk_select]);
> +		if (ret) {
> +			clk_disable_unprepare(fpc->ipg_clk);
> +			goto unwind;
> +		}
> +
> +		ret = clk_prepare_enable(fpc->clk[FSL_PWM_CLK_CNTEN]);
> +		if (ret) {
> +			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
> +			clk_disable_unprepare(fpc->ipg_clk);
> +			goto unwind;
> +		}
>  	}
>  
>  	/* restore all registers from cache */
>  	regcache_cache_only(fpc->regmap, false);
> -	regcache_sync(fpc->regmap);
> +	ret = regcache_sync(fpc->regmap);
> +	if (!ret)
> +		return 0;

I think this is semantically right, but still surprising as it looks
like error handling.

>  
> -	return 0;
> +	regcache_cache_only(fpc->regmap, true);
> +	regcache_mark_dirty(fpc->regmap);
> +
> +unwind:
> +	while (i--) {
> +		struct pwm_device *pwm = &chip->pwms[i];
> +
> +		if (!test_bit(PWMF_REQUESTED, &pwm->flags))
> +			continue;
> +
> +		if (pwm_is_enabled(pwm)) {
> +			clk_disable_unprepare(fpc->clk[FSL_PWM_CLK_CNTEN]);
> +			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
> +		}
> +
> +		clk_disable_unprepare(fpc->ipg_clk);
> +	}
> +
> +	return ret;
>  }
>  #endif

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]