| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8c699821-1fe8-4205-812e-b302f3d10d24@ti.com>
Date: Fri, 16 Jan 2026 14:40:37 +0530
From: Gokul Praveen <g-praveen@...com>
To: 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>, <u-kumar1@...com>, Gokul Praveen <g-praveen@...com>
Subject: Re: [PATCH v2] pwm: tiehrpwm: Enable EHRPWM controller before setting
configuration
Hi Uwe,
Sory for adding another reply on top of my latest reply.
Just wanted to add a couple more things;
On 16/01/26 14:11, Gokul Praveen wrote:
> Hi Uwe,
>
> On 15/01/26 22:47, Uwe Kleine-König wrote:
>> On Thu, Jan 15, 2026 at 04:02:16PM +0530, Gokul Praveen wrote:
>>> Hi Uwe,
>>>
>>> Apologies for the delay as it took some time for me to get the trace
>>> output
>>> as well as to get the way I reproduced the issue.
>>>
>>>
>>> On 10/01/26 04:23, Uwe Kleine-König wrote:
>>>> Hello Gokul,
>>>>
>>>> On Fri, Jan 09, 2026 at 11:21:46AM +0530, Gokul Praveen wrote:
>>>>> On 08/01/26 23:40, Uwe Kleine-König wrote:
>>>>>> On Thu, Jan 08, 2026 at 12:10:35PM +0530, Gokul Praveen wrote:
>>>>>>> On 08/01/26 01:17, Rafael V. Volkmer wrote:
>>>>>>>> Thanks for CC'ing me on this thread.
>>>>>>>>
>>>>>>>> On 07/01/26 15:21, Uwe Kleine-König wrote:
>>>>>>>>> adding Rafael to Cc: who sent a patch series for this driver
>>>>>>>>> that I
>>>>>>>>> didn't come around to review yet. Given that neither he nor me
>>>>>>>>> noticed
>>>>>>>>> the problem addressed in this patch I wonder if it applies to all
>>>>>>>>> hardware variants.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I also didn't observe the issue described here in my testing:
>>>>>>>> duty cycle and
>>>>>>>> period changes always appeared to take effect as expected.
>>>>>>>>
>>>>>>>> My tests were done on an AM623 EVM.
>>>>>>>>
>>>>>>>> One possible explanation is that my test flow mostly exercised
>>>>>>>> configuration
>>>>>>>> while the PWM was already enabled/active, which could mask the
>>>>>>>> effect of a
>>>>>>>> put_sync/reset happening after configuration.
>>>>>>>>
>>>>>>>
>>>>>>> Yes, this is the reason why the configuration was taking effect
>>>>>>> for you ,
>>>>>>> Rafael, as the PWM was already enabled when setting the
>>>>>>> configuration hence
>>>>>>> masking the effect of a put_sync/reset happening after
>>>>>>> configuration.
>>>>>>
>>>>>> Can you provide a list of commands that show the failure? That would
>>>>>> result in less guessing for me. My plan is to reproduce the failure
>>>>>> tomorrow to better understand it on my boneblack.
>>>>>
>>>>> Sure Uwe. These are the commands I have tried for PWM signal
>>>>> generation:
>>>>>
>>>>> cd /sys/class/pwm/pwmchip0
>>>>> /sys/class/pwm/pwmchip0# echo 0 > export
>>>>> /sys/class/pwm/pwmchip0# cd pwm0/
>>>>> /sys/class/pwm/pwmchip0/pwm0# echo 10000000 > period
>>>>> /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle
>>>>> /sys/class/pwm/pwmchip0/pwm0# echo "normal" > polarity
>>>>> /sys/class/pwm/pwmchip0/pwm0# echo 1 > enable
>>>>>
>>>>> Once these commands were executed, I measured the PWM signal using
>>>>> logic
>>>>> analyzer and the duty cycle was 100% even though we had set 30%
>>>>> duty cycle
>>>>> through the sysfs nodes.
>>>>
>>>> After that sequence I "see" a 30% relative duty cycle on my boneblack.
>>>> (With the follwing patch applied on top of pwm/for-next:
>>>>
>>>> diff --git a/arch/arm/boot/dts/ti/omap/am335x-boneblack.dts b/arch/
>>>> arm/boot/dts/ti/omap/am335x-boneblack.dts
>>>> index b4fdcf9c02b5..a3f4a4bb64e4 100644
>>>> --- a/arch/arm/boot/dts/ti/omap/am335x-boneblack.dts
>>>> +++ b/arch/arm/boot/dts/ti/omap/am335x-boneblack.dts
>>>> @@ -173,3 +173,25 @@ &gpio3 {
>>>> &baseboard_eeprom {
>>>> vcc-supply = <&ldo4_reg>;
>>>> };
>>>> +
>>>> +&am33xx_pinmux {
>>>> + ehrpwm0_pins: ehrpwm0-pins {
>>>> + pinctrl-single,pins = <
>>>> + /* P9.21 UART2_TXD -> ehrpwm0B */
>>>> + AM33XX_PADCONF(AM335X_PIN_SPI0_D0, PIN_OUTPUT_PULLDOWN,
>>>> MUX_MODE3)
>>>> + /* P9.22 UART2_RXD -> ehrpwm0A */
>>>> + AM33XX_PADCONF(AM335X_PIN_SPI0_SCLK,
>>>> PIN_OUTPUT_PULLDOWN, MUX_MODE3)
>>>> + >;
>>>> + };
>>>> +};
>>>> +
>>>> +&ehrpwm0 {
>>>> + pinctrl-names = "default";
>>>> + pinctrl-0 = <&ehrpwm0_pins>;
>>>> +
>>>> + status = "okay";
>>>> +};
>>>> +
>>>> +&epwmss0 {
>>>> + status = "okay";
>>>> +};
>>>>
>>>> )
>>>>
>>>> That makes me think the problem isn't understood well yet and needs
>>>> more
>>>> research. @Rafael, does the problem reproduce for you with Gokul's
>>>> recipe? (Or did you try that already? I understood your reply as "I
>>>> didn't encounter the issue but also didn't test specifically for
>>>> that.")
>>>>
>>>> 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.
>>>>
>>>
>>> PWM EVENT TRACING OUTPUT:
>>> =========================
>>> # tracer: nop
>>> #
>>> # entries-in-buffer/entries-written: 3/3 #P:8
>>> #
>>> # _-----=> irqs-off/BH-disabled
>>> # / _----=> need-resched
>>> # | / _---=> hardirq/softirq
>>> # || / _--=> preempt-depth
>>> # ||| / _-=> migrate-disable
>>> # |||| / delay
>>> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
>>> # | | | ||||| | |
>>> gen_pwm.sh-1039 [000] ..... 88.564669: pwm_apply:
>>> pwmchip0.1:
>>> period=100000000 duty_cycle=0 polarity=0 enabled=0 err=0
>>> gen_pwm.sh-1039 [000] ..... 88.564728: pwm_apply:
>>> pwmchip0.1:
>>> period=100000000 duty_cycle=30000000 polarity=0 enabled=0 err=0
>>> gen_pwm.sh-1039 [000] ..... 88.565065: pwm_apply:
>>> pwmchip0.1:
>>> period=100000000 duty_cycle=30000000 polarity=0 enabled=1 err=0
>>>
>>> The trace output might mislead us thinking that the duty cycle is set
>>> properly as the event tracer reads the duty_cycle variable which gets
>>> set
>>> irrespective of whether the value gets reflected in the pwm registers or
>>> not.
>>
>> Can you please also enable CONFIG_PWM_DEBUG? That should show the values
>> actually used as it enables a few calls to .get_state().
>>
>
> It is the same event trace output as given above, Uwe.
>
> Additionally, adding this config did not show any additional logs as
> there is no ,get_state callback implementation in the TI EHRPWM
> driver(pwm-tiehrpwm.c)
>
> I also confirmed the same using the below dmesg output:
>
> [ 0.484725] ehrpwm 3010000.pwm: Please implement the .get_state()
> callback
>
> I believe dumping the registers and capturing the signals using logic
> analyzer is the best way to reproduce this issue, Uwe.
>
I have found another easier way to show this issue by enabling debug
prints, Uwe. I have attached the patch for the
same(0001-Debug-prints-to-check-if-period-and-duty-cycle-is-re.patch).
This patch basically reads the registers and prints its value.
So, after applying the attached patch and running the following
commands, I got the following output.
Commands:
>>>>> cd /sys/class/pwm/pwmchip0
>>>>> /sys/class/pwm/pwmchip0# echo 1 > export
>>>>> /sys/class/pwm/pwmchip0# cd pwm1/
>>>>> /sys/class/pwm/pwmchip0/pwm1# echo 10000000 > period
>>>>> /sys/class/pwm/pwmchip0/pwm1# echo 3000000 > duty_cycle
>>>>> /sys/class/pwm/pwmchip0/pwm1# echo "normal" > polarity
>>>>> /sys/class/pwm/pwmchip0/pwm1# echo 1 > enable
Output:
Before put sync: Period:65103, Duty cycle:19531
EHRPWM enable function: Period:0, Duty cycle:0
This indicates that the duty cycle and period is not reflected.
Sorry for posting one reply on top of another, Uwe.
Best Regards
Gokul Praveen
> Best Regards
> Gokul Praveen.
>
>> Best regards
>> Uwe
>
View attachment "0001-Debug-prints-to-check-if-period-and-duty-cycle-is-re.patch" of type "text/x-patch" (2137 bytes)
Powered by blists - more mailing lists