[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <77e5712b-e1f9-4fb7-85df-45e1ade6a8eb@ti.com>
Date: Tue, 20 Jan 2026 11:23:54 +0530
From: Gokul Praveen <g-praveen@...com>
To: "Kumar, Udit" <u-kumar1@...com>, Uwe Kleine-König
<ukleinek@...nel.org>
CC: "Rafael V. Volkmer" <rafael.v.volkmer@...il.com>, <j-keerthy@...com>,
<linux-kernel@...r.kernel.org>, <linux-pwm@...r.kernel.org>,
<n-francis@...com>, Gokul Praveen <g-praveen@...com>
Subject: Re: [PATCH v2] pwm: tiehrpwm: Enable EHRPWM controller before setting
configuration
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@...il.com/
>> to make this really robust.)
>>
>> Best regards
>> Uwe
Powered by blists - more mailing lists