Hi Daniel, sorry it took a bit longer to reply.
Generally it looks good to me, just some minor issues inline below. It would also be nice to include the rockchip list (linux- [email protected]) for future patches. Am Dienstag, 6. Januar 2015, 12:03:53 schrieb Daniel Lezcano: > 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. > > This driver provides the basic timer functionnality as a backup for the > local timers at sleep time. > > The timer belongs to the alive subsystem. It includes two programmables 64 > bits timer channels but the driver only uses 32bits. It works with two > operations mode: free running and user defined count. > > Programing sequence: > > 1. Timer initialization: > * Disable the timer by writing '0' to the CONTROLREG register > * Program the timer mode by writing the mode to the CONTROLREG register > * Set the interrupt mask > > 2. Setting the count value: > * Load the count value to the registers COUNT0 and COUNT1 (not used). > > 3. Enable the timer > * Write '1' to the CONTROLREG register with the mode (free running or user) > > Signed-off-by: Daniel Lezcano <[email protected]> > --- > .../bindings/timer/rockchip,rk3288-timer.txt | 15 ++ > arch/arm/boot/dts/rk3288.dtsi | 7 + > arch/arm/mach-rockchip/Kconfig | 1 + > drivers/clocksource/Kconfig | 4 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/rockchip_timer.c | 158 > +++++++++++++++++++++ 6 files changed, 186 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt create > mode 100644 drivers/clocksource/rockchip_timer.c > > diff --git > a/Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt > b/Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt new > file mode 100644 > index 0000000..54ef747 > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/rockchip,rk3288-timer.txt > @@ -0,0 +1,15 @@ > +Rockchip rk3288 timer > + > +Required properties: > +- compatible: shall be "rockchip,rk3288-timer" > +- reg: base address of the timer register starting with TIMERS CONTROL > register +- interrupts: should contain the interrupts for Timer0 > +- clock-frequency: the frequency the timer is running > + > +Example: > + timer: timer@ff810000 { > + compatible = "rockchip,rk3288-timer"; > + reg = <0xff810000 0x20>; > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > + clock-frequency = <24000000>; wouldn't it make sense to use the actual supplying clock? For the timer you want to use it is just the non-gateable &xin24m, but the timers in the other block (timer0-5) actually do have gateable clocks. Similarly there is a pclk_timer supplying at least one of the two actual blocks. I'll try to inquire how the blocks are actually supplied. > + }; > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi > index 2a878a3..7a7db48 100644 > --- a/arch/arm/boot/dts/rk3288.dtsi > +++ b/arch/arm/boot/dts/rk3288.dtsi > @@ -149,6 +149,13 @@ > clock-frequency = <24000000>; > }; > > + timer: timer@ff810000 { > + compatible = "rockchip,rk3288-timer"; > + reg = <0xff810000 0x20>; > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > + clock-frequency = <24000000>; > + }; > + > sdmmc: dwmmc@ff0c0000 { > compatible = "rockchip,rk3288-dw-mshc"; > clock-freq-min-max = <400000 150000000>; > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig > index ac5803c..4c3fa7e 100644 > --- a/arch/arm/mach-rockchip/Kconfig > +++ b/arch/arm/mach-rockchip/Kconfig > @@ -11,6 +11,7 @@ config ARCH_ROCKCHIP > select HAVE_ARM_SCU if SMP > select HAVE_ARM_TWD if SMP > select DW_APB_TIMER_OF > + select RK3288_TIMER > select ARM_GLOBAL_TIMER > select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK > help > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index fc01ec2..6ed97a6 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -26,6 +26,10 @@ config DW_APB_TIMER_OF > select DW_APB_TIMER > select CLKSRC_OF > > +config RK3288_TIMER > + bool > + select CLKSRC_OF > + the config symbol to compile rockchip_timer.c is RK3288_TIMER? I'd think it could match a bit more ... ROCKCHIP_TIMER -> rockchip_timer.c or RK3288_TIMER -> rk3288_timer.c This timer-block is actually also present on the rk3188, but not on the rk3066 which uses an unmodified dw_apb_timer. > config ARMADA_370_XP_TIMER > bool > select CLKSRC_OF > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 94d90b2..39e4f77 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_CLKBLD_I8253) += i8253.o > obj-$(CONFIG_CLKSRC_MMIO) += mmio.o > obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o > obj-$(CONFIG_DW_APB_TIMER_OF) += dw_apb_timer_of.o > +obj-$(CONFIG_RK3288_TIMER) += rockchip_timer.o > obj-$(CONFIG_CLKSRC_NOMADIK_MTU) += nomadik-mtu.o > obj-$(CONFIG_CLKSRC_DBX500_PRCMU) += clksrc-dbx500-prcmu.o > obj-$(CONFIG_ARMADA_370_XP_TIMER) += time-armada-370-xp.o > diff --git a/drivers/clocksource/rockchip_timer.c > b/drivers/clocksource/rockchip_timer.c new file mode 100644 > index 0000000..00e24bd > --- /dev/null > +++ b/drivers/clocksource/rockchip_timer.c > @@ -0,0 +1,158 @@ > +/* > + * Rockchip timer support > + * > + * Copyright (C) Daniel Lezcano <[email protected]> > + * > + * 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/init.h> > +#include <linux/interrupt.h> > +#include <linux/clockchips.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) > + > +struct bc_timer { > + struct clock_event_device ce; > + void __iomem *base; > + u32 freq; > +}; > + > +static struct bc_timer bc_timer; > + > +static inline struct bc_timer *rk_timer(struct clock_event_device *ce) > +{ > + return container_of(ce, struct bc_timer, ce); > +} > + > +static inline void __iomem *rk_base(struct clock_event_device *ce) > +{ > + return rk_timer(ce)->base; > +} > + > +static inline void rk_timer_disable(struct clock_event_device *ce) > +{ > + writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG); > + dsb(); > +} > + > +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); > + dsb(); > +} > + > +static void rk_timer_update_counter(unsigned long cycles, > + struct clock_event_device *ce) > +{ > + writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0); > + writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1); > + dsb(); > +} > + > +static void rk_timer_interrupt_clear(struct clock_event_device *ce) > +{ > + writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS); > + dsb(); > +} > + > +static inline int rk_timer_set_next_event(unsigned long cycles, > + struct clock_event_device *ce) > +{ > + rk_timer_disable(ce); > + rk_timer_update_counter(cycles, ce); > + rk_timer_enable(ce, TIMER_MODE_USER_DEFINED_COUNT); > + return 0; > +} > + > +static inline void rk_timer_set_mode(enum clock_event_mode mode, > + struct clock_event_device *ce) > +{ > + switch (mode) { > + case CLOCK_EVT_MODE_PERIODIC: > + rk_timer_disable(ce); > + rk_timer_update_counter(rk_timer(ce)->freq / HZ - 1, ce); > + rk_timer_enable(ce, TIMER_MODE_FREE_RUNNING); > + case CLOCK_EVT_MODE_ONESHOT: > + case CLOCK_EVT_MODE_RESUME: > + break; > + case CLOCK_EVT_MODE_UNUSED: > + case CLOCK_EVT_MODE_SHUTDOWN: > + rk_timer_disable(ce); > + break; > + } > +} > + > +static irqreturn_t rk_timer_interrupt(int irq, void *dev_id) > +{ > + struct clock_event_device *ce = dev_id; > + > + rk_timer_interrupt_clear(ce); > + > + if (ce->mode == CLOCK_EVT_MODE_ONESHOT) { > + rk_timer_disable(ce); > + } > + > + ce->event_handler(ce); > + > + return IRQ_HANDLED; > +} > + > +static void __init rk_timer_init(struct device_node *np) > +{ > + struct clock_event_device *ce = &bc_timer.ce; > + 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; > + } > + > + irq = irq_of_parse_and_map(np, 0); > + if (irq == NO_IRQ) { > + pr_err("Failed to map interrupts for '%s'\n", TIMER_NAME); > + return; > + } > + > + if (of_property_read_u32(np, "clock-frequency", &bc_timer.freq)) { formatting issue with spaces instead of tabs in the 5 lines above Cheers, Heiko > + pr_err("Failed to read the frequency for '%s'\n", TIMER_NAME); > + return; > + } > + > + ce->name = TIMER_NAME; > + ce->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT; > + ce->set_next_event = rk_timer_set_next_event; > + ce->set_mode = rk_timer_set_mode; > + ce->irq = irq; > + ce->cpumask = cpumask_of(0); > + ce->rating = 250; > + > + rk_timer_interrupt_clear(ce); > + rk_timer_disable(ce); > + > + ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce); > + if (ret) { > + pr_err("Failed to initialize '%s': %d\n", TIMER_NAME, ret); > + return; > + } > + > + clockevents_config_and_register(ce, bc_timer.freq, 1, UINT_MAX); > +} > +CLOCKSOURCE_OF_DECLARE(rk_timer, "rockchip,rk3288-timer", rk_timer_init); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
