[PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Caesar Wang]

This series patches had been tested on rockchip inside kernel.
In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
save power in idle.
Okay, it still works bootup on rk3288/other SoCs, even though many socs hasn't used
the broadcast timer.

History version:
v1:
https://lkml.org/lkml/2016/5/25/186

Easy to test for my borad.
localhost / # cat /proc/interrupts
CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
1:          0          0          0          0          0          0     GICv3  29 Edge      arch_timer
...
5:          0          0          0          0          0          0     GICv3 113 Level     rk_timer
..

localhost / # cat /proc/timer_list | grep event_handler
get "event_handler:  hrtimer_interrupt"
event_handler:  tick_handle_oneshot_broadcast
event_handler:  hrtimer_interrupt

That should work for my board.


Changes in v2:
- Add the Rob' Ack.
- s/cpu_all_mask/cpu_possible_mask/, As Daniel comments on
  https://patchwork.kernel.org/patch/9135053/.
- As the Daniel suggests on https://patchwork.kernel.org/patch/9135061/,
  That will be better for the rockchip timer driver.
- %s/ARM64/arm64.

Huang Tao (2):
  dt-bindings: document rk3399 rk-timer bindings
  arm64: dts: rockchip: add rktimer device node for rk3399

Huang, Tao (2):
  clocksource: rockchip: add dynamic irq flag to the timer
  clocksource: rockchip: add support for rk3399 SoC

 ...chip,rk3288-timer.txt => rockchip,rk-timer.txt} |  6 ++--
 arch/arm64/boot/dts/rockchip/rk3399.dtsi           |  8 +++++
 drivers/clocksource/rockchip_timer.c               | 36 +++++++++++++++++-----
 3 files changed, 41 insertions(+), 9 deletions(-)
 rename Documentation/devicetree/bindings/timer/{rockchip,rk3288-timer.txt => rockchip,rk-timer.txt} (75%)

-- 
1.9.1

[PATCH v2 1/4] dt-bindings: document rk3399 rk-timer bindings

[Caesar Wang]

From: Huang Tao <huangtao@rock-chips.com>

Add compatible string for rk3399 because which timer is a little
different from older SoCs. So rename the file name from
rockchip,rk3288-timer.txt to rockchip,rk-timer.txt.
Clarify rockchip,rk3288-timer supported SoCs.

Signed-off-by: Huang Tao <huangtao@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>

---

Changes in v2:
- Add the Rob' Ack.

 .../timer/{rockchip,rk3288-timer.txt => rockchip,rk-timer.txt}      | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/timer/{rockchip,rk3288-timer.txt => rockchip,rk-timer.txt} (75%)

diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
similarity index 75%
rename from Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt
rename to Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
index 87f0b00..a41b184 100644
--- a/Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt
+++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
@@ -1,7 +1,9 @@
-Rockchip rk3288 timer
+Rockchip rk timer
 
 Required properties:
-- compatible: shall be "rockchip,rk3288-timer"
+- compatible: shall be one of:
+  "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368
+  "rockchip,rk3399-timer" - for rk3399
 - reg: base address of the timer register starting with TIMERS CONTROL register
 - interrupts: should contain the interrupts for Timer0
 - clocks : must contain an entry for each entry in clock-names
-- 
1.9.1

[PATCH v2 2/4] clocksource: rockchip: add dynamic irq flag to the timer

[Caesar Wang]

From: "Huang, Tao" <huangtao@rock-chips.com>

The rockchip timer is broadcast timer. Add CLOCK_EVT_FEAT_DYNIRQ
flag and set cpumask to all possible cpus to save power by avoid
unnecessary wakeups and IPIs.

Signed-off-by: Huang Tao <huangtao@rock-chips.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Tested-by: Jianqun Xu <jay.xu@rock-chips.com>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>

---

Changes in v2:
- s/cpu_all_mask/cpu_possible_mask/, As Daniel comments on
  https://patchwork.kernel.org/patch/9135053/.

 drivers/clocksource/rockchip_timer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index b991b28..b510863 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -150,12 +150,13 @@ static void __init rk_timer_init(struct device_node *np)
 	}
 
 	ce->name = TIMER_NAME;
-	ce->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	ce->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
+		       CLOCK_EVT_FEAT_DYNIRQ;
 	ce->set_next_event = rk_timer_set_next_event;
 	ce->set_state_shutdown = rk_timer_shutdown;
 	ce->set_state_periodic = rk_timer_set_periodic;
 	ce->irq = irq;
-	ce->cpumask = cpumask_of(0);
+	ce->cpumask = cpu_possible_mask;
 	ce->rating = 250;
 
 	rk_timer_interrupt_clear(ce);
-- 
1.9.1

[PATCH v2 3/4] clocksource: rockchip: add support for rk3399 SoC

[Caesar Wang]

From: "Huang, Tao" <huangtao@rock-chips.com>

The CONTROL register offset is different from old SoCs.
For Linux driver, there are not functional changes at all.
So add dedicated mapping for the CONTROL register.

Signed-off-by: Huang Tao <huangtao@rock-chips.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Tested-by: Jianqun Xu <jay.xu@rock-chips.com>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>

---

Changes in v2:
- As the Daniel suggests on https://patchwork.kernel.org/patch/9135061/,
  That will be better for the rockchip timer driver.

 drivers/clocksource/rockchip_timer.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index b510863..a3f22b0 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -19,7 +19,8 @@
 
 #define TIMER_LOAD_COUNT0	0x00
 #define TIMER_LOAD_COUNT1	0x04
-#define TIMER_CONTROL_REG	0x10
+#define TIMER_CONTROL_REG3288	0x10
+#define TIMER_CONTROL_REG3399	0x1c
 #define TIMER_INT_STATUS	0x18
 
 #define TIMER_DISABLE		0x0
@@ -31,6 +32,7 @@
 struct bc_timer {
 	struct clock_event_device ce;
 	void __iomem *base;
+	void __iomem *ctrl;
 	u32 freq;
 };
 
@@ -46,15 +48,20 @@ static inline void __iomem *rk_base(struct clock_event_device *ce)
 	return rk_timer(ce)->base;
 }
 
+static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
+{
+	return rk_timer(ce)->ctrl;
+}
+
 static inline void rk_timer_disable(struct clock_event_device *ce)
 {
-	writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG);
+	writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
 }
 
 static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
 {
 	writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
-		       rk_base(ce) + TIMER_CONTROL_REG);
+		       rk_ctrl(ce));
 }
 
 static void rk_timer_update_counter(unsigned long cycles,
@@ -106,7 +113,7 @@ static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void __init rk_timer_init(struct device_node *np)
+static void __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
 {
 	struct clock_event_device *ce = &bc_timer.ce;
 	struct clk *timer_clk;
@@ -118,6 +125,7 @@ static void __init rk_timer_init(struct device_node *np)
 		pr_err("Failed to get base address for '%s'\n", TIMER_NAME);
 		return;
 	}
+	bc_timer.ctrl = bc_timer.base + ctrl_reg;
 
 	pclk = of_clk_get_by_name(np, "pclk");
 	if (IS_ERR(pclk)) {
@@ -180,4 +188,17 @@ out_unmap:
 	iounmap(bc_timer.base);
 }
 
-CLOCKSOURCE_OF_DECLARE(rk_timer, "rockchip,rk3288-timer", rk_timer_init);
+static void __init rk3288_timer_init(struct device_node *np)
+{
+	rk_timer_init(np, TIMER_CONTROL_REG3288);
+}
+
+static void __init rk3399_timer_init(struct device_node *np)
+{
+	rk_timer_init(np, TIMER_CONTROL_REG3399);
+}
+
+CLOCKSOURCE_OF_DECLARE(rk3288_timer, "rockchip,rk3288-timer",
+		       rk3288_timer_init);
+CLOCKSOURCE_OF_DECLARE(rk3399_timer, "rockchip,rk3399-timer",
+		       rk3399_timer_init);
-- 
1.9.1

[PATCH v2 4/4] arm64: dts: rockchip: add rktimer device node for rk3399

[Caesar Wang]

From: Huang Tao <huangtao@rock-chips.com>

Select rktimer0 as broadcast timer.

Signed-off-by: Huang Tao <huangtao@rock-chips.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Tested-by: Jianqun Xu <jay.xu@rock-chips.com>

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
---

Changes in v2:
- %s/ARM64/arm64.

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d331e80..f0d1935 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -542,6 +542,14 @@
 		interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
+	rktimer: rktimer@ff850000 {
+		compatible = "rockchip,rk3399-timer";
+		reg = <0x0 0xff850000 0x0 0x1000>;
+		interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru PCLK_TIMER0>, <&cru SCLK_TIMER00>;
+		clock-names = "pclk", "timer";
+	};
+
 	spdif: spdif@ff870000 {
 		compatible = "rockchip,rk3399-spdif";
 		reg = <0x0 0xff870000 0x0 0x1000>;
-- 
1.9.1

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Daniel Lezcano]

On Tue, Jun 07, 2016 at 12:54:29PM +0800, Caesar Wang wrote:
> This series patches had been tested on rockchip inside kernel.
> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
> save power in idle.

For my personnal information, are the arch_timer in the same power domain 
than the CPU ? IOW, what is the 'always-on' property in the DT ?

> Okay, it still works bootup on rk3288/other SoCs, even though many socs 
> hasn't used
> the broadcast timer.

Yes, unfortunately the SoC design on rk3288 and the previous ones do not 
allow to use a cpuidle driver with cpu/cluster power down, so obviously the 
broadcast timer is pointless on these boards :)

> History version:
> v1:
> https://lkml.org/lkml/2016/5/25/186
> 
> Easy to test for my borad.
> localhost / # cat /proc/interrupts
> CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
> 1:          0          0          0          0          0          0     GICv3  29 Edge      arch_timer
> ...
> 5:          0          0          0          0          0          0     GICv3 113 Level     rk_timer
> ..
> 
> localhost / # cat /proc/timer_list | grep event_handler
> get "event_handler:  hrtimer_interrupt"
> event_handler:  tick_handle_oneshot_broadcast
> event_handler:  hrtimer_interrupt

What are you trying to demonstrate here ? There are no interrupts for both 
arch_timer and rk_timer.

> That should work for my board.

What do you mean ?

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Huang, Tao]

Hi Daniel:
On 2016年06月13日 21:06, Daniel Lezcano wrote:
> On Tue, Jun 07, 2016 at 12:54:29PM +0800, Caesar Wang wrote:
>> This series patches had been tested on rockchip inside kernel.
>> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
>> save power in idle.
> 
> For my personnal information, are the arch_timer in the same power domain 
> than the CPU ? IOW, what is the 'always-on' property in the DT ?

Yes. In our SoC design, all arch (generic) timer in the same power
domain of CPU core. So if one CPU core power down, the arch (generic)
timer will lose it's state and stop working.
While rk timer maybe in peri power domain or pmu power domain, so the
timer will still work when CPU power down.

But before RK3399, all SoCs with CPU power domain, do not support auto
power down while cpu idle. So the arch timer can be seem as always on,
i.e. we don't need a broadcast timer at all.

> 
>> Okay, it still works bootup on rk3288/other SoCs, even though many socs 
>> hasn't used
>> the broadcast timer.
> 
> Yes, unfortunately the SoC design on rk3288 and the previous ones do not 
> allow to use a cpuidle driver with cpu/cluster power down, so obviously the 
> broadcast timer is pointless on these boards :)
> 

You are right.

>> History version:
>> v1:
>> https://lkml.org/lkml/2016/5/25/186
>>
>> Easy to test for my borad.
>> localhost / # cat /proc/interrupts
>> CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
>> 1:          0          0          0          0          0          0     GICv3  29 Edge      arch_timer
>> ...
>> 5:          0          0          0          0          0          0     GICv3 113 Level     rk_timer
>> ..
>>
>> localhost / # cat /proc/timer_list | grep event_handler
>> get "event_handler:  hrtimer_interrupt"
>> event_handler:  tick_handle_oneshot_broadcast
>> event_handler:  hrtimer_interrupt
> 
> What are you trying to demonstrate here ? There are no interrupts for both 
> arch_timer and rk_timer.

I don't know. Maybe Caesar do something wrong :(
This is my output:
           CPU0       CPU1       CPU2       CPU3       CPU4       CPU5

...
  2:       2911       1967       1588       1608       1295       1606
   GICv3  30 Edge      arch_timer
  5:        578        637        684        626        161        165
   GICv3 113 Level     rk_timer

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Caesar Wang]

On 2016年06月14日 12:00, Huang, Tao wrote:
> Hi Daniel:
> On 2016年06月13日 21:06, Daniel Lezcano wrote:
>> On Tue, Jun 07, 2016 at 12:54:29PM +0800, Caesar Wang wrote:
>>> This series patches had been tested on rockchip inside kernel.
>>> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
>>> save power in idle.
>> For my personnal information, are the arch_timer in the same power domain
>> than the CPU ? IOW, what is the 'always-on' property in the DT ?
> Yes. In our SoC design, all arch (generic) timer in the same power
> domain of CPU core. So if one CPU core power down, the arch (generic)
> timer will lose it's state and stop working.
> While rk timer maybe in peri power domain or pmu power domain, so the
> timer will still work when CPU power down.
>
> But before RK3399, all SoCs with CPU power domain, do not support auto
> power down while cpu idle. So the arch timer can be seem as always on,
> i.e. we don't need a broadcast timer at all.
>
>>> Okay, it still works bootup on rk3288/other SoCs, even though many socs
>>> hasn't used
>>> the broadcast timer.
>> Yes, unfortunately the SoC design on rk3288 and the previous ones do not
>> allow to use a cpuidle driver with cpu/cluster power down, so obviously the
>> broadcast timer is pointless on these boards :)
>>
> You are right.
>
>>> History version:
>>> v1:
>>> https://lkml.org/lkml/2016/5/25/186
>>>
>>> Easy to test for my borad.
>>> localhost / # cat /proc/interrupts
>>> CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
>>> 1:          0          0          0          0          0          0     GICv3  29 Edge      arch_timer
>>> ...
>>> 5:          0          0          0          0          0          0     GICv3 113 Level     rk_timer
>>> ..
>>>
>>> localhost / # cat /proc/timer_list | grep event_handler
>>> get "event_handler:  hrtimer_interrupt"
>>> event_handler:  tick_handle_oneshot_broadcast
>>> event_handler:  hrtimer_interrupt
>> What are you trying to demonstrate here ? There are no interrupts for both
>> arch_timer and rk_timer.

My god!! let's forget it now!
Sorry for forgetting what happened.
---

Re-picked them up for my board since I'm doing other things to run a 
single cpu.

localhost / # cat /proc/interrupts
            CPU0
   1:          0     GICv3  29 Edge      arch_timer
   2:        807     GICv3  30 Edge      arch_timer
   5:        712     GICv3 113 Level     rk_timer
....


> I don't know. Maybe Caesar do something wrong :(
> This is my output:
>             CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
>
> ...
>    2:       2911       1967       1588       1608       1295       1606
>     GICv3  30 Edge      arch_timer
>    5:        578        637        684        626        161        165
>     GICv3 113 Level     rk_timer
>
>
>

-- 
caesar wang | software engineer | wxt@rock-chip.com

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Daniel Lezcano]

On 06/07/2016 06:54 AM, Caesar Wang wrote:
> This series patches had been tested on rockchip inside kernel.
> In order to support the rk3399 SoC timer and turn off interrupts and IPIs to
> save power in idle.
> Okay, it still works bootup on rk3288/other SoCs, even though many socs hasn't used
> the broadcast timer.
>
> History version:
> v1:
> https://lkml.org/lkml/2016/5/25/186
>
> Easy to test for my borad.
> localhost / # cat /proc/interrupts
> CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
> 1:          0          0          0          0          0          0     GICv3  29 Edge      arch_timer
> ...
> 5:          0          0          0          0          0          0     GICv3 113 Level     rk_timer
> ..
>
> localhost / # cat /proc/timer_list | grep event_handler
> get "event_handler:  hrtimer_interrupt"
> event_handler:  tick_handle_oneshot_broadcast
> event_handler:  hrtimer_interrupt
>
> That should work for my board.

Hi Caesar,

I applied the patches on my tree. I humbly reworded the changelog, if 
you disagree with the content, let me know.

Please, in the future, take care of the following:

  - patch prefix is 'clocksource/driver/<driver_name>: [A-Z].*'

  - the change log is important, it deserves some love.

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

Re: [PATCH v2 0/4] clocksource: rockchip/timer: Support rktimer for rk3399

[Caesar Wang]

On 2016年06月16日 22:26, Daniel Lezcano wrote:
> On 06/07/2016 06:54 AM, Caesar Wang wrote:
>> This series patches had been tested on rockchip inside kernel.
>> In order to support the rk3399 SoC timer and turn off interrupts and 
>> IPIs to
>> save power in idle.
>> Okay, it still works bootup on rk3288/other SoCs, even though many 
>> socs hasn't used
>> the broadcast timer.
>>
>> History version:
>> v1:
>> https://lkml.org/lkml/2016/5/25/186
>>
>> Easy to test for my borad.
>> localhost / # cat /proc/interrupts
>> CPU0       CPU1       CPU2       CPU3       CPU4       CPU5
>> 1:          0          0          0          0 0          0     
>> GICv3  29 Edge      arch_timer
>> ...
>> 5:          0          0          0          0 0          0     GICv3 
>> 113 Level     rk_timer
>> ..
>>
>> localhost / # cat /proc/timer_list | grep event_handler
>> get "event_handler:  hrtimer_interrupt"
>> event_handler:  tick_handle_oneshot_broadcast
>> event_handler:  hrtimer_interrupt
>>
>> That should work for my board.
>
> Hi Caesar,
>
> I applied the patches on my tree. I humbly reworded the changelog, if 
> you disagree with the content, let me know.

Have a look at your tree. 
(https://git.linaro.org/people/daniel.lezcano/linux.git/shortlog/refs/heads/clockevents/next)
That's LGTM.


>
> Please, in the future, take care of the following:
>
>  - patch prefix is 'clocksource/driver/<driver_name>: [A-Z].*'
>
>  - the change log is important, it deserves some love.
>

Thanks for pointing out, I will keep in mind it.

-
Caesar

> Thanks.
>
>   -- Daniel
>
>

-- 
caesar wang | software engineer | wxt@rock-chip.com