Skip to contentSkip to conversation

clocksource: add rockchip rk3288 specific armv7-timer

PATCH In review Heiko Stübner 3 participants 8 messages Jun 20, 2014 – Jun 23, 2014

Subsystems (4)
lore
Share & tools
lore

Heiko Stübner

author maintainer
#0 lore raw

cover clocksource: add rockchip rk3288 specific armv7-timer

The architected timer on Rockchip's RK3288 SoCs needs a bit of special handling to start.

This seems to be similar to a recent discussion about the Exynos arch-timer that also seems to need similar special handling (and mct timer running).

And, the arch-timer on recent Mediatek SoCs also uses this pattern of a supplying timer below the arch-timer.

Therefore follow a suggestion from Arnd Bergmann and implement a separate clocksource for it that does the setup and then initializes the arch-timer.

This series uses "of: Provide function to request and map memory" from Matthias Brugger.

Heiko Stuebner (2):
clocksource: arm_arch_timer: remove static from arch_timer_init
clocksource: add rockchip-specific armv7-timer setup

 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/arm_arch_timer.c               |  2 +-
 drivers/clocksource/arm_arch_timer.h               |  7 +++
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 5 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/arm_arch_timer.h
 create mode 100644 drivers/clocksource/rockchip_timer.c
Signature · 1 line
1.9.0
author maintainer
#1 lore raw

PATCH 1/2 clocksource: arm_arch_timer: remove static from arch_timer_init

There exist SoCs that need some special setup procedure for their architected-timer to work. Therefore allow arch_timer_init to be called from such specialized clocksources.

---
 drivers/clocksource/arm_arch_timer.c | 2 +-
 drivers/clocksource/arm_arch_timer.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clocksource/arm_arch_timer.h
drivers/clocksource/arm_arch_timer.c +1 -1
index 5163ec1..55116d6 100644
@@ -635,7 +635,7 @@ static void __init arch_timer_common_init(void)
 	arch_timer_arch_init();
 }
 
-static void __init arch_timer_init(struct device_node *np)
+void __init arch_timer_init(struct device_node *np)
 {
 	int i;
 
drivers/clocksource/arm_arch_timer.h new file +7 -0
new file mode 100644
index 0000000..af3974d
@@ -0,0 +1,7 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+void __init arch_timer_init(struct device_node *np);
Signature · 1 line
1.9.0
maintainer
#2 lore raw

Re: [PATCH 1/2] clocksource: arm_arch_timer: remove static from arch_timer_init

Quoted 30 lines · Heiko St?bner

There exist SoCs that need some special setup procedure for their architected-timer to work. Therefore allow arch_timer_init to be called from such specialized clocksources.

---
 drivers/clocksource/arm_arch_timer.c | 2 +-
 drivers/clocksource/arm_arch_timer.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clocksource/arm_arch_timer.h
drivers/clocksource/arm_arch_timer.c +1 -1
index 5163ec1..55116d6 100644
@@ -635,7 +635,7 @@ static void __init arch_timer_common_init(void)
 	arch_timer_arch_init();
 }
 
-static void __init arch_timer_init(struct device_node *np)
+void __init arch_timer_init(struct device_node *np)
 {
 	int i;
 
drivers/clocksource/arm_arch_timer.h new file +0 -0
new file mode 100644
index 0000000..af3974d

drivers/clocksource/arm_arch_timer.h line 7

Quoted 7 lines
drivers/clocksource/arm_arch_timer.h new file +7 -0
@@ -0,0 +1,7 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+void __init arch_timer_init(struct device_node *np);

If we _really_ need this (which I hope we don't), we already have include/clocksource/arm_arch_timer.h.

Mark.

author maintainer
#3 lore raw

PATCH 2/2 clocksource: add rockchip-specific armv7-timer setup

In reply to Heiko Stübner

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run. Therefore the special rockchip,rk3288-armv7-timer does this setup and then initializes the architected timer using the new locally exposed arch_timer_init.

---
 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/rockchip_timer.c
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +22 -0
new file mode 100644
index 0000000..4c1950a
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
+Example:
+
+architected-timer {
+	compatible = "rockchip,rk3288-armv7-timer";
+	reg = <0xff810020 0x20>;
+	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+	clock-frequency = <24000000>;
+};
drivers/clocksource/Makefile +1 -0
index 800b130..cbad225 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
+obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
 obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
 obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
 obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
drivers/clocksource/rockchip_timer.c new file +57 -0

57 lines not on this page · raw message

Signature · 1 line
1.9.0
maintainer
#4 lore raw

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

Quoted 103 lines · Heiko St?bner

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run. Therefore the special rockchip,rk3288-armv7-timer does this setup and then initializes the architected timer using the new locally exposed arch_timer_init.

---
 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/rockchip_timer.c
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +22 -0
new file mode 100644
index 0000000..4c1950a
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
+Example:
+
+architected-timer {
+	compatible = "rockchip,rk3288-armv7-timer";
+	reg = <0xff810020 0x20>;
+	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+	clock-frequency = <24000000>;
+};
drivers/clocksource/Makefile +1 -0
index 800b130..cbad225 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
+obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
 obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
 obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
 obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
drivers/clocksource/rockchip_timer.c new file +42 -0
new file mode 100644
index 0000000..46c2146
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Heiko Stuebner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include "arm_arch_timer.h"
+
+#define TIMER_LOAD_COUNT0		0x00
+#define TIMER_LOAD_COUNT1		0x04
+#define TIMER_CURRENT_VALUE0		0x08
+#define TIMER_CURRENT_VALUE1		0x0c
+#define TIMER_CONTROL_REG		0x10
+#define TIMER_INT_STATUS		0x18
+
+#define TIMER_DISABLE			(0 << 0)
+#define TIMER_ENABLE			(1 << 0)
+#define TIMER_MODE_FREE_RUNNING		(0 << 1)
+#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
+#define TIMER_INT_MASK			(0 << 2)
+#define TIMER_INT_UNMASK		(1 << 2)
+
+static __init void rk3288_arch_timer_init(struct device_node *np)
+{
+	void __iomem *reg_base;
+
+	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
+	if (!reg_base) {
+		pr_warn("%s: Can't get resource\n", __func__);

drivers/clocksource/rockchip_timer.c line 52

Quoted 10 lines
drivers/clocksource/rockchip_timer.c new file +10 -0
+		return;
+	}
+
+	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
+
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
+
+	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
+	       reg_base + TIMER_CONTROL_REG);

Given that this is unconditionally set up in this way, could this not be done in the fimrware/bootloader?

That way you could also setup CNTFREQ correctly, and virtualisation can just work out of the box.

This looks like a clock driver in disguise...

Mark.

author maintainer
#5 lore raw

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

Am Freitag, 20. Juni 2014, 11:51:23 schrieb Mark Rutland:

Quoted 38 lines · Heiko Stübner
Quoted 37 lines

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run. Therefore the special rockchip,rk3288-armv7-timer does this setup and then initializes the architected timer using the new locally exposed arch_timer_init.

---

 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57
 ++++++++++++++++++++++ 3 files changed, 80 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt create mode 100644 drivers/clocksource/rockchip_timer.c

diff --git a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file mode 100644
index 0000000..4c1950a

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +12 -0
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
Quoted 8 lines

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt

Quoted 8 lines
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +6 -0
+Example:
+
+architected-timer {
+	compatible = "rockchip,rk3288-armv7-timer";
+	reg = <0xff810020 0x20>;
+	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |

IRQ_TYPE_LEVEL_HIGH)>,

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +1 -0
+		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | 

IRQ_TYPE_LEVEL_HIGH)>,

Quoted 1 line

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt

Quoted 1 line
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +1 -0
+		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | 

IRQ_TYPE_LEVEL_HIGH)>;

Quoted 80 lines
Quoted 66 lines
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +2 -0
+	clock-frequency = <24000000>;
+};
drivers/clocksource/Makefile +1 -0
index 800b130..cbad225 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
 
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
 
+obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
 

obj-$(CONFIG_ARCH_U300) += timer-u300.o
obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o
obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o

(quoted diff) +0 -0

b/drivers/clocksource/rockchip_timer.c new file mode 100644 index 0000000..46c2146

drivers/clocksource/rockchip_timer.c new file +42 -0
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Heiko Stuebner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include "arm_arch_timer.h"
+
+#define TIMER_LOAD_COUNT0		0x00
+#define TIMER_LOAD_COUNT1		0x04
+#define TIMER_CURRENT_VALUE0		0x08
+#define TIMER_CURRENT_VALUE1		0x0c
+#define TIMER_CONTROL_REG		0x10
+#define TIMER_INT_STATUS		0x18
+
+#define TIMER_DISABLE			(0 << 0)
+#define TIMER_ENABLE			(1 << 0)
+#define TIMER_MODE_FREE_RUNNING		(0 << 1)
+#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
+#define TIMER_INT_MASK			(0 << 2)
+#define TIMER_INT_UNMASK		(1 << 2)
+
+static __init void rk3288_arch_timer_init(struct device_node *np)
+{
+	void __iomem *reg_base;
+
+	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
+	if (!reg_base) {
+		pr_warn("%s: Can't get resource\n", __func__);

drivers/clocksource/rockchip_timer.c line 52

Quoted 10 lines
drivers/clocksource/rockchip_timer.c new file +10 -0
+		return;
+	}
+
+	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
+
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
+
+	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
+	       reg_base + TIMER_CONTROL_REG);

Given that this is unconditionally set up in this way, could this not be done in the fimrware/bootloader?

Quoted 2 lines

That way you could also setup CNTFREQ correctly, and virtualisation can just work out of the box.

It always comes back to this :-) . And as always the counter-argument would be that I cannot influence the current bootloader, there are already devices shipping in this constellation. And of course quite some more SoCs using this pattern.

Quoted 1 line

This looks like a clock driver in disguise...

Yeah, that was my other idea - doing this as a clock. So would you be ok, with the arch-timer getting the possibility to handle an optional clock?

Thanks
Heiko

maintainer
#6 lore raw

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

Quoted 130 lines · Heiko St?bner

Am Freitag, 20. Juni 2014, 11:51:23 schrieb Mark Rutland:

Quoted 38 lines · Heiko St?bner
Quoted 37 lines

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run. Therefore the special rockchip,rk3288-armv7-timer does this setup and then initializes the architected timer using the new locally exposed arch_timer_init.

---

 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57
 ++++++++++++++++++++++ 3 files changed, 80 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt create mode 100644 drivers/clocksource/rockchip_timer.c

diff --git a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file mode 100644
index 0000000..4c1950a

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +12 -0
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
Quoted 8 lines

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt

Quoted 8 lines
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +6 -0
+Example:
+
+architected-timer {
+	compatible = "rockchip,rk3288-armv7-timer";
+	reg = <0xff810020 0x20>;
+	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |

IRQ_TYPE_LEVEL_HIGH)>,

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +1 -0
+		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | 

IRQ_TYPE_LEVEL_HIGH)>,

Quoted 1 line

Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt

Quoted 1 line
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +1 -0
+		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | 

IRQ_TYPE_LEVEL_HIGH)>;

Quoted 80 lines
Quoted 66 lines
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +2 -0
+	clock-frequency = <24000000>;
+};
drivers/clocksource/Makefile +1 -0
index 800b130..cbad225 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
 
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
 
+obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
 

obj-$(CONFIG_ARCH_U300) += timer-u300.o
obj-$(CONFIG_SUN4I_TIMER) += sun4i_timer.o
obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o

(quoted diff) +0 -0

b/drivers/clocksource/rockchip_timer.c new file mode 100644 index 0000000..46c2146

drivers/clocksource/rockchip_timer.c new file +42 -0
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Heiko Stuebner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include "arm_arch_timer.h"
+
+#define TIMER_LOAD_COUNT0		0x00
+#define TIMER_LOAD_COUNT1		0x04
+#define TIMER_CURRENT_VALUE0		0x08
+#define TIMER_CURRENT_VALUE1		0x0c
+#define TIMER_CONTROL_REG		0x10
+#define TIMER_INT_STATUS		0x18
+
+#define TIMER_DISABLE			(0 << 0)
+#define TIMER_ENABLE			(1 << 0)
+#define TIMER_MODE_FREE_RUNNING		(0 << 1)
+#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
+#define TIMER_INT_MASK			(0 << 2)
+#define TIMER_INT_UNMASK		(1 << 2)
+
+static __init void rk3288_arch_timer_init(struct device_node *np)
+{
+	void __iomem *reg_base;
+
+	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
+	if (!reg_base) {
+		pr_warn("%s: Can't get resource\n", __func__);

drivers/clocksource/rockchip_timer.c line 52

Quoted 10 lines
drivers/clocksource/rockchip_timer.c new file +10 -0
+		return;
+	}
+
+	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
+
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
+
+	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
+	       reg_base + TIMER_CONTROL_REG);

Given that this is unconditionally set up in this way, could this not be done in the fimrware/bootloader?

Quoted 7 lines
Quoted 2 lines

That way you could also setup CNTFREQ correctly, and virtualisation can just work out of the box.

It always comes back to this :-) . And as always the counter-argument would be that I cannot influence the current bootloader, there are already devices shipping in this constellation. And of course quite some more SoCs using this pattern.

Sure, but we aren't yet supporting the architected timer on those systems. :)

This isn't so much a pattern as an unholy tapestry of braindead.

Quoted 4 lines
Quoted 1 line

This looks like a clock driver in disguise...

Yeah, that was my other idea - doing this as a clock. So would you be ok, with the arch-timer getting the possibility to handle an optional clock?

I would very much not like to do this, but it is better than a driver whose sole purpose is to not be a clock driver.

Mark.

#7 lore raw

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

In reply to Mark Rutland

2014-06-20 12:51 GMT+02:00 Mark Rutland <[email protected]>:

Quoted 109 lines · Heiko St?bner
Quoted 108 lines

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run. Therefore the special rockchip,rk3288-armv7-timer does this setup and then initializes the architected timer using the new locally exposed arch_timer_init.

---
 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/rockchip_timer.c
Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file +22 -0
new file mode 100644
index 0000000..4c1950a
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
+Example:
+
+architected-timer {
+     compatible = "rockchip,rk3288-armv7-timer";
+     reg = <0xff810020 0x20>;
+     interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+                  <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+                  <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+     clock-frequency = <24000000>;
+};
drivers/clocksource/Makefile +1 -0
index 800b130..cbad225 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)    += timer-marco.o
 obj-$(CONFIG_ARCH_MOXART)    += moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)               += mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)    += timer-prima2.o
+obj-$(CONFIG_ARCH_ROCKCHIP)  += rockchip_timer.o
 obj-$(CONFIG_ARCH_U300)              += timer-u300.o
 obj-$(CONFIG_SUN4I_TIMER)    += sun4i_timer.o
 obj-$(CONFIG_SUN5I_HSTIMER)  += timer-sun5i.o
drivers/clocksource/rockchip_timer.c new file +47 -0
new file mode 100644
index 0000000..46c2146
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Heiko Stuebner <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include "arm_arch_timer.h"
+
+#define TIMER_LOAD_COUNT0            0x00
+#define TIMER_LOAD_COUNT1            0x04
+#define TIMER_CURRENT_VALUE0         0x08
+#define TIMER_CURRENT_VALUE1         0x0c
+#define TIMER_CONTROL_REG            0x10
+#define TIMER_INT_STATUS             0x18
+
+#define TIMER_DISABLE                        (0 << 0)
+#define TIMER_ENABLE                 (1 << 0)
+#define TIMER_MODE_FREE_RUNNING              (0 << 1)
+#define TIMER_MODE_USER_DEFINED_COUNT        (1 << 1)
+#define TIMER_INT_MASK                       (0 << 2)
+#define TIMER_INT_UNMASK             (1 << 2)
+
+static __init void rk3288_arch_timer_init(struct device_node *np)
+{
+     void __iomem *reg_base;
+
+     reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
+     if (!reg_base) {
+             pr_warn("%s: Can't get resource\n", __func__);
+             return;
+     }
+
+     writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
+
Quoted 8 lines

drivers/clocksource/rockchip_timer.c line 52

Quoted 5 lines
drivers/clocksource/rockchip_timer.c new file +5 -0
+     writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
+     writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
+
+     writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
+            reg_base + TIMER_CONTROL_REG);

Given that this is unconditionally set up in this way, could this not be done in the fimrware/bootloader?

IMHO this is a candidate for the init_time function of the machine description file until we have a bootloader that enables the arm arch timer properly.

Quoted 3 lines

That way you could also setup CNTFREQ correctly, and virtualisation can just work out of the box.

Setting CNTFREQ as done in mach-shmobile [0] does not help here, right?

Regards,
Matthias

[0] http://lxr.free-electrons.com/source/arch/arm/mach-shmobile/setup-rcar-gen2.c#L77

Quoted 4 lines

This looks like a clock driver in disguise...

Mark.

Signature · 1 line
motzblog.wordpress.com