Hi Daniel, one big request and some more style nitpicks :-) With the nitpicks fixed Reviewed-by: Heiko Stuebner <heiko@xxxxxxxxx> Before applying this patch could you drop the rk3288.dtsi change please? Instead I'd like to add the following patch separately to _my_ devicetree branch for 3.20. I already wasn't fast enough to prevent the ethernet controller changes going through the network tree and would like to prevent a third tree sending changes for the same dts area to Linus - merge conflicts and all. I don't think that neither the nitpicks nor dropping the dtsi segment need another submission though. Heiko ----- 8< ----------- From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> Date: Sun, 25 Jan 2015 10:42:59 +0100 Subject: [PATCH] ARM: dts: rockchip: Add rockchip timer node for rk3288 The rk3288 board uses the architected timers and these ones are shutdown when the cpu is powered down. There is a need of a broadcast timer in this case to ensure proper wakeup when the cpus are in sleep mode and a timer expires. Add the timer node for the broadcast timer. Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx> --- arch/arm/boot/dts/rk3288.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index c7235fa..37847c1 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -149,6 +149,14 @@ clock-frequency = <24000000>; }; + timer: timer@ff810000 { + compatible = "rockchip,rk3288-timer"; + reg = <0xff810000 0x20>; + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&xin24m>, <&cru PCLK_TIMER>; + clock-names = "timer", "pclk"; + }; + display-subsystem { compatible = "rockchip,display-subsystem"; ports = <&vopl_out>, <&vopb_out>; -- 2.1.1 ------- 8< ----------- Am Sonntag, 25. Januar 2015, 10:42:59 schrieb Daniel Lezcano: [...] > diff --git a/drivers/clocksource/rockchip_timer.c > b/drivers/clocksource/rockchip_timer.c new file mode 100644 > index 0000000..5ea290d > --- /dev/null > +++ b/drivers/clocksource/rockchip_timer.c > @@ -0,0 +1,180 @@ > +/* > + * Rockchip timer support > + * > + * Copyright (C) Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > + * > + * 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. > + */ > +#include <linux/clk.h> > +#include <linux/clockchips.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > + > +#define TIMER_NAME "rk_timer" > + > +#define TIMER_LOAD_COUNT0 0x00 > +#define TIMER_LOAD_COUNT1 0x04 > +#define TIMER_CONTROL_REG 0x10 > +#define TIMER_INT_STATUS 0x18 > + > +#define TIMER_DISABLE 0x0 > +#define TIMER_ENABLE 0x1 > +#define TIMER_MODE_FREE_RUNNING (0 << 1) > +#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1) > +#define TIMER_INT_UNMASK (1 << 2) the above constants are aligned using spaces I guess either simply use 1 space, or align using tabs? [...] > +static void __init rk_timer_init(struct device_node *np) > +{ > + struct clock_event_device *ce = &bc_timer.ce; > + struct clk *timer_clk; > + struct clk *pclk; > + int ret, irq; > + > + bc_timer.base = of_iomap(np, 0); > + if (!bc_timer.base) { > + pr_err("Failed to get base address for '%s'\n", TIMER_NAME); > + return; > + } > + > + pclk = of_clk_get_by_name(np, "pclk"); > + if (IS_ERR(pclk)) { the above 2 lines are indented using spaces instead of tabs > + pr_err("Failed to get pclk for '%s'\n", TIMER_NAME); > + return; > + } > + > + if (clk_prepare_enable(pclk)) { > + pr_err("Failed to enable pclk for '%s'\n", TIMER_NAME); > + return; > + } > + > + timer_clk = of_clk_get_by_name(np, "timer"); > + if (IS_ERR(timer_clk)) { same here -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html