On Thu, 13 Dec 2012 11:07:25 -0600 Jon Hunter <jon-hunter@xxxxxx> wrote:
>
> On 12/12/2012 09:06 PM, NeilBrown wrote:
> >
> >>> +
> >>> +#if CONFIG_PM
> >>> +static int omap_pwm_suspend(struct platform_device *pdev, pm_message_t state)
> >>> +{
> >>> + struct omap_chip *omap = platform_get_drvdata(pdev);
> >>> + /* No one preserve these values during suspend so reset them
> >>> + * Otherwise driver leaves PWM unconfigured if same values
> >>> + * passed to pwm_config
> >>> + */
> >>> + omap->period_ns = 0;
> >>> + omap->duty_ns = 0;
> >>
> >>
> >> Hmmm, looks like you are trying to force a reconfiguration after suspend
> >> if the same values are used. Is there an underlying problem here that
> >> you are trying to workaround?
> >
> > I copied that from pwm-samsung.c.
> >
> > The key question is: does a dmtimer preserve all register values over suspend.
> > If so, then I guess we don't need this.
> > If not, we do (because omap_pwm_config short circuits if it thinks the config
> > hasn't changed).
>
> I gave it a quick test on omap3/4 when just operating the timer as a
> counter (not driving a pwm output) and suspend/resume works fine.
> However, it does not work if I enable off mode (via the debugfs). This
> is not enabled by default and may be I should put that on my to-do list
> as well.
I've been playing with off-mode and discovered that the first attempt to set
the PWM after resume didn't work, but subsequent ones did.
I did some digging and came up with the following patch.
With this in place, the omap_pwm_suspend() above is definitely pointless (was
wasn't really useful even without it).
NeilBrown
From: NeilBrown <neilb@xxxxxxx>
Date: Mon, 7 Jan 2013 07:53:03 +1100
Subject: [PATCH] OMAP dmtimer - simplify context-loss handling.
The context loss handling in dmtimer appears to assume that
omap_dm_timer_set_load_start() or omap_dm_timer_start()
and
omap_dm_timer_stop()
bracket all interactions. Only the first two restore the context and
the last updates the context loss counter.
However omap_dm_timer_set_load() or omap_dm_timer_set_match() can
reasonably be called outside this bracketing, and the fact that they
call omap_dm_timer_enable() / omap_dm_timer_disable() suggest that
is expected.
So if, after a transition into and out of off-mode which would cause
the dm timer to loose all state, omap_dm_timer_set_match() is called
before omap_dm_timer_start(), the value read from OMAP_TIMER_CTRL_REG
will be 'wrong' and this wrong value will be stored context.tclr so
a subsequent omap_dm_timer_start() can fail (As the control register
is wrong).
Simplify this be doing the restore-from-context in
omap_dm_timer_enable() so that whenever the timer is enabled, the
context is correct.
Also update the ctx_loss_count at the same time as we notice it is
wrong - these is no value in delaying this until the
omap_dm_timer_disable() as it cannot change while the timer is
enabled.
Signed-off-by: NeilBrown <neilb@xxxxxxx>
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 938b50a..c216696 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -253,6 +253,15 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_free);
void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
pm_runtime_get_sync(&timer->pdev->dev);
+
+ if (!(timer->capability & OMAP_TIMER_ALWON)) {
+ int loss_count =
+ omap_pm_get_dev_context_loss_count(&timer->pdev->dev);
+ if (loss_count != timer->ctx_loss_count) {
+ omap_timer_restore_context(timer);
+ timer->ctx_loss_count = loss_count;
+ }
+ }
}
EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
@@ -347,12 +356,6 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
omap_dm_timer_enable(timer);
- if (!(timer->capability & OMAP_TIMER_ALWON)) {
- if (omap_pm_get_dev_context_loss_count(&timer->pdev->dev) !=
- timer->ctx_loss_count)
- omap_timer_restore_context(timer);
- }
-
l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
if (!(l & OMAP_TIMER_CTRL_ST)) {
l |= OMAP_TIMER_CTRL_ST;
@@ -377,10 +380,6 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
__omap_dm_timer_stop(timer, timer->posted, rate);
- if (!(timer->capability & OMAP_TIMER_ALWON))
- timer->ctx_loss_count =
- omap_pm_get_dev_context_loss_count(&timer->pdev->dev);
-
/*
* Since the register values are computed and written within
* __omap_dm_timer_stop, we need to use read to retrieve the
@@ -494,12 +493,6 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
omap_dm_timer_enable(timer);
- if (!(timer->capability & OMAP_TIMER_ALWON)) {
- if (omap_pm_get_dev_context_loss_count(&timer->pdev->dev) !=
- timer->ctx_loss_count)
- omap_timer_restore_context(timer);
- }
-
l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
if (autoreload) {
l |= OMAP_TIMER_CTRL_AR;
Attachment:
signature.asc
Description: PGP signature
