On Fri, 16 Jan 2015, Alexandre Belloni wrote:
> There is no point in calling suspend/resume for unused
> clockevents as they are already stopped and disabled.
>
> Furthermore, it can take some time to wait for some IPs to stop counting.
While I agree with the patch itself, I really can't understand that
last sentence.
If stuff is stopped and disabled, what takes time to stop counting?
Thanks,
tglx
Hello Thomas,
On Mon, May 25, 2015 at 08:48:06PM +0200, Thomas Gleixner wrote:
> On Fri, 16 Jan 2015, Alexandre Belloni wrote:
>
> > There is no point in calling suspend/resume for unused
> > clockevents as they are already stopped and disabled.
> >
> > Furthermore, it can take some time to wait for some IPs to stop counting.
>
> While I agree with the patch itself, I really can't understand that
> last sentence.
>
> If stuff is stopped and disabled, what takes time to stop counting?
Atmel PIT is a bit weird, writing to AT91_PIT_MR restarts the timer even
if you just want to stop it and then the only way to stop the timer is
to wait for a complete timer cycle.
The problem is not when suspending, restarting the timer just before
suspending is not such a problem because is will eventually stop at
some point in the future.
However it can takes a very long time if the system switchs to slow
clock, therefore when resuming the timer is still running and we have to
wait for the PIT to stop counting because we re-enabled it for one cycle
when suspending, which is weird, it adds about ~128ms resumt time for
Atmel SoC.
The previous proposed patch was to do nothing in PIT suspend and resume
callbacks if PIT is unused[1], which fixed the PIT problem, but we
decided to put the condition in the upper level because other drivers
might be interested too.
Sylvain
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311496.html
On Mon, 25 May 2015, Sylvain Rochet wrote:
> On Mon, May 25, 2015 at 08:48:06PM +0200, Thomas Gleixner wrote:
> > On Fri, 16 Jan 2015, Alexandre Belloni wrote:
> >
> > > There is no point in calling suspend/resume for unused
> > > clockevents as they are already stopped and disabled.
> > >
> > > Furthermore, it can take some time to wait for some IPs to stop counting.
> >
> > While I agree with the patch itself, I really can't understand that
> > last sentence.
> >
> > If stuff is stopped and disabled, what takes time to stop counting?
>
> Atmel PIT is a bit weird, writing to AT91_PIT_MR restarts the timer even
> if you just want to stop it and then the only way to stop the timer is
> to wait for a complete timer cycle.
>
> The problem is not when suspending, restarting the timer just before
> suspending is not such a problem because is will eventually stop at
> some point in the future.
>
> However it can takes a very long time if the system switchs to slow
> clock, therefore when resuming the timer is still running and we have to
> wait for the PIT to stop counting because we re-enabled it for one cycle
> when suspending, which is weird, it adds about ~128ms resumt time for
> Atmel SoC.
That's a reasonable explanation.
While timer IPs seem to be designed by janitors in general, this one
has an extraordinary level of stupidity.
Thanks,
tglx
Hello Thomas,
On Mon, May 25, 2015 at 10:11:15PM +0200, Thomas Gleixner wrote:
>
> That's a reasonable explanation.
>
> While timer IPs seem to be designed by janitors in general, this one
> has an extraordinary level of stupidity.
Yes, that's quite a stupid design, however I wasn't totally right from
looking at the datasheet again, the timer does not restart when asked to
stop while it is already stopped, my bad. But it doesn't change
anything, the issue is still that this timer cannot be stopped right
now, you have to ask for it to stop then wait the overflow.
1. suspend PIT[1] -> ask to stop
2. suspended, timer is still counting and will eventually reaches the
overflow condition then stop, that's probably not going to happen if
system switched to slow clock
3. resume PIT[2] -> wait for PIT to stop counting (up to 126ms) because
we need it to be synched back if we want to use it at some point in the
future, that's also the only way to clear pending interrupts.
Sylvain
[1] http://lxr.free-electrons.com/source/drivers/clocksource/timer-atmel-pit.c#L122
[2] http://lxr.free-electrons.com/source/drivers/clocksource/timer-atmel-pit.c#L130