[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <kemjjoyrhqglqq4p2j6kygspevq2mdbiujtnksw4rkdapoqcfy@zte2c7fhqvn3>
Date: Tue, 6 Jan 2026 11:22:57 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Sean Nyekjaer <sean@...nix.com>
Cc: Fabrice Gasnier <fabrice.gasnier@...s.st.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>,
linux-pwm@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] pwm: stm32: handle polarity change when PWM is enabled
Hello Sean,
On Tue, Jan 06, 2026 at 08:01:57AM +0100, Sean Nyekjaer wrote:
> After commit 7346e7a058a2 ("pwm: stm32: Always do lazy disabling"),
> polarity changes are ignored. Updates to the TIMx_CCER CCxP bits are
> ignored by the hardware when the master output is enabled via the
> TIMx_BDTR MOE bit.
>
> Handle polarity changes by temporarily disabling the PWM when required,
> in line with apply() implementations used by other PWM drivers.
>
> Fixes: 7346e7a058a2 ("pwm: stm32: Always do lazy disabling")
> Cc: stable@...r.kernel.org
> Signed-off-by: Sean Nyekjaer <sean@...nix.com>
> ---
> This patch is only applicable for stable tree's <= 6.12
> How to explicitly state that and what is the procedure?
I haven't checked in detail yet but I wonder if the problem also exists
in newer kernels. Also I think that changing the polarity with the
hardware on happend before 7346e7a058a2; in that case you blamed the
wrong commit.
So even if we decide to apply a small targetted fix for the issue you
report to stable without an equivalent commit in mainline (due to the
rework the driver saw in v6.13-rc1~157^2~9^2~3 ("pwm: stm32:
Implementation of the waveform callbacks")), I refuse to do that if the
problem still exists in mainline.
> ---
> drivers/pwm/pwm-stm32.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index eb24054f9729734da21eb96f2e37af03339e3440..d5f79e87a0653e1710d46e6bf9268a59638943fe 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -452,15 +452,23 @@ static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>
> enabled = pwm->state.enabled;
>
> + if (state->polarity != pwm->state.polarity) {
> + if (enabled) {
> + stm32_pwm_disable(priv, pwm->hwpwm);
> + enabled = false;
> + }
> +
> + ret = stm32_pwm_set_polarity(priv, pwm->hwpwm, state->polarity);
> + if (ret)
> + return ret;
> + }
> +
> if (!state->enabled) {
> if (enabled)
> stm32_pwm_disable(priv, pwm->hwpwm);
> return 0;
> }
>
> - if (state->polarity != pwm->state.polarity)
> - stm32_pwm_set_polarity(priv, pwm->hwpwm, state->polarity);
> -
> ret = stm32_pwm_config(priv, pwm->hwpwm,
> state->duty_cycle, state->period);
> if (ret)
I would prefer the following change:
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index eb24054f9729..5f118c20f1ca 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -452,12 +452,16 @@ static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
enabled = pwm->state.enabled;
- if (!state->enabled) {
+ /* The hardware must be disabled to honor polarity changes. */
+ if (!state->enabled || state->polarity != pwm->state.polarity) {
if (enabled)
stm32_pwm_disable(priv, pwm->hwpwm);
- return 0;
+ enabled = false;
}
+ if (!state->enabled)
+ return 0;
+
if (state->polarity != pwm->state.polarity)
stm32_pwm_set_polarity(priv, pwm->hwpwm, state->polarity);
Maybe it's just me, but I think the resulting code is simpler with this
hunk.
I have hardware using this driver, will set it up later this week for
testing.
Best regards
Uwe
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists