Re: [PATCH v2] pwm: tiehrpwm: Enable EHRPWM controller before setting configuration
From: Gokul Praveen
Date: Fri Jan 16 2026 - 04:10:46 EST
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 RegardsFrom 793171b857a7c37c838d882b2610b89d6ef90e1a Mon Sep 17 00:00:00 2001
Gokul Praveen.
Best regards
Uwe
From: Gokul Praveen <g-praveen@xxxxxx>
Date: Fri, 16 Jan 2026 14:31:02 +0530
Subject: [PATCH] Debug prints to check if period and duty cycle is reflected
or not
Signed-off-by: Gokul Praveen <g-praveen@xxxxxx>
---
drivers/pwm/pwm-tiehrpwm.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7a86cb090f76..3626375c5e4a 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -283,6 +283,7 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
ehrpwm_write(pc->mmio_base, TBPRD, period_cycles - 1);
+
/* Configure ehrpwm counter for up-count mode */
ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CTRMODE_MASK,
TBCTL_CTRMODE_UP);
@@ -290,7 +291,15 @@ 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);
+ u16 duty_cycle = ehrpwm_read(pc->mmio_base, CMPB);
+ u16 period = ehrpwm_read(pc->mmio_base, TBPRD);
+
+ printk("Before put sync: Period:%u, Duty cycle:%u\n",(unsigned int)period,(unsigned int)duty_cycle);
pm_runtime_put_sync(pwmchip_parent(chip));
+
+ //duty_cycle = ehrpwm_read(pc->mmio_base, CMPB);
+ //period = ehrpwm_read(pc->mmio_base, TBPRD);
+ //printk("After put sync: Period:%d, Duty cycle:%d\n",period,duty_cycle);
return 0;
}
@@ -304,6 +313,10 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
/* Leave clock enabled on enabling PWM */
pm_runtime_get_sync(pwmchip_parent(chip));
+ u16 duty_cycle = ehrpwm_read(pc->mmio_base, CMPB);
+ u16 period = ehrpwm_read(pc->mmio_base, TBPRD);
+ printk("EHRPWM enable function: Period:%u, Duty cycle:%u\n",(unsigned int)period,(unsigned int)duty_cycle);
+
/* Disabling Action Qualifier on PWM output */
if (pwm->hwpwm) {
aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
@@ -327,6 +340,7 @@ static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
return ret;
}
+
return 0;
}
--
2.34.1