drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
The period and duty cycle configurations does not get reflected
after setting them using sysfs nodes. This is because at the
end of ehrpwm_pwm_config function, the put_sync function is
called which resets the hardware.
Fix it by preventing the pwm controller from going into
low-power mode.
Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement .apply() callback")
Signed-off-by: Gokul Praveen <g-praveen@ti.com>
---
v2 <==> v1
==========
* Removed space between Fixes and Signed-off tag
drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7a86cb090f76..408aed70be8c 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
if (period_cycles < 1)
period_cycles = 1;
- pm_runtime_get_sync(pwmchip_parent(chip));
/* Update clock prescaler values */
ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
@@ -290,12 +289,11 @@ 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);
- pm_runtime_put_sync(pwmchip_parent(chip));
-
return 0;
}
-static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
{
struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
u16 aqcsfrc_val, aqcsfrc_mask;
@@ -304,6 +302,13 @@ 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));
+ ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
+
+ if (ret) {
+ pm_runtime_put_sync(pwmchip_parent(chip));
+ return ret;
+ }
+
/* Disabling Action Qualifier on PWM output */
if (pwm->hwpwm) {
aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
@@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
return 0;
}
- err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
- if (err)
- return err;
-
if (!enabled)
- err = ehrpwm_pwm_enable(chip, pwm);
+ err = ehrpwm_pwm_enable(chip, pwm, state);
+ else
+ err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
+ state->period, state->polarity);
return err;
}
--
2.34.1Hello,
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.
On Wed, Jan 07, 2026 at 11:23:39AM +0530, Gokul Praveen wrote:
> The period and duty cycle configurations does not get reflected
> after setting them using sysfs nodes. This is because at the
> end of ehrpwm_pwm_config function, the put_sync function is
> called which resets the hardware.
>
> Fix it by preventing the pwm controller from going into
> low-power mode.
>
> Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement .apply() callback")
> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
> ---
> v2 <==> v1
> ==========
> * Removed space between Fixes and Signed-off tag
>
> drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..408aed70be8c 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> if (period_cycles < 1)
> period_cycles = 1;
>
> - pm_runtime_get_sync(pwmchip_parent(chip));
>
> /* Update clock prescaler values */
> ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
> @@ -290,12 +289,11 @@ 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);
>
> - pm_runtime_put_sync(pwmchip_parent(chip));
> -
> return 0;
> }
>
> -static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
With this function also caring for *state the name isn't appropriate any
more.
> {
> struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
> u16 aqcsfrc_val, aqcsfrc_mask;
> @@ -304,6 +302,13 @@ 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));
>
> + ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
> +
> + if (ret) {
> + pm_runtime_put_sync(pwmchip_parent(chip));
> + return ret;
> + }
> +
> /* Disabling Action Qualifier on PWM output */
> if (pwm->hwpwm) {
> aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
> @@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> return 0;
> }
>
> - err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
> - if (err)
> - return err;
> -
> if (!enabled)
> - err = ehrpwm_pwm_enable(chip, pwm);
> + err = ehrpwm_pwm_enable(chip, pwm, state);
> + else
> + err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
> + state->period, state->polarity);
>
> return err;
> }
Why are the changes from the two hunks above needed? Reading the change
log I only understand the first hunk and would expect it to be enough.
Best regards
Uwe
Hi Uwe, Gokul, Thanks for CC'ing me on this thread. On 07/01/26 15:21, Uwe Kleine-König wrote: > Hello, > > 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. Regarding my pending patch series for this driver: it should not conflict this change, as it's largely preparatory refactoring for a follow-up series. Best regards, Rafael V. Volkmer
Hi Rafael, On 08/01/26 01:17, Rafael V. Volkmer wrote: > Hi Uwe, Gokul, > > Thanks for CC'ing me on this thread. > > On 07/01/26 15:21, Uwe Kleine-König wrote: >> Hello, >> >> 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. Best Regards Gokul Praveen > Regarding my pending patch series for this driver: it should not > conflict this change, as it's largely preparatory refactoring for a > follow-up series. > > Best regards, > Rafael V. Volkmer
Hello Gokul, 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. Best regards Uwe
Hi Uwe, On 08/01/26 23:40, Uwe Kleine-König wrote: > Hello Gokul, > > 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. > > Best regards > Uwe 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. However, with the below command sequence, the duty cycle was getting set properly 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 /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle PWM is working only if we re-update the duty cycle after enabling the module. If we do not re-update the duty cycle after enabling the module then PWM signal line is being high(100 % duty) always. Test Environment: TI J784S4 EVM board. Best Regards Gokul Praveen
Hello Gokul, On 1/9/2026 11:21 AM, Gokul Praveen wrote: > Hi Uwe, > > > On 08/01/26 23:40, Uwe Kleine-König wrote: >> Hello Gokul, >> >> 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. >> >> Best regards >> Uwe > > 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. > > However, with the below command sequence, the duty cycle was getting > set properly > > 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 > /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle I don't think , this last call of setting duty_cycle , even will land in driver code . > > > PWM is working only if we re-update the duty cycle after enabling the > module. > > If we do not re-update the duty cycle after enabling the module then > PWM signal line is being high(100 % duty) always. > > Test Environment: TI J784S4 EVM board. > > Best Regards > Gokul Praveen > > >
Hi Udit, On 15/01/26 10:50, Kumar, Udit wrote: > Hello Gokul, > > On 1/9/2026 11:21 AM, Gokul Praveen wrote: >> Hi Uwe, >> >> >> On 08/01/26 23:40, Uwe Kleine-König wrote: >>> Hello Gokul, >>> >>> 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. >>> >>> Best regards >>> Uwe >> >> 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. >> >> However, with the below command sequence, the duty cycle was getting >> set properly >> >> 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 >> /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle > > > I don't think , this last call of setting duty_cycle , even will land in > driver code . > > > Can you change the last command to the following. Ideally , it should be a different duty cycle set to enter into the driver code. >> /sys/class/pwm/pwmchip0/pwm0# echo 6000000 > duty_cycle Best Regards Gokul Praveen >> >> >> PWM is working only if we re-update the duty cycle after enabling the >> module. >> >> If we do not re-update the duty cycle after enabling the module then >> PWM signal line is being high(100 % duty) always. >> >> Test Environment: TI J784S4 EVM board. >> >> Best Regards >> Gokul Praveen >> >> >>
On 1/15/2026 11:44 AM, Gokul Praveen wrote: > Hi Udit, > > On 15/01/26 10:50, Kumar, Udit wrote: >> Hello Gokul, >> >> On 1/9/2026 11:21 AM, Gokul Praveen wrote: >>> Hi Uwe, >>> >>> >>> On 08/01/26 23:40, Uwe Kleine-König wrote: >>>> Hello Gokul, >>>> >>>> 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. >>>> >>>> Best regards >>>> Uwe >>> >>> 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. >>> >>> However, with the below command sequence, the duty cycle was getting >>> set properly >>> >>> 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 >>> /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle >> >> >> I don't think , this last call of setting duty_cycle , even will land >> in driver code . >> >> >> > > Can you change the last command to the following. Ideally , it should > be a different duty cycle set to enter into the driver code. > > >> /sys/class/pwm/pwmchip0/pwm0# echo 6000000 > duty_cycle Yes it will leads to entry into driver code. But between below calls, (enable and set duty cycle again) /sys/class/pwm/pwmchip0/pwm0# echo 1 > enable /sys/class/pwm/pwmchip0/pwm0# echo 3000000 > duty_cycle I don't see ehrpwm_pwm_disable will be called. > > Best Regards > Gokul Praveen >>> >>> >>> PWM is working only if we re-update the duty cycle after enabling >>> the module. >>> >>> If we do not re-update the duty cycle after enabling the module then >>> PWM signal line is being high(100 % duty) always. >>> >>> Test Environment: TI J784S4 EVM board. >>> >>> Best Regards >>> Gokul Praveen >>> >>> >>> >
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.
Best regards
Uwe
Hello Uwe, Gokul, Sorry for the late response, I was on vacation and away from my setup. On Fri, 9 Jan 2026 23:53:22 +0100, Uwe Kleine-König wrote: > 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.") Right, my previous reply meant I hadn't explicitly targeted this issue yet. I have now re-tested using Gokul's sysfs configuration sequence, but I still cannot reproduce it on my setup. > 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. Since I cannot reproduce the issue here, I can't validate whether adding pm_runtime_get_sync() changes the behavior, and I don't have a failing trace to share. For reference, I ran the tests on an AM62P EVM using TI's default SDK userspace, with a custom kernel on top, and U-Boot from the SDK. The board was booted from SD card. I used pwm1 instead of pwm0, since the PWM pin routed to the EVM 40-pin header is ball B21 (SPI0_CLK / EHRPWM1_A). The signal was verified with a logic analyzer at 24 MHz sampling rate. This makes me suspect the behavior Gokul observed might depend on another configuration interacting in parallel. If possible, could Gokul try the same recipe on an AM62 EVM using TI's default images and confirm whether the issue reproduces there? That is the platform I am currently working with. This should either match the AM62P results or help identify a relevant configuration difference. Best regards, Rafael V. Volkmer
Hi Rafael. On 19/01/26 08:25, Rafael V. Volkmer wrote: > Hello Uwe, Gokul, > > Sorry for the late response, I was on vacation and away from my setup. > > On Fri, 9 Jan 2026 23:53:22 +0100, Uwe Kleine-König wrote: >> 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.") > > Right, my previous reply meant I hadn't explicitly targeted this issue yet. > I have now re-tested using Gokul's sysfs configuration sequence, but I still > cannot reproduce it on my setup. > >> 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. > > Since I cannot reproduce the issue here, I can't validate whether adding > pm_runtime_get_sync() changes the behavior, and I don't have a failing > trace to share. > > For reference, I ran the tests on an AM62P EVM using TI's default SDK > userspace, with a custom kernel on top, and U-Boot from the SDK. The > board was booted from SD card. > > I used pwm1 instead of pwm0, since the PWM pin routed to the EVM 40-pin > header is ball B21 (SPI0_CLK / EHRPWM1_A). The signal was verified with a > logic analyzer at 24 MHz sampling rate. > > This makes me suspect the behavior Gokul observed might depend on another > configuration interacting in parallel. > > If possible, could Gokul try the same recipe on an AM62 EVM using TI's > default images and confirm whether the issue reproduces there? That is the > platform I am currently working with. This should either match the AM62P > results or help identify a relevant configuration difference. > Can you test the same on TI J784S4 EVM as I reproduced the issue on this board. I believe dumping the registers and capturing the signals using logic analyzer is the best way to reproduce this issue. The easiest way I tried to reproduce this issue is by enabling debug prints. 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. It would be really helpful, if you can try the same procedure on J784S4 EVM, Rafael. I have also attached a patch(0001-Enable-EHRPWM-1_B-using AC33-pin.patch) which includes the device tree changes for enabling EHRPWM1_B. The testpoint used for signal capture is TP126. Additionally, it would be great if you can also share the output after applying the attached patch((0001-Debug-prints-to-check-if-period-and-duty-cycle-is-re.patch) on both AM62 EVM and J784S4 EVM. The issue is that I do not have an AM62 EVM with me, actually. Apologies for that, Rafael. Thanks in advance for your help, Rafael. Best Regards Gokul Praveen > Best regards, > Rafael V. Volkmer
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
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().
Best regards
Uwe
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.
Best Regards
Gokul Praveen.
> Best regards
> Uwe
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
>
Hi Uwe,
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.
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.
>
> Best regards
> Uwe
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.) Best regards Uwe
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 https://gist.github.com/uditkumarti/6e36ca14423afc06378b8fa447ca3fe6 > Best regards > Uwe
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));
+
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
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
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
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
Hello,
On Tue, Jan 20, 2026 at 11:23:54AM +0530, Gokul Praveen wrote:
> On 20/01/26 10:10, Kumar, Udit wrote:
> > On 1/20/2026 3:03 AM, Uwe Kleine-König wrote:
> > > 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
Yes, with my suggested change you can drop ehrpwm_pwm_config() grabbing
and releasing the pm_runtime reference making the whole change:
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7a86cb090f76..2533c95b0ba9 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -237,8 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
if (period_cycles < 1)
period_cycles = 1;
- pm_runtime_get_sync(pwmchip_parent(chip));
-
/* Update clock prescaler values */
ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
@@ -290,8 +288,6 @@ 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);
- pm_runtime_put_sync(pwmchip_parent(chip));
-
return 0;
}
@@ -378,6 +374,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));
+
if (state->polarity != pwm->state.polarity) {
if (enabled) {
ehrpwm_pwm_disable(chip, pwm);
> 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.
I guess that's the only thing that brings us forward, and as I cannot
reproduce the issue but you can, yes please.
Best regards
Uwe
Hi Uwe,
On 20/01/26 13:53, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Jan 20, 2026 at 11:23:54AM +0530, Gokul Praveen wrote:
>> On 20/01/26 10:10, Kumar, Udit wrote:
>>> On 1/20/2026 3:03 AM, Uwe Kleine-König wrote:
>>>> 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
>
> Yes, with my suggested change you can drop ehrpwm_pwm_config() grabbing
> and releasing the pm_runtime reference making the whole change:
>
> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
> index 7a86cb090f76..2533c95b0ba9 100644
> --- a/drivers/pwm/pwm-tiehrpwm.c
> +++ b/drivers/pwm/pwm-tiehrpwm.c
> @@ -237,8 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> if (period_cycles < 1)
> period_cycles = 1;
>
> - pm_runtime_get_sync(pwmchip_parent(chip));
> -
> /* Update clock prescaler values */
> ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
>
> @@ -290,8 +288,6 @@ 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);
>
> - pm_runtime_put_sync(pwmchip_parent(chip));
> -
> return 0;
> }
>
> @@ -378,6 +374,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));
> +
> if (state->polarity != pwm->state.polarity) {
> if (enabled) {
> ehrpwm_pwm_disable(chip, pwm);
>
>> 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.
>
> I guess that's the only thing that brings us forward, and as I cannot
> reproduce the issue but you can, yes please.
>
Thanks for the confirmation. I will make the v3 changes and send it /
Thank you for you great support,Uwe.
> Best regards
> Uwe
Hi Uwe,
Thank you for your prompt response and reply.
On 07/01/26 15:21, Uwe Kleine-König wrote:
> Hello,
>
> 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.
>
Yes, it applies to all hardware variants, Uwe.
> On Wed, Jan 07, 2026 at 11:23:39AM +0530, Gokul Praveen wrote:
>> The period and duty cycle configurations does not get reflected
>> after setting them using sysfs nodes. This is because at the
>> end of ehrpwm_pwm_config function, the put_sync function is
>> called which resets the hardware.
>>
>> Fix it by preventing the pwm controller from going into
>> low-power mode.
>>
>> Fixes: 5f027d9b83db("pwm: tiehrpwm: Implement .apply() callback")
>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>> ---
>> v2 <==> v1
>> ==========
>> * Removed space between Fixes and Signed-off tag
>>
>> drivers/pwm/pwm-tiehrpwm.c | 22 +++++++++++++---------
>> 1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
>> index 7a86cb090f76..408aed70be8c 100644
>> --- a/drivers/pwm/pwm-tiehrpwm.c
>> +++ b/drivers/pwm/pwm-tiehrpwm.c
>> @@ -237,7 +237,6 @@ static int ehrpwm_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>> if (period_cycles < 1)
>> period_cycles = 1;
>>
>> - pm_runtime_get_sync(pwmchip_parent(chip));
>>
>> /* Update clock prescaler values */
>> ehrpwm_modify(pc->mmio_base, TBCTL, TBCTL_CLKDIV_MASK, tb_divval);
>> @@ -290,12 +289,11 @@ 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);
>>
>> - pm_runtime_put_sync(pwmchip_parent(chip));
>> -
>> return 0;
>> }
>>
>> -static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>> +static int ehrpwm_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm,
>> + const struct pwm_state *state)
>
> With this function also caring for *state the name isn't appropriate any
> more.
>
But the duty cycle, period and polarity values are extracted from the
state parameter, right?
Please feel free to correct me if I am wrong, Uwe?
>> {
>> struct ehrpwm_pwm_chip *pc = to_ehrpwm_pwm_chip(chip);
>> u16 aqcsfrc_val, aqcsfrc_mask;
>> @@ -304,6 +302,13 @@ 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));
>>
>> + ret = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
>> +
>> + if (ret) {
>> + pm_runtime_put_sync(pwmchip_parent(chip));
>> + return ret;
>> + }
>> +
>> /* Disabling Action Qualifier on PWM output */
>> if (pwm->hwpwm) {
>> aqcsfrc_val = AQCSFRC_CSFB_FRCDIS;
>> @@ -391,12 +396,11 @@ static int ehrpwm_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> return 0;
>> }
>>
>> - err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle, state->period, state->polarity);
>> - if (err)
>> - return err;
>> -
>> if (!enabled)
>> - err = ehrpwm_pwm_enable(chip, pwm);
>> + err = ehrpwm_pwm_enable(chip, pwm, state);
>> + else
>> + err = ehrpwm_pwm_config(chip, pwm, state->duty_cycle,
>> + state->period, state->polarity);
>>
>> return err;
>> }
>
> Why are the changes from the two hunks above needed? Reading the change
> log I only understand the first hunk and would expect it to be enough.
>
The 2nd hunk is needed I believe because now, the ehrpwm_pwm_config
function is called inside the ehrpwm_pwm_enable function.
Hence, calling the ehrpwm_pwm_config function before the
ehrpwm_pwm_enable function within the ehrpwm_pwm_apply function would be
a redundant call I believe when we try to enable the ehrpwm.
Now, coming to the ehrpwm_pwm_config function which I have added after
the ehrpwm_pwm_enable function in the else case, it would be needed I
believe in order to change the duty cycle and period at runtime after
the pwm has been enabled.
Please feel free and do correct me if I am wrong, Uwe.
> Best regards
> Uwe
© 2016 - 2026 Red Hat, Inc.