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

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

[-- Attachment #1: Type: text/plain, Size: 6345 bytes --]

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.

The best way to check if the value is reflected is by dumping the 
registers.

On J784S4 EVM , I executed the script attached(gen_pwm.sh)and dumped the 
EPWM_CMPB Register(reflects the duty cycle) using the below command:

 >devmem2 0x03010014

Output:
0x00000000

The above output indicates that the duty cycle was not set as the 
register values are 0.

However, after executing the command(echo 40000000 > 
/sys/class/pwm/pwmchip0/pwm1/duty_cycle) after the above steps were 
done(ie: executing script and dumping registers), the duty cycle is 
reflected properly.

 >devmem2 0x03010014

Output :
0x000065B9

The LSB 2 bytes(65B9)(reflecting EPWM_CMPB register) , indicates 40% 
duty cycle set properly.

This is how I confirmed that there is a need to set the 
duty-cycle(different from the previous one) again, even after enabling 
the pwm.

I am also sharing the device tree 
changes(0001-Enable-EHRPWM-1_B-using-AC33-pin.patch file) made to enable 
ehrpwm1_B on J784S4 EVM.

Also, just ensure that SW2.2 is set high(1), so that the AC33 pin(used 
for EHRPWM1_B output) is routed to TP 126.

Additionally, I also measured the PWM signal using logic analyzer and 
was able to reproduce this issue.

Best Regards
Gokul Praveen

> Best regards
> Uwe

[-- Attachment #2: gen_pwm.sh --]
[-- Type: application/x-shellscript, Size: 358 bytes --]

[-- Attachment #3: 0001-Enable-EHRPWM-1_B-using-AC33-pin.patch --]
[-- Type: text/x-patch, Size: 3396 bytes --]

From d4da610f7104f254e7dc5f9d7f6aa1b112fda287 Mon Sep 17 00:00:00 2001
From: Gokul Praveen <g-praveen@ti.com>
Date: Thu, 15 Jan 2026 14:52:59 +0530
Subject: [PATCH] Enable EHRPWM 1_B using AC33 pin Enable PWM EVENT TRACING

---
 .../boot/dts/ti/k3-j784s4-j742s2-common.dtsi   |  1 +
 .../dts/ti/k3-j784s4-j742s2-evm-common.dtsi    | 18 ++++++++++++++++--
 arch/arm64/configs/defconfig                   |  7 ++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi
index 1dceff119a47..af4103f159e3 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi
@@ -103,6 +103,7 @@ cbass_main: bus@100000 {
 			 <0x44 0x00000000 0x44 0x00000000 0x00 0x08000000>, /* PCIe2 DAT0 */
 			 <0x44 0x10000000 0x44 0x10000000 0x00 0x08000000>, /* PCIe3 DAT0 */
 			 <0x4e 0x20000000 0x4e 0x20000000 0x00 0x00080000>, /* GPU */
+			 <0x00 0x03010000 0x00 0x03010000 0x00 0x00000100>, /*EHRPWM1*/
 
 			 /* MCUSS_WKUP Range */
 			 <0x00 0x28380000 0x00 0x28380000 0x00 0x03880000>,
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-evm-common.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-evm-common.dtsi
index e50735577737..4a63ddeaa372 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-evm-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-evm-common.dtsi
@@ -296,6 +296,13 @@ J784S4_IOPAD(0x0e8, PIN_INPUT, 8) /* (AR38) TIMER_IO0.MMC1_SDCD */
 		>;
 	};
 
+	main_ehrpwm1_pins_default: main-ehrpwm1-pins-default {
+		bootph-all;
+		pinctrl-single,pins = <			
+			J784S4_IOPAD(0x074, PIN_OUTPUT, 9) /* (AC33) EHRPWM1_B.GPIO0_29 */
+		>;
+	};
+
 	vdd_sd_dv_pins_default: vdd-sd-dv-default-pins {
 		pinctrl-single,pins = <
 			J784S4_IOPAD(0x020, PIN_INPUT, 7) /* (AJ35) MCAN15_RX.GPIO0_8 */
@@ -661,6 +668,13 @@ &mcu_uart0 {
 	pinctrl-0 = <&mcu_uart0_pins_default>;
 };
 
+&main_ehrpwm1 {
+       /* EHRPWM1*/
+       pinctrl-names = "default";
+       pinctrl-0 = <&main_ehrpwm1_pins_default>;
+       status = "okay";
+};
+
 &main_uart8 {
 	bootph-all;
 	status = "okay";
@@ -852,7 +866,7 @@ p15-hog {
 			/* P15 - CANUART_MUX1_SEL1 */
 			gpio-hog;
 			gpios = <15 GPIO_ACTIVE_HIGH>;
-			output-high;
+			output-low;
 			line-name = "CANUART_MUX1_SEL1";
 		};
 	};
@@ -888,7 +902,7 @@ &main_sdhci0 {
 &main_sdhci1 {
 	bootph-all;
 	/* SD card */
-	status = "okay";
+	status = "disabled";
 	pinctrl-0 = <&main_mmc1_pins_default>;
 	pinctrl-names = "default";
 	disable-wp;
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 45288ec9eaf7..d08d9346e848 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1643,7 +1643,7 @@ CONFIG_PWM_STM32=m
 CONFIG_PWM_SUN4I=m
 CONFIG_PWM_TEGRA=m
 CONFIG_PWM_TIECAP=m
-CONFIG_PWM_TIEHRPWM=m
+CONFIG_PWM_TIEHRPWM=y
 CONFIG_PWM_VISCONTI=m
 CONFIG_SL28CPLD_INTC=y
 CONFIG_QCOM_PDC=y
@@ -1859,6 +1859,10 @@ CONFIG_DEBUG_FS=y
 # CONFIG_SCHED_DEBUG is not set
 # CONFIG_DEBUG_PREEMPT is not set
 # CONFIG_FTRACE is not set
+CONFIG_FTRACE=y
+CONFIG_TRACING=y
+CONFIG_EVENT_TRACING=y
+CONFIG_TRACING_SUPPORT=y
 CONFIG_CORESIGHT=m
 CONFIG_CORESIGHT_LINK_AND_SINK_TMC=m
 CONFIG_CORESIGHT_CATU=m
@@ -1868,3 +1872,4 @@ CONFIG_CORESIGHT_STM=m
 CONFIG_CORESIGHT_CPU_DEBUG=m
 CONFIG_CORESIGHT_CTI=m
 CONFIG_MEMTEST=y
+
-- 
2.34.1