[PATCH 0/4] Support hisilicon 64bit mode timer

[PATCH 0/4] Support hisilicon 64bit mode timer

[Kefeng Wang]

There is a kind of 64bit mode timer in hisilicon soc(like Hip05, Hip06 and
some arm32 soc), it is very similar with ARM sp804 Dual Timers, but TimerX
LOAD/Value/BGLoad are 64bit(two 32bit regs), and reg offset is different.

This patchset make some cleanup and enable 64bit mode timer for hisilicon.

Kefeng Wang (4):
  clocksource: sp804: cleanup clk_get_sys()
  clocksource: sp804: introduce helper sp804_load_mode_set()
  clocksource: sp804: use sp804_timer_disable() where possible
  clocksource: sp804: support 64bit mode for hisilicon timer64

 .../devicetree/bindings/timer/arm,sp804.txt        |   1 +
 drivers/clocksource/timer-sp.h                     |   1 +
 drivers/clocksource/timer-sp804.c                  | 142 +++++++++++++--------
 3 files changed, 92 insertions(+), 52 deletions(-)

-- 
1.7.12.4

[PATCH 1/4] clocksource: sp804: cleanup clk_get_sys()

[Kefeng Wang]

Move the clk_get_sys() part into sp804_get_clock_rate(), cleanup the same code.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/clocksource/timer-sp804.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index 5f45b9a..362e5d0 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -34,11 +34,19 @@
 
 #include "timer-sp.h"
 
-static long __init sp804_get_clock_rate(struct clk *clk)
+static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
 {
 	long rate;
 	int err;
 
+	if (!clk) {
+		clk = clk_get_sys("sp804", name);
+		if (IS_ERR(clk)) {
+			pr_err("sp804: clock not found: %ld\n", PTR_ERR(clk));
+			return PTR_ERR(clk);
+		}
+	}
+
 	err = clk_prepare(clk);
 	if (err) {
 		pr_err("sp804: clock failed to prepare: %d\n", err);
@@ -82,18 +90,7 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
 						     struct clk *clk,
 						     int use_sched_clock)
 {
-	long rate;
-
-	if (!clk) {
-		clk = clk_get_sys("sp804", name);
-		if (IS_ERR(clk)) {
-			pr_err("sp804: clock not found: %d\n",
-			       (int)PTR_ERR(clk));
-			return;
-		}
-	}
-
-	rate = sp804_get_clock_rate(clk);
+	long rate = sp804_get_clock_rate(clk, name);
 
 	if (rate < 0)
 		return;
@@ -189,17 +186,8 @@ static struct irqaction sp804_timer_irq = {
 void __init __sp804_clockevents_init(void __iomem *base, unsigned int irq, struct clk *clk, const char *name)
 {
 	struct clock_event_device *evt = &sp804_clockevent;
-	long rate;
-
-	if (!clk)
-		clk = clk_get_sys("sp804", name);
-	if (IS_ERR(clk)) {
-		pr_err("sp804: %s clock not found: %d\n", name,
-			(int)PTR_ERR(clk));
-		return;
-	}
+	long rate = sp804_get_clock_rate(clk, name);
 
-	rate = sp804_get_clock_rate(clk);
 	if (rate < 0)
 		return;
 
-- 
1.7.12.4

[PATCH 2/4] clocksource: sp804: introduce helper sp804_load_mode_set()

[Kefeng Wang]

Introduce helper sp804_load_mode_set(), and use it, prepare for 64bit
mode timer support.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/clocksource/timer-sp804.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index 362e5d0..b8848e5 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -73,6 +73,15 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
 	return rate;
 }
 
+static inline void sp804_load_mode_set(void __iomem *base, unsigned long load, int mode)
+{
+	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
+			     mode | TIMER_CTRL_ENABLE;
+
+	writel(load, base + TIMER_LOAD);
+	writel(ctrl, base + TIMER_CTRL);
+}
+
 static void __iomem *sched_clock_base;
 
 static u64 notrace sp804_read(void)
@@ -97,10 +106,8 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
 
 	/* setup timer 0 as free-running clocksource */
 	writel(0, base + TIMER_CTRL);
-	writel(0xffffffff, base + TIMER_LOAD);
 	writel(0xffffffff, base + TIMER_VALUE);
-	writel(TIMER_CTRL_32BIT | TIMER_CTRL_ENABLE | TIMER_CTRL_PERIODIC,
-		base + TIMER_CTRL);
+	sp804_load_mode_set(base, 0xffffffff, TIMER_CTRL_PERIODIC & ~TIMER_CTRL_IE);
 
 	clocksource_mmio_init(base + TIMER_VALUE, name,
 		rate, 200, 32, clocksource_mmio_readl_down);
@@ -143,24 +150,16 @@ static int sp804_shutdown(struct clock_event_device *evt)
 
 static int sp804_set_periodic(struct clock_event_device *evt)
 {
-	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
-			     TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
-
 	timer_shutdown(evt);
-	writel(clkevt_reload, clkevt_base + TIMER_LOAD);
-	writel(ctrl, clkevt_base + TIMER_CTRL);
+	sp804_load_mode_set(clkevt_base, clkevt_reload, TIMER_CTRL_PERIODIC);
 	return 0;
 }
 
 static int sp804_set_next_event(unsigned long next,
-	struct clock_event_device *evt)
+				struct clock_event_device *evt)
 {
-	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
-			     TIMER_CTRL_ONESHOT | TIMER_CTRL_ENABLE;
-
-	writel(next, clkevt_base + TIMER_LOAD);
-	writel(ctrl, clkevt_base + TIMER_CTRL);
 
+	sp804_load_mode_set(clkevt_base, next, TIMER_CTRL_ONESHOT);
 	return 0;
 }
 
-- 
1.7.12.4

[PATCH 3/4] clocksource: sp804: use sp804_timer_disable() where possible

[Kefeng Wang]

Use sp804_timer_disable() where possible, prepare for 64bit mode timer support.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/clocksource/timer-sp804.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index b8848e5..2ff8777 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -105,7 +105,7 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
 		return;
 
 	/* setup timer 0 as free-running clocksource */
-	writel(0, base + TIMER_CTRL);
+	sp804_timer_disable(base);
 	writel(0xffffffff, base + TIMER_VALUE);
 	sp804_load_mode_set(base, 0xffffffff, TIMER_CTRL_PERIODIC & ~TIMER_CTRL_IE);
 
@@ -196,8 +196,7 @@ void __init __sp804_clockevents_init(void __iomem *base, unsigned int irq, struc
 	evt->irq = irq;
 	evt->cpumask = cpu_possible_mask;
 
-	writel(0, base + TIMER_CTRL);
-
+	sp804_timer_disable(base);
 	setup_irq(irq, &sp804_timer_irq);
 	clockevents_config_and_register(evt, rate, 0xf, 0xffffffff);
 }
@@ -216,8 +215,8 @@ static void __init sp804_of_init(struct device_node *np)
 		return;
 
 	/* Ensure timers are disabled */
-	writel(0, base + TIMER_CTRL);
-	writel(0, base + TIMER_2_BASE + TIMER_CTRL);
+	sp804_timer_disable(base);
+	sp804_timer_disable(base + TIMER_2_BASE);
 
 	if (initialized || !of_device_is_available(np))
 		goto err;
@@ -274,7 +273,7 @@ static void __init integrator_cp_of_init(struct device_node *np)
 		return;
 
 	/* Ensure timer is disabled */
-	writel(0, base + TIMER_CTRL);
+	sp804_timer_disable(base);
 
 	if (init_count == 2 || !of_device_is_available(np))
 		goto err;
-- 
1.7.12.4

[PATCH 4/4] clocksource: sp804: support 64bit mode for hisilicon timer64

[Kefeng Wang]

There is a kind of 64bit mode timer in hisilicon soc(like Hip05, Hip06 and
some arm32 soc), it is very similar with ARM sp804 Dual Timers, but TimerX
LOAD/Value/BGLoad are 64bit(two 32bit regs), and reg offset is different.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 .../devicetree/bindings/timer/arm,sp804.txt        |  1 +
 drivers/clocksource/timer-sp.h                     |  1 +
 drivers/clocksource/timer-sp804.c                  | 76 ++++++++++++++++++----
 3 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/arm,sp804.txt b/Documentation/devicetree/bindings/timer/arm,sp804.txt
index 5cd8eee7..d8e8f8f 100644
--- a/Documentation/devicetree/bindings/timer/arm,sp804.txt
+++ b/Documentation/devicetree/bindings/timer/arm,sp804.txt
@@ -17,6 +17,7 @@ Optional properties:
 - arm,sp804-has-irq = <#>: In the case of only 1 timer irq line connected, this
 	specifies if the irq connection is for timer 1 or timer 2. A value of 1
 	or 2 should be used.
+- hisilicon,timer64: Support hisilicon 64bit mode timer.
 
 Example:
 
diff --git a/drivers/clocksource/timer-sp.h b/drivers/clocksource/timer-sp.h
index 050d885..adf82f4 100644
--- a/drivers/clocksource/timer-sp.h
+++ b/drivers/clocksource/timer-sp.h
@@ -16,6 +16,7 @@
 #define TIMER_VALUE	0x04			/* ACVR ro */
 #define TIMER_CTRL	0x08			/* ACVR rw */
 #define TIMER_CTRL_ONESHOT	(1 << 0)	/*  CVR */
+/* Used in hisilicon timer64, it means enabling 64bit mode */
 #define TIMER_CTRL_32BIT	(1 << 1)	/*  CVR */
 #define TIMER_CTRL_DIV1		(0 << 2)	/* ACVR */
 #define TIMER_CTRL_DIV16	(1 << 2)	/* ACVR */
diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index 2ff8777..7f1d947 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -34,6 +34,16 @@
 
 #include "timer-sp.h"
 
+#define TIMER64_2_BASE	0x40
+#define TIMER64_LOAD_L	0x00
+#define TIMER64_LOAD_H	0x04
+#define TIMER64_VALUE_L	0x08
+#define TIMER64_VALUE_H	0x0C
+
+#define HISI_OFFSET	0x8
+
+static int timer64_offset;
+
 static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
 {
 	long rate;
@@ -78,8 +88,8 @@ static inline void sp804_load_mode_set(void __iomem *base, unsigned long load, i
 	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
 			     mode | TIMER_CTRL_ENABLE;
 
-	writel(load, base + TIMER_LOAD);
-	writel(ctrl, base + TIMER_CTRL);
+	writel(load, base + TIMER_LOAD); /* equal TIMER64_LOAD_L when timer64*/
+	writel(ctrl, base + TIMER_CTRL + timer64_offset);
 }
 
 static void __iomem *sched_clock_base;
@@ -89,11 +99,37 @@ static u64 notrace sp804_read(void)
 	return ~readl_relaxed(sched_clock_base + TIMER_VALUE);
 }
 
+static u64 notrace hisi_timer64_read(void)
+{
+	u32 val_lo, val_hi, tmp_hi;
+
+	do {
+		val_hi = readl_relaxed(sched_clock_base + TIMER64_VALUE_H);
+		val_lo = readl_relaxed(sched_clock_base + TIMER64_VALUE_L);
+		tmp_hi = readl_relaxed(sched_clock_base + TIMER64_VALUE_H);
+	} while (val_hi != tmp_hi);
+
+	return ((u64) val_hi << 32) | val_lo;
+}
+
 void __init sp804_timer_disable(void __iomem *base)
 {
-	writel(0, base + TIMER_CTRL);
+	writel(0, base + TIMER_CTRL + timer64_offset);
 }
 
+static cycle_t hisi_clocksource_read(struct clocksource *cs)
+{
+	return hisi_timer64_read();
+}
+
+static struct clocksource hisi_clocksource = {
+	.name	= "hisilicon_timer64",
+	.rating	= 200,
+	.read	= hisi_clocksource_read,
+	.mask	= CLOCKSOURCE_MASK(64),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
 void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
 						     const char *name,
 						     struct clk *clk,
@@ -106,15 +142,25 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
 
 	/* setup timer 0 as free-running clocksource */
 	sp804_timer_disable(base);
-	writel(0xffffffff, base + TIMER_VALUE);
+	writel(0xffffffff, base + TIMER_VALUE); /* equal TIMER64_LOAD_H when tiemr64*/
+	if (timer64_offset) {
+		writel(0xffffffff, base + TIMER64_VALUE_L);
+		writel(0xffffffff, base + TIMER64_VALUE_H);
+	}
 	sp804_load_mode_set(base, 0xffffffff, TIMER_CTRL_PERIODIC & ~TIMER_CTRL_IE);
 
-	clocksource_mmio_init(base + TIMER_VALUE, name,
-		rate, 200, 32, clocksource_mmio_readl_down);
+	if (timer64_offset)
+		clocksource_register_hz(&hisi_clocksource, rate);
+	else
+		clocksource_mmio_init(base + TIMER_VALUE, name, rate, 200, 32,
+				      clocksource_mmio_readl_down);
 
 	if (use_sched_clock) {
 		sched_clock_base = base;
-		sched_clock_register(sp804_read, 32, rate);
+		if (timer64_offset)
+			sched_clock_register(hisi_timer64_read, 64, rate);
+		else
+			sched_clock_register(sp804_read, 32, rate);
 	}
 }
 
@@ -130,7 +176,7 @@ static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id)
 	struct clock_event_device *evt = dev_id;
 
 	/* clear the interrupt */
-	writel(1, clkevt_base + TIMER_INTCLR);
+	writel(1, clkevt_base + TIMER_INTCLR + timer64_offset);
 
 	evt->event_handler(evt);
 
@@ -139,7 +185,7 @@ static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id)
 
 static inline void timer_shutdown(struct clock_event_device *evt)
 {
-	writel(0, clkevt_base + TIMER_CTRL);
+	writel(0, clkevt_base + TIMER_CTRL + timer64_offset);
 }
 
 static int sp804_shutdown(struct clock_event_device *evt)
@@ -204,6 +250,7 @@ void __init __sp804_clockevents_init(void __iomem *base, unsigned int irq, struc
 static void __init sp804_of_init(struct device_node *np)
 {
 	static bool initialized = false;
+	int timer_2_base = TIMER_2_BASE;
 	void __iomem *base;
 	int irq;
 	u32 irq_num = 0;
@@ -214,9 +261,14 @@ static void __init sp804_of_init(struct device_node *np)
 	if (WARN_ON(!base))
 		return;
 
+	if (of_property_read_bool(np, "hisilicon,timer64")) {
+		timer64_offset = HISI_OFFSET;
+		timer_2_base = TIMER64_2_BASE;
+	}
+
 	/* Ensure timers are disabled */
 	sp804_timer_disable(base);
-	sp804_timer_disable(base + TIMER_2_BASE);
+	sp804_timer_disable(base + timer_2_base);
 
 	if (initialized || !of_device_is_available(np))
 		goto err;
@@ -242,11 +294,11 @@ static void __init sp804_of_init(struct device_node *np)
 
 	of_property_read_u32(np, "arm,sp804-has-irq", &irq_num);
 	if (irq_num == 2) {
-		__sp804_clockevents_init(base + TIMER_2_BASE, irq, clk2, name);
+		__sp804_clockevents_init(base + timer_2_base, irq, clk2, name);
 		__sp804_clocksource_and_sched_clock_init(base, name, clk1, 1);
 	} else {
 		__sp804_clockevents_init(base, irq, clk1 , name);
-		__sp804_clocksource_and_sched_clock_init(base + TIMER_2_BASE,
+		__sp804_clocksource_and_sched_clock_init(base + timer_2_base,
 							 name, clk2, 1);
 	}
 	initialized = true;
-- 
1.7.12.4

Re: [PATCH 2/4] clocksource: sp804: introduce helper sp804_load_mode_set()

[Daniel Lezcano]

On 05/28/2016 11:33 AM, Kefeng Wang wrote:
> Introduce helper sp804_load_mode_set(), and use it, prepare for 64bit
> mode timer support.

More verbose please.

> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   drivers/clocksource/timer-sp804.c | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
> index 362e5d0..b8848e5 100644
> --- a/drivers/clocksource/timer-sp804.c
> +++ b/drivers/clocksource/timer-sp804.c
> @@ -73,6 +73,15 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
>   	return rate;
>   }
>
> +static inline void sp804_load_mode_set(void __iomem *base, unsigned long load, int mode)
> +{
> +	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
> +			     mode | TIMER_CTRL_ENABLE;
> +
> +	writel(load, base + TIMER_LOAD);
> +	writel(ctrl, base + TIMER_CTRL);
> +}
> +
>   static void __iomem *sched_clock_base;
>
>   static u64 notrace sp804_read(void)
> @@ -97,10 +106,8 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
>
>   	/* setup timer 0 as free-running clocksource */
>   	writel(0, base + TIMER_CTRL);
> -	writel(0xffffffff, base + TIMER_LOAD);
>   	writel(0xffffffff, base + TIMER_VALUE);
> -	writel(TIMER_CTRL_32BIT | TIMER_CTRL_ENABLE | TIMER_CTRL_PERIODIC,
> -		base + TIMER_CTRL);
> +	sp804_load_mode_set(base, 0xffffffff, TIMER_CTRL_PERIODIC & ~TIMER_CTRL_IE);

Can you explain this change ?

>   	clocksource_mmio_init(base + TIMER_VALUE, name,
>   		rate, 200, 32, clocksource_mmio_readl_down);
> @@ -143,24 +150,16 @@ static int sp804_shutdown(struct clock_event_device *evt)
>
>   static int sp804_set_periodic(struct clock_event_device *evt)
>   {
> -	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
> -			     TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
> -
>   	timer_shutdown(evt);
> -	writel(clkevt_reload, clkevt_base + TIMER_LOAD);
> -	writel(ctrl, clkevt_base + TIMER_CTRL);
> +	sp804_load_mode_set(clkevt_base, clkevt_reload, TIMER_CTRL_PERIODIC);
>   	return 0;
>   }
>
>   static int sp804_set_next_event(unsigned long next,
> -	struct clock_event_device *evt)
> +				struct clock_event_device *evt)
>   {
> -	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
> -			     TIMER_CTRL_ONESHOT | TIMER_CTRL_ENABLE;
> -
> -	writel(next, clkevt_base + TIMER_LOAD);
> -	writel(ctrl, clkevt_base + TIMER_CTRL);
>
> +	sp804_load_mode_set(clkevt_base, next, TIMER_CTRL_ONESHOT);
>   	return 0;
>   }
>
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 3/4] clocksource: sp804: use sp804_timer_disable() where possible

[Daniel Lezcano]

On 05/28/2016 11:33 AM, Kefeng Wang wrote:
> Use sp804_timer_disable() where possible, prepare for 64bit mode timer support.

Explain this change please.

> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   drivers/clocksource/timer-sp804.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
> index b8848e5..2ff8777 100644
> --- a/drivers/clocksource/timer-sp804.c
> +++ b/drivers/clocksource/timer-sp804.c
> @@ -105,7 +105,7 @@ void __init __sp804_clocksource_and_sched_clock_init(void __iomem *base,
>   		return;
>
>   	/* setup timer 0 as free-running clocksource */
> -	writel(0, base + TIMER_CTRL);
> +	sp804_timer_disable(base);
>   	writel(0xffffffff, base + TIMER_VALUE);
>   	sp804_load_mode_set(base, 0xffffffff, TIMER_CTRL_PERIODIC & ~TIMER_CTRL_IE);
>
> @@ -196,8 +196,7 @@ void __init __sp804_clockevents_init(void __iomem *base, unsigned int irq, struc
>   	evt->irq = irq;
>   	evt->cpumask = cpu_possible_mask;
>
> -	writel(0, base + TIMER_CTRL);
> -
> +	sp804_timer_disable(base);
>   	setup_irq(irq, &sp804_timer_irq);
>   	clockevents_config_and_register(evt, rate, 0xf, 0xffffffff);
>   }
> @@ -216,8 +215,8 @@ static void __init sp804_of_init(struct device_node *np)
>   		return;
>
>   	/* Ensure timers are disabled */
> -	writel(0, base + TIMER_CTRL);
> -	writel(0, base + TIMER_2_BASE + TIMER_CTRL);
> +	sp804_timer_disable(base);
> +	sp804_timer_disable(base + TIMER_2_BASE);
>
>   	if (initialized || !of_device_is_available(np))
>   		goto err;
> @@ -274,7 +273,7 @@ static void __init integrator_cp_of_init(struct device_node *np)
>   		return;
>
>   	/* Ensure timer is disabled */
> -	writel(0, base + TIMER_CTRL);
> +	sp804_timer_disable(base);
>
>   	if (init_count == 2 || !of_device_is_available(np))
>   		goto err;
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 4/4] clocksource: sp804: support 64bit mode for hisilicon timer64

[Daniel Lezcano]

On 05/28/2016 11:33 AM, Kefeng Wang wrote:
> There is a kind of 64bit mode timer in hisilicon soc(like Hip05, Hip06 and
> some arm32 soc), it is very similar with ARM sp804 Dual Timers, but TimerX
> LOAD/Value/BGLoad are 64bit(two 32bit regs), and reg offset is different.

"A kind of 64bit mode timer" is very vague. Please describe more the 
timer you want to support in this driver and split dt-bindings, so Rob 
can ack it independently.

Especially, explain why 64bits is needed here, when the clocksource 
wraps up, etc ...

> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   .../devicetree/bindings/timer/arm,sp804.txt        |  1 +
>   drivers/clocksource/timer-sp.h                     |  1 +
>   drivers/clocksource/timer-sp804.c                  | 76 ++++++++++++++++++----
>   3 files changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/arm,sp804.txt b/Documentation/devicetree/bindings/timer/arm,sp804.txt
> index 5cd8eee7..d8e8f8f 100644
> --- a/Documentation/devicetree/bindings/timer/arm,sp804.txt
> +++ b/Documentation/devicetree/bindings/timer/arm,sp804.txt
> @@ -17,6 +17,7 @@ Optional properties:
>   - arm,sp804-has-irq = <#>: In the case of only 1 timer irq line connected, this
>   	specifies if the irq connection is for timer 1 or timer 2. A value of 1
>   	or 2 should be used.
> +- hisilicon,timer64: Support hisilicon 64bit mode timer.
>
>   Example:
>
> diff --git a/drivers/clocksource/timer-sp.h b/drivers/clocksource/timer-sp.h
> index 050d885..adf82f4 100644
> --- a/drivers/clocksource/timer-sp.h
> +++ b/drivers/clocksource/timer-sp.h
> @@ -16,6 +16,7 @@
>   #define TIMER_VALUE	0x04			/* ACVR ro */
>   #define TIMER_CTRL	0x08			/* ACVR rw */
>   #define TIMER_CTRL_ONESHOT	(1 << 0)	/*  CVR */
> +/* Used in hisilicon timer64, it means enabling 64bit mode */
>   #define TIMER_CTRL_32BIT	(1 << 1)	/*  CVR */
>   #define TIMER_CTRL_DIV1		(0 << 2)	/* ACVR */
>   #define TIMER_CTRL_DIV16	(1 << 2)	/* ACVR */
> diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
> index 2ff8777..7f1d947 100644
> --- a/drivers/clocksource/timer-sp804.c
> +++ b/drivers/clocksource/timer-sp804.c
> @@ -34,6 +34,16 @@
>
>   #include "timer-sp.h"
>
> +#define TIMER64_2_BASE	0x40
> +#define TIMER64_LOAD_L	0x00
> +#define TIMER64_LOAD_H	0x04
> +#define TIMER64_VALUE_L	0x08
> +#define TIMER64_VALUE_H	0x0C
> +
> +#define HISI_OFFSET	0x8
> +
> +static int timer64_offset;
> +
>   static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
>   {
>   	long rate;
> @@ -78,8 +88,8 @@ static inline void sp804_load_mode_set(void __iomem *base, unsigned long load, i
>   	unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE |
>   			     mode | TIMER_CTRL_ENABLE;
>
> -	writel(load, base + TIMER_LOAD);
> -	writel(ctrl, base + TIMER_CTRL);
> +	writel(load, base + TIMER_LOAD); /* equal TIMER64_LOAD_L when timer64*/
> +	writel(ctrl, base + TIMER_CTRL + timer64_offset);
>   }
>
>   static void __iomem *sched_clock_base;
> @@ -89,11 +99,37 @@ static u64 notrace sp804_read(void)
>   	return ~readl_relaxed(sched_clock_base + TIMER_VALUE);
>   }
>
> +static u64 notrace hisi_timer64_read(void)
> +{
> +	u32 val_lo, val_hi, tmp_hi;
> +
> +	do {
> +		val_hi = readl_relaxed(sched_clock_base + TIMER64_VALUE_H);
> +		val_lo = readl_relaxed(sched_clock_base + TIMER64_VALUE_L);
> +		tmp_hi = readl_relaxed(sched_clock_base + TIMER64_VALUE_H);
> +	} while (val_hi != tmp_hi);
> +
> +	return ((u64) val_hi << 32) | val_lo;
> +}

64bits is not free. Is it really needed ? How long before it wraps up in 
32bits ?

> +
>   void __init sp804_timer_disable(void __iomem *base)
>   {
> -	writel(0, base + TIMER_CTRL);
> +	writel(0, base + TIMER_CTRL + timer64_offset);
>   }

Nope, sp804_timer_disable() should be called with the right parameter.

Ok, so before going further, do the following:

1. Create a structure:

struct sp804_clockevent {
	struct clockevent clkevt;
	void __iomem *base;
	void __iomem *ctrl;
	int width;
};

2. Add a function to return the sp804_clockevent from a clkevt

struct sp804_clockevent sp804_timer(struct clockevent *clkevt)
{
	return container_of(clkevt, struct sp804_clockevent, clkevt);
}

3. Change the different function to use:
	struct sp804_clockevent -> base | ctrl
	
4. And then at init time, fill the structure with the right info.

5. Make sure, 64bits clocksource is really needed and if it is possible 
to share a common timer_read()

Thanks !
   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog