All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	<linux-rockchip@lists.infradead.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Brian Norris <briannorris@chromium.org>
Subject: [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend
Date: Fri, 16 Sep 2016 13:49:17 +0800	[thread overview]
Message-ID: <20160916054917.16930-1-briannorris@chromium.org> (raw)

Since commit 4fbcdc813fb9 ("clocksource: arm_arch_timer: Use clocksource
for suspend timekeeping"), this driver assumes that the ARM architected
timer keeps running in suspend. This is not the case for some ARM SoCs,
depending on the HW state used for system suspend. Let's not assume that
all SoCs support this, and instead only support this if the device tree
explicitly tells us it's "always on". In all other cases, just fall back
to the RTC. This should be relatively harmless.

It seems fair to key the system-suspend behavior off the same property
used for C3STOP, since if the timer doesn't keep context for CPU sleep,
it likely doesn't for system sleep either.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/clocksource/arm_arch_timer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 57700541f951..e28677a34f02 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -490,7 +490,7 @@ static struct clocksource clocksource_counter = {
 	.rating	= 400,
 	.read	= arch_counter_read,
 	.mask	= CLOCKSOURCE_MASK(56),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static struct cyclecounter cyclecounter = {
@@ -526,6 +526,8 @@ static void __init arch_counter_register(unsigned type)
 		clocksource_counter.name = "arch_mem_counter";
 	}
 
+	if (!arch_timer_c3stop)
+		clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
 	start_count = arch_timer_read_counter();
 	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
 	cyclecounter.mult = clocksource_counter.mult;
-- 
2.8.1.340.g018a5d0.dirty

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris@chromium.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	linux-rockchip@lists.infradead.org,
	Marc Zyngier <marc.zyngier@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Brian Norris <briannorris@chromium.org>
Subject: [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend
Date: Fri, 16 Sep 2016 13:49:17 +0800	[thread overview]
Message-ID: <20160916054917.16930-1-briannorris@chromium.org> (raw)

Since commit 4fbcdc813fb9 ("clocksource: arm_arch_timer: Use clocksource
for suspend timekeeping"), this driver assumes that the ARM architected
timer keeps running in suspend. This is not the case for some ARM SoCs,
depending on the HW state used for system suspend. Let's not assume that
all SoCs support this, and instead only support this if the device tree
explicitly tells us it's "always on". In all other cases, just fall back
to the RTC. This should be relatively harmless.

It seems fair to key the system-suspend behavior off the same property
used for C3STOP, since if the timer doesn't keep context for CPU sleep,
it likely doesn't for system sleep either.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/clocksource/arm_arch_timer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 57700541f951..e28677a34f02 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -490,7 +490,7 @@ static struct clocksource clocksource_counter = {
 	.rating	= 400,
 	.read	= arch_counter_read,
 	.mask	= CLOCKSOURCE_MASK(56),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static struct cyclecounter cyclecounter = {
@@ -526,6 +526,8 @@ static void __init arch_counter_register(unsigned type)
 		clocksource_counter.name = "arch_mem_counter";
 	}
 
+	if (!arch_timer_c3stop)
+		clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
 	start_count = arch_timer_read_counter();
 	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
 	cyclecounter.mult = clocksource_counter.mult;
-- 
2.8.1.340.g018a5d0.dirty

WARNING: multiple messages have this Message-ID (diff)
From: briannorris@chromium.org (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend
Date: Fri, 16 Sep 2016 13:49:17 +0800	[thread overview]
Message-ID: <20160916054917.16930-1-briannorris@chromium.org> (raw)

Since commit 4fbcdc813fb9 ("clocksource: arm_arch_timer: Use clocksource
for suspend timekeeping"), this driver assumes that the ARM architected
timer keeps running in suspend. This is not the case for some ARM SoCs,
depending on the HW state used for system suspend. Let's not assume that
all SoCs support this, and instead only support this if the device tree
explicitly tells us it's "always on". In all other cases, just fall back
to the RTC. This should be relatively harmless.

It seems fair to key the system-suspend behavior off the same property
used for C3STOP, since if the timer doesn't keep context for CPU sleep,
it likely doesn't for system sleep either.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/clocksource/arm_arch_timer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 57700541f951..e28677a34f02 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -490,7 +490,7 @@ static struct clocksource clocksource_counter = {
 	.rating	= 400,
 	.read	= arch_counter_read,
 	.mask	= CLOCKSOURCE_MASK(56),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static struct cyclecounter cyclecounter = {
@@ -526,6 +526,8 @@ static void __init arch_counter_register(unsigned type)
 		clocksource_counter.name = "arch_mem_counter";
 	}
 
+	if (!arch_timer_c3stop)
+		clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
 	start_count = arch_timer_read_counter();
 	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
 	cyclecounter.mult = clocksource_counter.mult;
-- 
2.8.1.340.g018a5d0.dirty

             reply	other threads:[~2016-09-16  5:49 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-16  5:49 Brian Norris [this message]
2016-09-16  5:49 ` [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend Brian Norris
2016-09-16  5:49 ` Brian Norris
2016-09-16  8:06 ` Marc Zyngier
2016-09-16  8:06   ` Marc Zyngier
     [not found]   ` <57DBA81F.2060404-5wv7dgnIgG8@public.gmane.org>
2016-09-16  8:10     ` Daniel Lezcano
2016-09-16  8:10       ` Daniel Lezcano
2016-09-19 23:14   ` Brian Norris
2016-09-19 23:14     ` Brian Norris
2016-09-20  7:47     ` Marc Zyngier
2016-09-20  7:47       ` Marc Zyngier
2016-09-28  1:23       ` Brian Norris
2016-09-28  1:23         ` Brian Norris
2016-09-29 16:08         ` Marc Zyngier
2016-09-29 16:08           ` Marc Zyngier
2016-10-04 17:49           ` Brian Norris
2016-10-04 17:49             ` Brian Norris
2016-10-19  1:24             ` Stephen Boyd
2016-10-19  1:24               ` Stephen Boyd
2016-10-19  1:36               ` Brian Norris
2016-10-19  1:36                 ` Brian Norris
2016-10-19  1:36                 ` Brian Norris
2016-10-19  1:55                 ` Stephen Boyd
2016-10-19  1:55                   ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160916054917.16930-1-briannorris@chromium.org \
    --to=briannorris@chromium.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=sboyd@codeaurora.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.