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

[Gokul Praveen <g-praveen@ti.com>]

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