linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend
       [not found] <20161122094300.GA2017@mai>
@ 2016-11-22  9:44 ` Daniel Lezcano
  2016-11-22  9:44   ` [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend Daniel Lezcano
                     ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Daniel Lezcano @ 2016-11-22  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Brian Norris <briannorris@chromium.org>

The ARM specifies that the system counter "must be implemented in an
always-on power domain," and so we try to use the counter as a source of
timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
Rockchip's RK3399) do not keep the counter ticking properly when
switched from their high-power clock to the lower-power clock used in
system suspend. Support this quirk by adding a new device tree property.

Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++
 drivers/clocksource/arm_arch_timer.c                 | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index ef5fbe9..ad440a2 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -38,6 +38,11 @@ to deliver its interrupts via SPIs.
   architecturally-defined reset values. Only supported for 32-bit
   systems which follow the ARMv7 architected reset values.
 
+- arm,no-tick-in-suspend : The main counter does not tick when the system is in
+  low-power system suspend on some SoCs. This behavior does not match the
+  Architecture Reference Manual's specification that the system counter "must
+  be implemented in an always-on power domain."
+
 
 Example:
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 73c487d..a2503db 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -81,6 +81,7 @@ static struct clock_event_device __percpu *arch_timer_evt;
 static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
 static bool arch_timer_c3stop;
 static bool arch_timer_mem_use_virtual;
+static bool arch_counter_suspend_stop;
 
 static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
 
@@ -576,7 +577,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 = {
@@ -616,6 +617,8 @@ static void __init arch_counter_register(unsigned type)
 		arch_timer_read_counter = arch_counter_get_cntvct_mem;
 	}
 
+	if (!arch_counter_suspend_stop)
+		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;
@@ -907,6 +910,10 @@ static int __init arch_timer_of_init(struct device_node *np)
 	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
 		arch_timer_uses_ppi = PHYS_SECURE_PPI;
 
+	/* On some systems, the counter stops ticking when in suspend. */
+	arch_counter_suspend_stop = of_property_read_bool(np,
+							 "arm,no-tick-in-suspend");
+
 	return arch_timer_init();
 }
 CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend
  2016-11-22  9:44 ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Daniel Lezcano
@ 2016-11-22  9:44   ` Daniel Lezcano
  2016-11-22 15:02     ` Heiko Stübner
  2016-11-22  9:44   ` [PATCH 3/4] clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map() Daniel Lezcano
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2016-11-22  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Brian Norris <briannorris@chromium.org>

The "arm,no-tick-in-suspend" property was introduced to note
implementations where the system counter does not quite follow the ARM
specification that it "must be implemented in an always-on power
domain".

Particularly, RK3399's counter stops ticking when we switch from the
24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
such.

Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b65c193..d85b651 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -174,6 +174,7 @@
 			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
 			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
 			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
+		arm,no-tick-in-suspend;
 	};
 
 	xin24m: xin24m {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map()
  2016-11-22  9:44 ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Daniel Lezcano
  2016-11-22  9:44   ` [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend Daniel Lezcano
@ 2016-11-22  9:44   ` Daniel Lezcano
  2016-11-22  9:44   ` [PATCH 4/4] clocksource/drivers/bcm2835_timer: Unmap region obtained by of_iomap Daniel Lezcano
  2016-11-23 23:07   ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Rob Herring
  3 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2016-11-22  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Boyd <sboyd@codeaurora.org>

Let's use the of_io_request_and_map() API so that the frame
region is protected and shows up in /proc/iomem.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index a2503db..02fef68 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -971,8 +971,9 @@ static int __init arch_timer_mem_init(struct device_node *np)
 	}
 
 	ret= -ENXIO;
-	base = arch_counter_base = of_iomap(best_frame, 0);
-	if (!base) {
+	base = arch_counter_base = of_io_request_and_map(best_frame, 0,
+							 "arch_mem_timer");
+	if (IS_ERR(base)) {
 		pr_err("arch_timer: Can't map frame's registers\n");
 		goto out;
 	}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] clocksource/drivers/bcm2835_timer: Unmap region obtained by of_iomap
  2016-11-22  9:44 ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Daniel Lezcano
  2016-11-22  9:44   ` [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend Daniel Lezcano
  2016-11-22  9:44   ` [PATCH 3/4] clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map() Daniel Lezcano
@ 2016-11-22  9:44   ` Daniel Lezcano
  2016-11-23 23:07   ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Rob Herring
  3 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2016-11-22  9:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Arvind Yadav <arvind.yadav.cs@gmail.com>

Free memory mapping, if bcm2835_timer_init is not successful.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/bcm2835_timer.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c
index e71acf2..f2f29d2 100644
--- a/drivers/clocksource/bcm2835_timer.c
+++ b/drivers/clocksource/bcm2835_timer.c
@@ -96,7 +96,7 @@ static int __init bcm2835_timer_init(struct device_node *node)
 	ret = of_property_read_u32(node, "clock-frequency", &freq);
 	if (ret) {
 		pr_err("Can't read clock-frequency");
-		return ret;
+		goto err_iounmap;
 	}
 
 	system_clock = base + REG_COUNTER_LO;
@@ -108,13 +108,15 @@ static int __init bcm2835_timer_init(struct device_node *node)
 	irq = irq_of_parse_and_map(node, DEFAULT_TIMER);
 	if (irq <= 0) {
 		pr_err("Can't parse IRQ");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_iounmap;
 	}
 
 	timer = kzalloc(sizeof(*timer), GFP_KERNEL);
 	if (!timer) {
 		pr_err("Can't allocate timer struct\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_iounmap;
 	}
 
 	timer->control = base + REG_CONTROL;
@@ -133,7 +135,7 @@ static int __init bcm2835_timer_init(struct device_node *node)
 	ret = setup_irq(irq, &timer->act);
 	if (ret) {
 		pr_err("Can't set up timer IRQ\n");
-		return ret;
+		goto err_iounmap;
 	}
 
 	clockevents_config_and_register(&timer->evt, freq, 0xf, 0xffffffff);
@@ -141,6 +143,10 @@ static int __init bcm2835_timer_init(struct device_node *node)
 	pr_info("bcm2835: system timer (irq = %d)\n", irq);
 
 	return 0;
+
+err_iounmap:
+	iounmap(base);
+	return ret;
 }
 CLOCKSOURCE_OF_DECLARE(bcm2835, "brcm,bcm2835-system-timer",
 			bcm2835_timer_init);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend
  2016-11-22  9:44   ` [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend Daniel Lezcano
@ 2016-11-22 15:02     ` Heiko Stübner
  0 siblings, 0 replies; 6+ messages in thread
From: Heiko Stübner @ 2016-11-22 15:02 UTC (permalink / raw)
  To: linux-arm-kernel

Am Dienstag, 22. November 2016, 10:44:22 schrieb Daniel Lezcano:
> From: Brian Norris <briannorris@chromium.org>
> 
> The "arm,no-tick-in-suspend" property was introduced to note
> implementations where the system counter does not quite follow the ARM
> specification that it "must be implemented in an always-on power
> domain".
> 
> Particularly, RK3399's counter stops ticking when we switch from the
> 24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
> such.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

not sure if needed, but anyway
Acked-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index b65c193..d85b651 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -174,6 +174,7 @@
>  			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
>  			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
>  			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
> +		arm,no-tick-in-suspend;
>  	};
> 
>  	xin24m: xin24m {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend
  2016-11-22  9:44 ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Daniel Lezcano
                     ` (2 preceding siblings ...)
  2016-11-22  9:44   ` [PATCH 4/4] clocksource/drivers/bcm2835_timer: Unmap region obtained by of_iomap Daniel Lezcano
@ 2016-11-23 23:07   ` Rob Herring
  3 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2016-11-23 23:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 22, 2016 at 10:44:21AM +0100, Daniel Lezcano wrote:
> From: Brian Norris <briannorris@chromium.org>
> 
> The ARM specifies that the system counter "must be implemented in an
> always-on power domain," and so we try to use the counter as a source of
> timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
> Rockchip's RK3399) do not keep the counter ticking properly when
> switched from their high-power clock to the lower-power clock used in
> system suspend. Support this quirk by adding a new device tree property.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/clocksource/arm_arch_timer.c                 | 9 ++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-11-23 23:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20161122094300.GA2017@mai>
2016-11-22  9:44 ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Daniel Lezcano
2016-11-22  9:44   ` [PATCH 2/4] arm64: dts: rockchip: Arch counter doesn't tick in system suspend Daniel Lezcano
2016-11-22 15:02     ` Heiko Stübner
2016-11-22  9:44   ` [PATCH 3/4] clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map() Daniel Lezcano
2016-11-22  9:44   ` [PATCH 4/4] clocksource/drivers/bcm2835_timer: Unmap region obtained by of_iomap Daniel Lezcano
2016-11-23 23:07   ` [PATCH 1/4] clocksource/drivers/arm_arch_timer: Don't assume clock runs in suspend Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).