| Message ID | 20200228095651.32464-5-lokeshvutla@ti.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pwm: omap-dmtimer: Allow for dynamic pwm period updates | expand |
* Lokesh Vutla <lokeshvutla@ti.com> [200228 09:58]: > pwm_omap_dmtimer_disable() calls .stop callback which abruptly stops the > timer counter. This doesn't complete the current pwm cycle and > immediately disables the pwm. Instead disable the auto reload > functionality which allows to complete the current pwm cycle and then > disables the timer. > > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > drivers/pwm/pwm-omap-dmtimer.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c > index bc338619232d..89b3c25d02b8 100644 > --- a/drivers/pwm/pwm-omap-dmtimer.c > +++ b/drivers/pwm/pwm-omap-dmtimer.c > @@ -93,8 +93,16 @@ static void pwm_omap_dmtimer_disable(struct pwm_chip *chip, > { > struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); > > + /* > + * Disable auto reload so that the current cycle gets completed and > + * then the counter stops. > + */ > mutex_lock(&omap->mutex); > - omap->pdata->stop(omap->dm_timer); > + omap->pdata->set_pwm(omap->dm_timer, > + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, > + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, > + false); > + > mutex_unlock(&omap->mutex); > } I'm seeing an issue with this patch where after use something is left on and power consumption stays higher by about 30 mW after use. I can reproduce this easily on droid4 with Sebastian's rumble-test app[0]. After use, I sometimes also hear the vibrator keep chirping quietly, so there seems to be some pwm still happening after disable :) Reloading modules for pwm-vibra and pwm-omap-dmtimer make the power consumption go back down again. If you have a scope set up, maybe check the lines are quiet after disable after this patch? Regards, Tony [0] https://git.collabora.com/cgit/user/sre/rumble-test.git/plain/rumble-test.c
Hi Tony, On 06/03/20 11:44 PM, Tony Lindgren wrote: > * Lokesh Vutla <lokeshvutla@ti.com> [200228 09:58]: >> pwm_omap_dmtimer_disable() calls .stop callback which abruptly stops the >> timer counter. This doesn't complete the current pwm cycle and >> immediately disables the pwm. Instead disable the auto reload >> functionality which allows to complete the current pwm cycle and then >> disables the timer. >> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >> --- >> drivers/pwm/pwm-omap-dmtimer.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c >> index bc338619232d..89b3c25d02b8 100644 >> --- a/drivers/pwm/pwm-omap-dmtimer.c >> +++ b/drivers/pwm/pwm-omap-dmtimer.c >> @@ -93,8 +93,16 @@ static void pwm_omap_dmtimer_disable(struct pwm_chip *chip, >> { >> struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); >> >> + /* >> + * Disable auto reload so that the current cycle gets completed and >> + * then the counter stops. >> + */ >> mutex_lock(&omap->mutex); >> - omap->pdata->stop(omap->dm_timer); >> + omap->pdata->set_pwm(omap->dm_timer, >> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, >> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, >> + false); >> + >> mutex_unlock(&omap->mutex); >> } > > I'm seeing an issue with this patch where after use something is > left on and power consumption stays higher by about 30 mW after > use. Interesting...What is the PWM period and duty cycle in the test case? Can you dump the following registers before and after disabling: - TLDR - TMAR - TCLR > > I can reproduce this easily on droid4 with Sebastian's rumble-test > app[0]. After use, I sometimes also hear the vibrator keep chirping > quietly, so there seems to be some pwm still happening after disable :) hmm..The line clearly goes down on the scope after the current pwm duty cycle is done and never comes back. > > Reloading modules for pwm-vibra and pwm-omap-dmtimer make the power > consumption go back down again. > > If you have a scope set up, maybe check the lines are quiet after > disable after this patch? > > Regards, > > Tony > > > [0] https://git.collabora.com/cgit/user/sre/rumble-test.git/plain/rumble-test.c This is redirecting to collabora.com. Is this code available in github or some public repo? Thanks and regards, Lokesh
Hi, On Mon, Mar 09, 2020 at 10:21:59AM +0530, Lokesh Vutla wrote: > > [0] https://git.collabora.com/cgit/user/sre/rumble-test.git/plain/rumble-test.c > > This is redirecting to collabora.com. Is this code available in github or some > public repo? > > Thanks and regards, > Lokesh Sorry, we migrated to gitlab under different domain. I pushed it here now: https://github.com/sre/rumble-test -- Sebastian
* Lokesh Vutla <lokeshvutla@ti.com> [200309 04:53]: > Hi Tony, > > On 06/03/20 11:44 PM, Tony Lindgren wrote: > > * Lokesh Vutla <lokeshvutla@ti.com> [200228 09:58]: > >> pwm_omap_dmtimer_disable() calls .stop callback which abruptly stops the > >> timer counter. This doesn't complete the current pwm cycle and > >> immediately disables the pwm. Instead disable the auto reload > >> functionality which allows to complete the current pwm cycle and then > >> disables the timer. > >> > >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > >> --- > >> drivers/pwm/pwm-omap-dmtimer.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c > >> index bc338619232d..89b3c25d02b8 100644 > >> --- a/drivers/pwm/pwm-omap-dmtimer.c > >> +++ b/drivers/pwm/pwm-omap-dmtimer.c > >> @@ -93,8 +93,16 @@ static void pwm_omap_dmtimer_disable(struct pwm_chip *chip, > >> { > >> struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); > >> > >> + /* > >> + * Disable auto reload so that the current cycle gets completed and > >> + * then the counter stops. > >> + */ > >> mutex_lock(&omap->mutex); > >> - omap->pdata->stop(omap->dm_timer); > >> + omap->pdata->set_pwm(omap->dm_timer, > >> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, > >> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, > >> + false); > >> + > >> mutex_unlock(&omap->mutex); > >> } > > > > I'm seeing an issue with this patch where after use something is > > left on and power consumption stays higher by about 30 mW after > > use. > > Interesting...What is the PWM period and duty cycle in the test case? > Can you dump the following registers before and after disabling: > - TLDR > - TMAR > - TCLR Here's the state dumped before and after in omap_dm_timer_set_pwm(): omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 So looks like the start bit is still enabled after use? I think the duty cycle depends on the strength set for rumble-test.c. > > I can reproduce this easily on droid4 with Sebastian's rumble-test > > app[0]. After use, I sometimes also hear the vibrator keep chirping > > quietly, so there seems to be some pwm still happening after disable :) > > hmm..The line clearly goes down on the scope after the current pwm duty cycle is > done and never comes back. OK Regards, Tony
Hi Tony, [...snip...] >>>> >>>> + /* >>>> + * Disable auto reload so that the current cycle gets completed and >>>> + * then the counter stops. >>>> + */ >>>> mutex_lock(&omap->mutex); >>>> - omap->pdata->stop(omap->dm_timer); >>>> + omap->pdata->set_pwm(omap->dm_timer, >>>> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, >>>> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, >>>> + false); >>>> + >>>> mutex_unlock(&omap->mutex); >>>> } >>> >>> I'm seeing an issue with this patch where after use something is >>> left on and power consumption stays higher by about 30 mW after >>> use. >> >> Interesting...What is the PWM period and duty cycle in the test case? >> Can you dump the following registers before and after disabling: >> - TLDR >> - TMAR >> - TCLR > > Here's the state dumped before and after in omap_dm_timer_set_pwm(): > > omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > Looking at the registers: period = 327 *(1000/clk_freq in MHz) ns duty_cycle = perioid. I did simulate this behavior on BeagleBoneBlack on timer7. PWM is going down after disabling. > So looks like the start bit is still enabled after use? Right, that is expected. The start bit gets disabled automatically once the pwm period completes. This is because auto reload bit is off. That's the main idea of this patch so that PWM period is completed after disabling, else PWM is stopped abruptly. Not sure why it is not happening in your case. If you think it is not needed, I can drop this patch and add a limitation saying that PWM gets disabled immediately without completing the current cycle. Thanks and regards, Lokesh
* Lokesh Vutla <lokeshvutla@ti.com> [200310 07:06]: > Hi Tony, > > [...snip...] > > >>>> > >>>> + /* > >>>> + * Disable auto reload so that the current cycle gets completed and > >>>> + * then the counter stops. > >>>> + */ > >>>> mutex_lock(&omap->mutex); > >>>> - omap->pdata->stop(omap->dm_timer); > >>>> + omap->pdata->set_pwm(omap->dm_timer, > >>>> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, > >>>> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, > >>>> + false); > >>>> + > >>>> mutex_unlock(&omap->mutex); > >>>> } > >>> > >>> I'm seeing an issue with this patch where after use something is > >>> left on and power consumption stays higher by about 30 mW after > >>> use. > >> > >> Interesting...What is the PWM period and duty cycle in the test case? > >> Can you dump the following registers before and after disabling: > >> - TLDR > >> - TMAR > >> - TCLR > > > > Here's the state dumped before and after in omap_dm_timer_set_pwm(): > > > > omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > > omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > > omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > > omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > > omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > > omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > > omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > > omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > > > > Looking at the registers: > period = 327 *(1000/clk_freq in MHz) ns > duty_cycle = perioid. > > I did simulate this behavior on BeagleBoneBlack on timer7. PWM is going down > after disabling. > > > So looks like the start bit is still enabled after use? > > Right, that is expected. The start bit gets disabled automatically once the pwm > period completes. This is because auto reload bit is off. That's the main idea > of this patch so that PWM period is completed after disabling, else PWM is > stopped abruptly. OK > Not sure why it is not happening in your case. If you think it is not needed, I > can drop this patch and add a limitation saying that PWM gets disabled > immediately without completing the current cycle. Could it be that we now have the cpu_pm notifier restore something invalid after exiting idle that restarts the counter? Regards, Tony
On 10/03/20 9:22 PM, Tony Lindgren wrote: > * Lokesh Vutla <lokeshvutla@ti.com> [200310 07:06]: >> Hi Tony, >> >> [...snip...] >> >>>>>> >>>>>> + /* >>>>>> + * Disable auto reload so that the current cycle gets completed and >>>>>> + * then the counter stops. >>>>>> + */ >>>>>> mutex_lock(&omap->mutex); >>>>>> - omap->pdata->stop(omap->dm_timer); >>>>>> + omap->pdata->set_pwm(omap->dm_timer, >>>>>> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, >>>>>> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, >>>>>> + false); >>>>>> + >>>>>> mutex_unlock(&omap->mutex); >>>>>> } >>>>> >>>>> I'm seeing an issue with this patch where after use something is >>>>> left on and power consumption stays higher by about 30 mW after >>>>> use. >>>> >>>> Interesting...What is the PWM period and duty cycle in the test case? >>>> Can you dump the following registers before and after disabling: >>>> - TLDR >>>> - TMAR >>>> - TCLR >>> >>> Here's the state dumped before and after in omap_dm_timer_set_pwm(): >>> >>> omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 >>> omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 >>> omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 >>> omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 >>> omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 >>> omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 >>> omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 >>> omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 >>> >> >> Looking at the registers: >> period = 327 *(1000/clk_freq in MHz) ns >> duty_cycle = perioid. >> >> I did simulate this behavior on BeagleBoneBlack on timer7. PWM is going down >> after disabling. >> >>> So looks like the start bit is still enabled after use? >> >> Right, that is expected. The start bit gets disabled automatically once the pwm >> period completes. This is because auto reload bit is off. That's the main idea >> of this patch so that PWM period is completed after disabling, else PWM is >> stopped abruptly. > > OK > >> Not sure why it is not happening in your case. If you think it is not needed, I >> can drop this patch and add a limitation saying that PWM gets disabled >> immediately without completing the current cycle. > > Could it be that we now have the cpu_pm notifier restore something > invalid after exiting idle that restarts the counter? If that's the case, mis behavior should have happened without this patch as well. Is it possible for you to dump the registers when you are observing higher power consumption after the use? However, I see an issue with the patch itself as pm_runtime is not disabled after the pwm is stopped. Not sure how that could be nullified with this approach. Thanks and regards, Lokesh
* Lokesh Vutla <lokeshvutla@ti.com> [200311 04:14]: > > > On 10/03/20 9:22 PM, Tony Lindgren wrote: > > * Lokesh Vutla <lokeshvutla@ti.com> [200310 07:06]: > >> Hi Tony, > >> > >> [...snip...] > >> > >>>>>> > >>>>>> + /* > >>>>>> + * Disable auto reload so that the current cycle gets completed and > >>>>>> + * then the counter stops. > >>>>>> + */ > >>>>>> mutex_lock(&omap->mutex); > >>>>>> - omap->pdata->stop(omap->dm_timer); > >>>>>> + omap->pdata->set_pwm(omap->dm_timer, > >>>>>> + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, > >>>>>> + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, > >>>>>> + false); > >>>>>> + > >>>>>> mutex_unlock(&omap->mutex); > >>>>>> } > >>>>> > >>>>> I'm seeing an issue with this patch where after use something is > >>>>> left on and power consumption stays higher by about 30 mW after > >>>>> use. > >>>> > >>>> Interesting...What is the PWM period and duty cycle in the test case? > >>>> Can you dump the following registers before and after disabling: > >>>> - TLDR > >>>> - TMAR > >>>> - TCLR > >>> > >>> Here's the state dumped before and after in omap_dm_timer_set_pwm(): > >>> > >>> omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > >>> omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > >>> omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00000040 > >>> omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001842 > >>> omap_timer 4013e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > >>> omap_timer 4013e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > >>> omap_timer 4803e000.timer: XXX set_pwm before: tldr: fffffeb8 tmar: fffffffe tclr: 00001843 > >>> omap_timer 4803e000.timer: XXX set_pwm after: tldr: fffffeb8 tmar: fffffffe tclr: 00001841 > >>> > >> > >> Looking at the registers: > >> period = 327 *(1000/clk_freq in MHz) ns > >> duty_cycle = perioid. > >> > >> I did simulate this behavior on BeagleBoneBlack on timer7. PWM is going down > >> after disabling. > >> > >>> So looks like the start bit is still enabled after use? > >> > >> Right, that is expected. The start bit gets disabled automatically once the pwm > >> period completes. This is because auto reload bit is off. That's the main idea > >> of this patch so that PWM period is completed after disabling, else PWM is > >> stopped abruptly. > > > > OK > > > >> Not sure why it is not happening in your case. If you think it is not needed, I > >> can drop this patch and add a limitation saying that PWM gets disabled > >> immediately without completing the current cycle. > > > > Could it be that we now have the cpu_pm notifier restore something > > invalid after exiting idle that restarts the counter? > > If that's the case, mis behavior should have happened without this patch as well. Hmm but without this patch we stop the timer so enable bit is cleared before we ever save context. I think now we can have enable bit still on, save the context, and keep restoring the enable bit that never has a chance to clear if we do this at a fast enough rate. > Is it possible for you to dump the registers when you are observing higher power > consumption after the use? Well they seem to be the same as in the dump above, here are the regs dumped after use with chirping happening: $ sudo rwmem 0x4803e000+0x60 0x4803e000 = 0x4fff1301 0x4803e004 = 0000000000 0x4803e008 = 0000000000 0x4803e00c = 0000000000 0x4803e010 = 0x0000000c 0x4803e014 = 0000000000 0x4803e018 = 0000000000 0x4803e01c = 0000000000 0x4803e020 = 0000000000 0x4803e024 = 0x00000003 0x4803e028 = 0000000000 0x4803e02c = 0000000000 0x4803e030 = 0000000000 0x4803e034 = 0000000000 0x4803e038 = 0x00001841 0x4803e03c = 0x00000402 0x4803e040 = 0xfffffeb8 0x4803e044 = 0xffffffff 0x4803e048 = 0000000000 0x4803e04c = 0xfffffffe 0x4803e050 = 0000000000 0x4803e054 = 0000000000 0x4803e058 = 0000000000 0x4803e05c = 0000000000 $ sudo rwmem 0x4013e000+0x60 0x4013e000 = 0x4fff0301 0x4013e004 = 0000000000 0x4013e008 = 0000000000 0x4013e00c = 0000000000 0x4013e010 = 0000000000 0x4013e014 = 0000000000 0x4013e018 = 0000000000 0x4013e01c = 0000000000 0x4013e020 = 0000000000 0x4013e024 = 0x00000003 0x4013e028 = 0000000000 0x4013e02c = 0000000000 0x4013e030 = 0000000000 0x4013e034 = 0000000000 0x4013e038 = 0x00001841 0x4013e03c = 0x00000407 0x4013e040 = 0xfffffeb8 0x4013e044 = 0xffffffff 0x4013e048 = 0000000000 0x4013e04c = 0xfffffffe 0x4013e050 = 0000000000 0x4013e054 = 0000000000 0x4013e058 = 0000000000 0x4013e05c = 0000000000 So looks like the enable bit is never cleared now. > However, I see an issue with the patch itself as pm_runtime is not disabled > after the pwm is stopped. Not sure how that could be nullified with this approach. Hmm yeah not sure what could be used to clear things when the current cycle is completed unless there's some interrupt for it. Regards, Tony
* Tony Lindgren <tony@atomide.com> [200312 00:59]: > * Lokesh Vutla <lokeshvutla@ti.com> [200311 04:14]: > > However, I see an issue with the patch itself as pm_runtime is not disabled > > after the pwm is stopped. Not sure how that could be nullified with this approach. > > Hmm yeah not sure what could be used to clear things > when the current cycle is completed unless there's > some interrupt for it. You could enable pm_runtime_use_autosuspend() for pwm use, then set the timeout to the cycle length, then in the runtime_suspend make sure the enable bit is cleared if requested. But this too seems inaccurate, it would be best to clear the enable bit on some cycle completion interrupt if such thing is available. Regards, Tony
* Tony Lindgren <tony@atomide.com> [200313 15:35]: > * Tony Lindgren <tony@atomide.com> [200312 00:59]: > > * Lokesh Vutla <lokeshvutla@ti.com> [200311 04:14]: > > > However, I see an issue with the patch itself as pm_runtime is not disabled > > > after the pwm is stopped. Not sure how that could be nullified with this approach. > > > > Hmm yeah not sure what could be used to clear things > > when the current cycle is completed unless there's > > some interrupt for it. > > You could enable pm_runtime_use_autosuspend() for pwm use, > then set the timeout to the cycle length, then in the > runtime_suspend make sure the enable bit is cleared if > requested. > > But this too seems inaccurate, it would be best to clear > the enable bit on some cycle completion interrupt if > such thing is available. I think enabling pm_runtime_use_autosuspend() for pwm, adding a flag for pwm_enabled, and blocking cpu_pm if pwm_enabled is set might do the trick though. Then clear pwm_enabled flag in runtime_suspend if set. This depend on cpuidle respecting NOTIFY_BAD that I fixed in the recent thread: [PATCH 0/3] Block idle in gpio-omap with cpu_pm Regards, Tony
diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c index bc338619232d..89b3c25d02b8 100644 --- a/drivers/pwm/pwm-omap-dmtimer.c +++ b/drivers/pwm/pwm-omap-dmtimer.c @@ -93,8 +93,16 @@ static void pwm_omap_dmtimer_disable(struct pwm_chip *chip, { struct pwm_omap_dmtimer_chip *omap = to_pwm_omap_dmtimer_chip(chip); + /* + * Disable auto reload so that the current cycle gets completed and + * then the counter stops. + */ mutex_lock(&omap->mutex); - omap->pdata->stop(omap->dm_timer); + omap->pdata->set_pwm(omap->dm_timer, + pwm_get_polarity(pwm) == PWM_POLARITY_INVERSED, + true, OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE, + false); + mutex_unlock(&omap->mutex); }
pwm_omap_dmtimer_disable() calls .stop callback which abruptly stops the timer counter. This doesn't complete the current pwm cycle and immediately disables the pwm. Instead disable the auto reload functionality which allows to complete the current pwm cycle and then disables the timer. Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- drivers/pwm/pwm-omap-dmtimer.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)