Re: [PATCH v2] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration

[Uwe Kleine-König]

Hello,

On Tue, Jan 20, 2026 at 11:23:54AM +0530, Gokul Praveen wrote:
> On 20/01/26 10:10, Kumar, Udit wrote:
> > On 1/20/2026 3:03 AM, Uwe Kleine-König wrote:
> > > I doesn't look good to me, it's way to complicated. Unless I still
> > > misunderstand something, I think
> > > 
> > > diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> > > index 7a86cb090f76..4942689105f3 100644
> > > --- a/drivers/pwm/pwm-tiehrpwm.c
> > > +++ b/drivers/pwm/pwm-tiehrpwm.c
> > > @@ -378,6 +378,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));
> > > +
> > 
> > 
> > Fair point,  only need i see to keep in hardware active state after
> > dropping count at
> > 
> > https://elixir.bootlin.com/linux/v6.18.6/source/drivers/pwm/pwm-
> > tiehrpwm.c#L293

Yes, with my suggested change you can drop ehrpwm_pwm_config() grabbing
and releasing the pm_runtime reference making the whole change:

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 above changes looks simpler than the one I had sent earlier.
> Shall I send a v3 patch with these updated changes, if it is okay for you,
> Uwe.

I guess that's the only thing that brings us forward, and as I cannot
reproduce the issue but you can, yes please.

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature