* [PATCH] clockevents: don't suspend/resume if unused
@ 2015-01-16 9:05 ` Alexandre Belloni
0 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-01-16 9:05 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Daniel Lezcano, Nicolas Ferre, Boris Brezillon, Maxime Ripard,
sylvain.rochet, linux-arm-kernel, linux-kernel, Alexandre Belloni
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.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
kernel/time/clockevents.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 55449909f114..0eff982c1c11 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -525,7 +525,7 @@ void clockevents_suspend(void)
struct clock_event_device *dev;
list_for_each_entry_reverse(dev, &clockevent_devices, list)
- if (dev->suspend)
+ if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
dev->suspend(dev);
}
@@ -537,7 +537,7 @@ void clockevents_resume(void)
struct clock_event_device *dev;
list_for_each_entry(dev, &clockevent_devices, list)
- if (dev->resume)
+ if (dev->resume && dev->mode != CLOCK_EVT_MODE_UNUSED)
dev->resume(dev);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 9:05 ` Alexandre Belloni
@ 2015-01-16 9:20 ` Sylvain Rochet
-1 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-01-16 9:20 UTC (permalink / raw)
To: linux-arm-kernel
Hello Alexandre,
On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Indeed, this is way better from what I did.
> + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
(or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
!CLOCK_EVT_MODE_UNUSED.
Sylvain
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-01-16 9:20 ` Sylvain Rochet
0 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-01-16 9:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Thomas Gleixner, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
Hello Alexandre,
On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Indeed, this is way better from what I did.
> + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
(or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
!CLOCK_EVT_MODE_UNUSED.
Sylvain
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 9:20 ` Sylvain Rochet
@ 2015-01-16 11:17 ` Russell King - ARM Linux
-1 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2015-01-16 11:17 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 16, 2015 at 10:20:14AM +0100, Sylvain Rochet wrote:
> Hello Alexandre,
>
>
> On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
> >
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
>
> Indeed, this is way better from what I did.
>
>
> > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
>
> I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> !CLOCK_EVT_MODE_UNUSED.
Definitely - consider the effect of the original patch set on a clock
source which is being used, has PM support, but does not have an
->enable callback.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-01-16 11:17 ` Russell King - ARM Linux
0 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2015-01-16 11:17 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Alexandre Belloni, Boris Brezillon, Daniel Lezcano, Nicolas Ferre,
linux-kernel, Thomas Gleixner, Maxime Ripard, linux-arm-kernel
On Fri, Jan 16, 2015 at 10:20:14AM +0100, Sylvain Rochet wrote:
> Hello Alexandre,
>
>
> On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
> >
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
>
> Indeed, this is way better from what I did.
>
>
> > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
>
> I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> !CLOCK_EVT_MODE_UNUSED.
Definitely - consider the effect of the original patch set on a clock
source which is being used, has PM support, but does not have an
->enable callback.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 11:17 ` Russell King - ARM Linux
@ 2015-01-16 11:20 ` Russell King - ARM Linux
-1 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2015-01-16 11:20 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 16, 2015 at 11:17:06AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 16, 2015 at 10:20:14AM +0100, Sylvain Rochet wrote:
> > Hello Alexandre,
> >
> >
> > On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
> > >
> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> >
> > Indeed, this is way better from what I did.
> >
> >
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
>
> Definitely - consider the effect of the original patch set on a clock
> source which is being used, has PM support, but does not have an
> ->enable callback.
Damn it, that comment should've been on the clocksource patches...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-01-16 11:20 ` Russell King - ARM Linux
0 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2015-01-16 11:20 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Boris Brezillon, Daniel Lezcano, Nicolas Ferre, linux-kernel,
Alexandre Belloni, Thomas Gleixner, Maxime Ripard,
linux-arm-kernel
On Fri, Jan 16, 2015 at 11:17:06AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 16, 2015 at 10:20:14AM +0100, Sylvain Rochet wrote:
> > Hello Alexandre,
> >
> >
> > On Fri, Jan 16, 2015 at 10:05:51AM +0100, 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.
> > >
> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> >
> > Indeed, this is way better from what I did.
> >
> >
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
>
> Definitely - consider the effect of the original patch set on a clock
> source which is being used, has PM support, but does not have an
> ->enable callback.
Damn it, that comment should've been on the clocksource patches...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 9:20 ` Sylvain Rochet
@ 2015-01-16 16:59 ` Alexandre Belloni
-1 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-01-16 16:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
>
> I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> !CLOCK_EVT_MODE_UNUSED.
>
I'll let Thomas or Daniel decide :) But I'm open to that.
>--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-01-16 16:59 ` Alexandre Belloni
0 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-01-16 16:59 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Thomas Gleixner, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
Hi,
On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
>
> I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> !CLOCK_EVT_MODE_UNUSED.
>
I'll let Thomas or Daniel decide :) But I'm open to that.
>--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 16:59 ` Alexandre Belloni
@ 2015-03-06 12:54 ` Alexandre Belloni
-1 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-03-06 12:54 UTC (permalink / raw)
To: linux-arm-kernel
Ping?
On 16/01/2015 at 17:59:14 +0100, Alexandre Belloni wrote :
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
>
> >--
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-03-06 12:54 ` Alexandre Belloni
0 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-03-06 12:54 UTC (permalink / raw)
To: Thomas Gleixner, Daniel Lezcano
Cc: Sylvain Rochet, Nicolas Ferre, Boris Brezillon, Maxime Ripard,
linux-arm-kernel, linux-kernel
Ping?
On 16/01/2015 at 17:59:14 +0100, Alexandre Belloni wrote :
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
>
> >--
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 16:59 ` Alexandre Belloni
@ 2015-05-25 16:19 ` Alexandre Belloni
-1 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-05-25 16:19 UTC (permalink / raw)
To: linux-arm-kernel
Thomas, Daniel,
Any opinion on that?
Maybe this patch could go in as is?
On 16/01/2015 at 17:59:14 +0100, Alexandre Belloni wrote :
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
>
> >--
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 16:19 ` Alexandre Belloni
0 siblings, 0 replies; 25+ messages in thread
From: Alexandre Belloni @ 2015-05-25 16:19 UTC (permalink / raw)
To: Thomas Gleixner, Daniel Lezcano
Cc: Boris Brezillon, Sylvain Rochet, Nicolas Ferre, linux-kernel,
Maxime Ripard, linux-arm-kernel
Thomas, Daniel,
Any opinion on that?
Maybe this patch could go in as is?
On 16/01/2015 at 17:59:14 +0100, Alexandre Belloni wrote :
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
>
> >--
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 16:59 ` Alexandre Belloni
@ 2015-05-25 16:56 ` Thomas Gleixner
-1 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 16:56 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Jan 2015, Alexandre Belloni wrote:
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
I'm unsure about the shutdown part. shutdown can be an intermediate
state where its not a given, that the clocks are disabled. I pick it
up as is.
Thanks,
tglx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 16:56 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 16:56 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Sylvain Rochet, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
On Fri, 16 Jan 2015, Alexandre Belloni wrote:
> Hi,
>
> On 16/01/2015 at 10:20:14 +0100, Sylvain Rochet wrote :
> > > + if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
> >
> > I wonder if we should use > CLOCK_EVT_MODE_SHUTDOWN
> > (or CLOCK_EVT_MODE_UNUSED || CLOCK_EVT_MODE_SHUTDOWN) instead of
> > !CLOCK_EVT_MODE_UNUSED.
> >
>
> I'll let Thomas or Daniel decide :) But I'm open to that.
I'm unsure about the shutdown part. shutdown can be an intermediate
state where its not a given, that the clocks are disabled. I pick it
up as is.
Thanks,
tglx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-01-16 9:05 ` Alexandre Belloni
@ 2015-05-25 18:48 ` Thomas Gleixner
-1 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 18:48 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 18:48 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 18:48 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Daniel Lezcano, Nicolas Ferre, Boris Brezillon, Maxime Ripard,
sylvain.rochet, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-05-25 18:48 ` Thomas Gleixner
@ 2015-05-25 19:06 ` Sylvain Rochet
-1 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-05-25 19:06 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 19:06 ` Sylvain Rochet
0 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-05-25 19:06 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Alexandre Belloni, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-05-25 19:06 ` Sylvain Rochet
@ 2015-05-25 20:11 ` Thomas Gleixner
-1 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 20:11 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 20:11 ` Thomas Gleixner
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2015-05-25 20:11 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Alexandre Belloni, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] clockevents: don't suspend/resume if unused
2015-05-25 20:11 ` Thomas Gleixner
@ 2015-05-25 20:53 ` Sylvain Rochet
-1 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-05-25 20:53 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] clockevents: don't suspend/resume if unused
@ 2015-05-25 20:53 ` Sylvain Rochet
0 siblings, 0 replies; 25+ messages in thread
From: Sylvain Rochet @ 2015-05-25 20:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Alexandre Belloni, Daniel Lezcano, Nicolas Ferre, Boris Brezillon,
Maxime Ripard, linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:timers/core] clockevents: Do not suspend/resume if unused
2015-01-16 9:05 ` Alexandre Belloni
` (2 preceding siblings ...)
(?)
@ 2015-05-26 23:07 ` tip-bot for Alexandre Belloni
-1 siblings, 0 replies; 25+ messages in thread
From: tip-bot for Alexandre Belloni @ 2015-05-26 23:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, alexandre.belloni, nicolas.ferre, boris.brezillon,
maxime.ripard, linux-kernel, daniel.lezcano, mingo, tglx,
sylvain.rochet
Commit-ID: ac34ad27fc160b5bd31c731cdaaf6e1d1890ccb2
Gitweb: http://git.kernel.org/tip/ac34ad27fc160b5bd31c731cdaaf6e1d1890ccb2
Author: Alexandre Belloni <alexandre.belloni@free-electrons.com>
AuthorDate: Fri, 16 Jan 2015 10:05:51 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 27 May 2015 01:03:38 +0200
clockevents: Do not suspend/resume if unused
There is no point in calling suspend/resume for unused clockevents as
they are already stopped and disabled.
This is really important for AT91 as the hardware is a trainwreck and
takes ages to synchronize.
Reported-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1421399151-26800-1-git-send-email-alexandre.belloni@free-electrons.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/time/clockevents.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 4922f1b..2a5c369 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -638,7 +638,7 @@ void clockevents_suspend(void)
struct clock_event_device *dev;
list_for_each_entry_reverse(dev, &clockevent_devices, list)
- if (dev->suspend)
+ if (dev->suspend && dev->mode != CLOCK_EVT_MODE_UNUSED)
dev->suspend(dev);
}
@@ -650,7 +650,7 @@ void clockevents_resume(void)
struct clock_event_device *dev;
list_for_each_entry(dev, &clockevent_devices, list)
- if (dev->resume)
+ if (dev->resume && dev->mode != CLOCK_EVT_MODE_UNUSED)
dev->resume(dev);
}
^ permalink raw reply related [flat|nested] 25+ messages in thread