linux-kernel - Re: [PATCH v3] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration

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] [day] [month] [year] [list]

Message-ID: <uge7sfrlkyqr7wclrps3o6wyod7rmndewwtm5bspkjyw5tadvs@qgqyoi7xnnoh>
Date: Tue, 20 Jan 2026 19:17:00 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Gokul Praveen <g-praveen@...com>
Cc: j-keerthy@...com, linux-pwm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, u-kumar1@...com, n-francis@...com
Subject: Re: [PATCH v3] pwm: tiehrpwm: Enable EHRPWM controller before
 setting configuration

Hello,

On Tue, Jan 20, 2026 at 02:49:22PM +0530, Gokul Praveen wrote:
> The period and duty cycle configurations does not get reflected
> after setting them using sysfs nodes. This is because at the
> end of ehrpwm_pwm_config function, the put_sync function is
> called which resets the hardware.

I'd like to have the text account for not all machines (SoCs?) suffering
from that problem. Also the subject is wrong, as you enable the parent
of the PWM controller and not the controller itself?!
 
> Fix it by preventing the pwm controller from going into
> low-power mode.

Please make this:

	Hold the PWM controller out of low-power mode during .apply() to
	make sure it accepts the writes to its registers.

	This renders the calls to pm_runtime_get_sync() and
	pm_runtime_put_sync() in ehrpwm_pwm_config() into no-ops, so
	these can be dropped.

as your sentence is wrong in general.

> Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement .apply() callback")
> Signed-off-by: Gokul Praveen <g-praveen@...com>
> ---
> v3 <==> v2
> ==========
> * Simplified the logic by adding guard function for runtime power 
>   management.
> 
> v2 <==> v1
> ==========
> * Removed space between Fixes and Signed-off tag
> 
>  drivers/pwm/pwm-tiehrpwm.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..2533c95b0ba9 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -237,8 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	if (period_cycles < 1)
>  		period_cycles = 1;
>  
> -	pm_runtime_get_sync(pwmchip_parent(chip));
> -
>  	/* Update clock prescaler values */
>  	ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
>  
> @@ -290,8 +288,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	if (!(duty_cycles > period_cycles))
>  		ehrpwm_write(pc->mmio_base, cmp_reg, duty_cycles);
>  
> -	pm_runtime_put_sync(pwmchip_parent(chip));
> -
>  	return 0;
>  }
>  
> @@ -378,6 +374,8 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	int err;
>  	bool enabled = pwm->state.enabled;
>  
> +	guard(pm_runtime_active)(pwmchip_parent(chip));
> +
>  	if (state->polarity != pwm->state.polarity) {
>  		if (enabled) {
>  			ehrpwm_pwm_disable(chip, pwm);

The patch is excellent :-D

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)