Re: [PATCH 2/8] pwm: mxs: Return early when disabling and fix error-path clock leak

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Nov 05, 2026 at 02:38:04PM +0000, Lee Jones wrote:
> When disabling a PWM channel (!state->enabled), the PWM core permits
> state->period to be zero.  mxs_pwm_apply() currently falls through into
> the period and duty-cycle calculations even when !state->enabled,
> causing do_div(c, state->period) to divide by zero when state->period is
> zero.  In addition, if the requested period exceeds the maximum divider
> (div >= PERIOD_CDIV_MAX) while the PWM channel is currently disabled,
> mxs_pwm_apply() returns -EINVAL without disabling and unpreparing
> mxs->clk.
>
> Handle the !state->enabled path upfront and return early, and balance
> clk_prepare_enable() on the -EINVAL error path.
>
> Fixes: bf29c2ff82fd ("pwm: mxs: Implement ->apply()")
> Closes: https://sashiko.dev/#/bug/linux-710f863f-6964-4e4f-990a-e5043dbc1a87
> Cc: stable@vger.kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> --

Reviewed-by: Frank Li <Frank.Li@nxp.com>


>  drivers/pwm/pwm-mxs.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
> index e1a3df6549fc..16233764fd3c 100644
> --- a/drivers/pwm/pwm-mxs.c
> +++ b/drivers/pwm/pwm-mxs.c
> @@ -56,6 +56,14 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	unsigned long long c;
>  	unsigned int pol_bits;
>
> +	if (!state->enabled) {
> +		if (pwm_is_enabled(pwm)) {
> +			writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + CLR);
> +			clk_disable_unprepare(mxs->clk);
> +		}
> +		return 0;
> +	}
> +
>  	/*
>  	 * If the PWM channel is disabled, make sure to turn on the
>  	 * clock before calling clk_get_rate() and writing to the
> @@ -67,9 +75,6 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  			return ret;
>  	}
>
> -	if (!state->enabled && pwm_is_enabled(pwm))
> -		writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + CLR);
> -
>  	rate = clk_get_rate(mxs->clk);
>  	while (1) {
>  		c = rate >> cdiv_shift[div];
> @@ -78,8 +83,11 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  		if (c < PERIOD_PERIOD_MAX)
>  			break;
>  		div++;
> -		if (div >= PERIOD_CDIV_MAX)
> +		if (div >= PERIOD_CDIV_MAX) {
> +			if (!pwm_is_enabled(pwm))
> +				clk_disable_unprepare(mxs->clk);
>  			return -EINVAL;
> +		}
>  	}
>
>  	period_cycles = c;
> @@ -101,16 +109,12 @@ static int mxs_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	writel(PERIOD_PERIOD(period_cycles) | pol_bits | PERIOD_CDIV(div),
>  	       mxs->base + PWM_PERIOD0 + pwm->hwpwm * 0x20);
>
> -	if (state->enabled) {
> -		if (!pwm_is_enabled(pwm)) {
> -			/*
> -			 * The clock was enabled above. Just enable
> -			 * the channel in the control register.
> -			 */
> -			writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + SET);
> -		}
> -	} else {
> -		clk_disable_unprepare(mxs->clk);
> +	if (!pwm_is_enabled(pwm)) {
> +		/*
> +		 * The clock was enabled above. Just enable
> +		 * the channel in the control register.
> +		 */
> +		writel(1 << pwm->hwpwm, mxs->base + PWM_CTRL + SET);
>  	}
>  	return 0;
>  }
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> --
> 2.56.0.398.g882cbad275-goog
>
>