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

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

Hi Uwe and Udit,


On 20/01/26 10:10, Kumar, Udit wrote:
> Hi Uwe,
> 
> On 1/20/2026 3:03 AM, Uwe Kleine-König wrote:
>> On Mon, Jan 19, 2026 at 08:30:08PM +0530, Kumar, Udit wrote:
>>> Hi Uwe/Gokul,
>>>
>>> On 1/12/2026 12:52 PM, Uwe Kleine-König wrote:
>>>> Hello Gokul,
>>>>
>>>> On Mon, Jan 12, 2026 at 11:21:50AM +0530, Gokul Praveen wrote:
>>>>> On 10/01/26 04:23, Uwe Kleine-König wrote:
>>>>>> As I cannot reproduce the issue, can you please check if adding
>>>>>>
>>>>>>     pm_runtime_get_sync(pwmchip_parent(chip));
>>>>>>
>>>>>> to the probe function makes the problem disappear? Also please 
>>>>>> boot with
>>>>>>
>>>>>>     trace_event=pwm
>>>>>>
>>>>>> on the command line and provide the content of
>>>>>> /sys/kernel/debug/tracing/trace after reproducing the problem.
>>>>> sure Uwe, I will try this from my side.
>>>>>
>>>>> In the meantime, will you able to test the same on TI J784S4 EVM as 
>>>>> the
>>>>> issue was reproduced on this board.
>>>> I don't have such hardware, sorry. The boards with TI SoC on my desk 
>>>> are
>>>> only a Beaglebone Black and a BeaglePlay. (And I didn't setup the
>>>> BeaglePlay yet, it was already quite a hassle to make the boneblack 
>>>> work
>>>> with a recent kernel. In the end it looks trivial, but
>>>> https://salsa.debian.org/kernel-team/linux/-/merge_requests/1777 plus
>>>> unreliable netbooting in the bootloader took me several hours to sort
>>>> out.)
>>> I am able to see this issue on J7200 hardware ,
>>>
>>> LTM, it may work on certain devices, depending upon how LPSC (Local 
>>> power
>>> state controller) and PSC (power state controller))
>>>
>>> are managed.
>>>
>>> In original code , while putting sync at
>>>
>>> https://elixir.bootlin.com/linux/v6.18.6/source/drivers/pwm/pwm- 
>>> tiehrpwm.c#L293
>>>
>>>
>>> will leads to calling genpd driver [0], which may put PWM IP in 
>>> powered down
>>> state, leading to loosing contents.
>>>
>>> So, we need retain pm count (genpd on in fact) between config and enable
>>> call.
>>>
>>> Therefore this patch LGTM
>> 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
> 

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.

Best Regards
Gokul Praveen

> Above changes will achieve same as well.
> 
> 
>>       if (state->polarity != pwm->state.polarity) {
>>           if (enabled) {
>>               ehrpwm_pwm_disable(chip, pwm);
>>
>> is enough to fix the issue. (We need something like
>> https://lore.kernel.org/linux-pwm/20251123233349.2122-1- 
>> rafael.v.volkmer@gmail.com/
>> to make this really robust.)
>>
>> Best regards
>> Uwe