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

[Gokul Praveen]

Hi Uwe,


On 20/01/26 13:53, Uwe Kleine-König wrote:
> 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.

Thanks for the confirmation. I will make the v3 changes and send it /

Thank you for you great support,Uwe.

> Best regards
> Uwe