linux-kernel - Re: [PATCH] pwm: stm32: handle polarity change when PWM is enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2e2iahbzcepbzwgk7xeta2afxmycfjgv2zofzngqjvp4on46r2@mzpi4bz4uqie>
Date: Tue, 06 Jan 2026 11:30:34 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Uwe Kleine-König <ukleinek@...nel.org>
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

Hi Uwe,

On Tue, Jan 06, 2026 at 11:22:57AM +0100, Uwe Kleine-König wrote:
> 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.

For your reference i bisected to that 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.
> 

I have tried to boot stable/master 6.19.0-rc4, my backlight is on!
In stm32_pwm_write_waveform() TIMx_CCER is set before MOE is set.

> > ---
> >  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.

Fine with me, I just looked at the other PWM drivers and copy/pasted
from them :)

> 
> I have hardware using this driver, will set it up later this week for
> testing.

Very cool, looking forward to hear if you can re-produce.

> 
> Best regards
> Uwe

/Sean