* [PATCH v2 1/2] dt-bindings: clocksource: Add Spreadtrum SC9860 timer @ 2017-11-28 2:45 Baolin Wang 2017-11-28 2:45 ` [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform Baolin Wang 0 siblings, 1 reply; 4+ messages in thread From: Baolin Wang @ 2017-11-28 2:45 UTC (permalink / raw) To: daniel.lezcano, tglx, robh+dt, mark.rutland Cc: devicetree, linux-kernel, broonie, baolin.wang, baolin.wang This patch adds documentation of device tree bindings for the timers found on Spreadtrum SC9860 platform. Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com> Acked-by: Rob Herring <robh@kernel.org> --- Changes since v1: - Add acked tag from Rob. --- .../bindings/timer/spreadtrum,sprd-timer.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt diff --git a/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt new file mode 100644 index 0000000..f9d5eb9 --- /dev/null +++ b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt @@ -0,0 +1,20 @@ +Spreadtrum timers + +The Spreadtrum SC9860 platform provides 3 general-purpose timers. +These timers can support 32bit or 64bit counter, as well as supporting +period mode or one-shot mode, and they are can be wakeup source +during deep sleep. + +Required properties: +- compatible: should be "sprd,sc9860-timer" for SC9860 platform. +- reg: The register address of the timer device. +- interrupts: Should contain the interrupt for the timer device. +- clock-frequency: The frequency of the clock that drives the counter, in Hz. + +Example: + timer@40050000 { + compatible = "sprd,sc9860-timer"; + reg = <0 0x40050000 0 0x20>; + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; + clock-frequency = <32768>; + }; -- 1.7.9.5 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform 2017-11-28 2:45 [PATCH v2 1/2] dt-bindings: clocksource: Add Spreadtrum SC9860 timer Baolin Wang @ 2017-11-28 2:45 ` Baolin Wang 2017-12-07 10:44 ` Daniel Lezcano 0 siblings, 1 reply; 4+ messages in thread From: Baolin Wang @ 2017-11-28 2:45 UTC (permalink / raw) To: daniel.lezcano, tglx, robh+dt, mark.rutland Cc: devicetree, linux-kernel, broonie, baolin.wang, baolin.wang The Spreadtrum SC9860 platform will use the architected timers as local clock events, but we also need a broadcast timer device to wakeup the cpus when the cpus are in sleep mode. Thus this patch registers the timer0 to be a broadcast timer supporting periodic and oneshot events. Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com> --- Changes since v1: - Change to 32bit counter to avoid build warning. --- drivers/clocksource/Kconfig | 8 ++ drivers/clocksource/Makefile | 1 + drivers/clocksource/sprd-timer.c | 211 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 220 insertions(+) create mode 100644 drivers/clocksource/sprd-timer.c diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index c729a88..48c8702 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -441,6 +441,14 @@ config MTK_TIMER help Support for Mediatek timer driver. +config SPRD_TIMER + bool "Spreadtrum timer driver" + depends on GENERIC_CLOCKEVENTS + depends on ARCH_SPRD || COMPILE_TEST + select TIMER_OF + help + Enables the support for the Spreadtrum timer driver. + config SYS_SUPPORTS_SH_MTU2 bool diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 72711f1..62bf264 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -54,6 +54,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o obj-$(CONFIG_OWL_TIMER) += owl-timer.o +obj-$(CONFIG_SPRD_TIMER) += sprd-timer.o obj-$(CONFIG_ARC_TIMERS) += arc_timer.o obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o diff --git a/drivers/clocksource/sprd-timer.c b/drivers/clocksource/sprd-timer.c new file mode 100644 index 0000000..e90f948 --- /dev/null +++ b/drivers/clocksource/sprd-timer.c @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2017 Spreadtrum Communications Inc. + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#include <linux/clocksource.h> +#include <linux/clockchips.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/sched_clock.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> + +#define TIMER_NAME "sprd_timer" + +#define TIMER_LOAD_LO 0x0 +#define TIMER_LOAD_HI 0x4 +#define TIMER_VALUE_LO 0x8 +#define TIMER_VALUE_HI 0xc + +#define TIMER_CTL 0x10 +#define TIMER_CTL_PERIOD_MODE BIT(0) +#define TIMER_CTL_ENABLE BIT(1) +#define TIMER_CTL_64BIT_WIDTH BIT(16) + +#define TIMER_INT 0x14 +#define TIMER_INT_EN BIT(0) +#define TIMER_INT_RAW_STS BIT(1) +#define TIMER_INT_MASK_STS BIT(2) +#define TIMER_INT_CLR BIT(3) + +#define TIMER_VALUE_SHDW_LO 0x18 +#define TIMER_VALUE_SHDW_HI 0x1c + +#define TIMER_VALUE_LO_MASK GENMASK(31, 0) + +struct sprd_timer_device { + struct clock_event_device ce; + void __iomem *base; + u32 freq; + int irq; +}; + +static inline struct sprd_timer_device * +to_sprd_timer(struct clock_event_device *c) +{ + return container_of(c, struct sprd_timer_device, ce); +} + +static void sprd_timer_enable(struct sprd_timer_device *timer, u32 flag) +{ + u32 val = readl_relaxed(timer->base + TIMER_CTL); + + val |= TIMER_CTL_ENABLE; + if (flag & TIMER_CTL_64BIT_WIDTH) + val |= TIMER_CTL_64BIT_WIDTH; + else + val &= ~TIMER_CTL_64BIT_WIDTH; + + if (flag & TIMER_CTL_PERIOD_MODE) + val |= TIMER_CTL_PERIOD_MODE; + else + val &= ~TIMER_CTL_PERIOD_MODE; + + writel_relaxed(val, timer->base + TIMER_CTL); +} + +static void sprd_timer_disable(struct sprd_timer_device *timer) +{ + u32 val = readl_relaxed(timer->base + TIMER_CTL); + + val &= ~TIMER_CTL_ENABLE; + writel_relaxed(val, timer->base + TIMER_CTL); +} + +static void sprd_timer_update_counter(struct sprd_timer_device *timer, + unsigned long cycles) +{ + writel_relaxed(cycles & TIMER_VALUE_LO_MASK, + timer->base + TIMER_LOAD_LO); + writel_relaxed(0, timer->base + TIMER_LOAD_HI); +} + +static void sprd_timer_enable_interrupt(struct sprd_timer_device *timer) +{ + writel_relaxed(TIMER_INT_EN, timer->base + TIMER_INT); +} + +static void sprd_timer_clear_interrupt(struct sprd_timer_device *timer) +{ + u32 val = readl_relaxed(timer->base + TIMER_INT); + + val |= TIMER_INT_CLR; + writel_relaxed(val, timer->base + TIMER_INT); +} + +static int sprd_timer_set_next_event(unsigned long cycles, + struct clock_event_device *ce) +{ + struct sprd_timer_device *timer = to_sprd_timer(ce); + + sprd_timer_disable(timer); + sprd_timer_update_counter(timer, cycles); + sprd_timer_enable(timer, 0); + + return 0; +} + +static int sprd_timer_set_periodic(struct clock_event_device *ce) +{ + struct sprd_timer_device *timer = to_sprd_timer(ce); + unsigned long cycles = DIV_ROUND_UP(timer->freq, HZ); + + sprd_timer_disable(timer); + sprd_timer_update_counter(timer, cycles); + sprd_timer_enable(timer, TIMER_CTL_PERIOD_MODE); + + return 0; +} + +static int sprd_timer_shutdown(struct clock_event_device *ce) +{ + struct sprd_timer_device *timer = to_sprd_timer(ce); + + sprd_timer_disable(timer); + return 0; +} + +static irqreturn_t sprd_timer_interrupt(int irq, void *dev_id) +{ + struct sprd_timer_device *timer = dev_id; + + sprd_timer_clear_interrupt(timer); + + if (clockevent_state_oneshot(&timer->ce)) + sprd_timer_disable(timer); + + timer->ce.event_handler(&timer->ce); + return IRQ_HANDLED; +} + +static void __init sprd_timer_clkevt_init(struct sprd_timer_device *timer) +{ + timer->ce.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_PERIODIC | + CLOCK_EVT_FEAT_ONESHOT; + timer->ce.set_next_event = sprd_timer_set_next_event; + timer->ce.set_state_periodic = sprd_timer_set_periodic; + timer->ce.set_state_shutdown = sprd_timer_shutdown; + timer->ce.name = TIMER_NAME; + timer->ce.rating = 300; + timer->ce.irq = timer->irq; + timer->ce.cpumask = cpu_possible_mask; + + sprd_timer_enable_interrupt(timer); + clockevents_config_and_register(&timer->ce, timer->freq, 1, UINT_MAX); +} + +static int __init sprd_timer_init(struct device_node *np) +{ + struct sprd_timer_device *timer; + int ret; + + timer = kzalloc(sizeof(*timer), GFP_KERNEL); + if (!timer) + return -ENOMEM; + + ret = of_property_read_u32(np, "clock-frequency", &timer->freq); + if (ret) { + pr_err("failed to get clock frequency\n"); + goto err_freq; + } + + timer->base = of_iomap(np, 0); + if (!timer->base) { + pr_err("%s: unable to map resource\n", np->name); + ret = -ENXIO; + goto err_freq; + } + + timer->irq = irq_of_parse_and_map(np, 0); + if (timer->irq < 0) { + pr_crit("%s: unable to parse timer irq\n", np->name); + ret = timer->irq; + goto err_map_irq; + } + + ret = request_irq(timer->irq, sprd_timer_interrupt, IRQF_TIMER, + TIMER_NAME, timer); + if (ret) { + pr_err("failed to setup irq %d\n", timer->irq); + goto err_request_irq; + } + + sprd_timer_clkevt_init(timer); + + return 0; + +err_request_irq: + irq_dispose_mapping(timer->irq); +err_map_irq: + iounmap(timer->base); +err_freq: + kfree(timer); + + return ret; +} + +TIMER_OF_DECLARE(sc9860_timer, "sprd,sc9860-timer", sprd_timer_init); -- 1.7.9.5 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform 2017-11-28 2:45 ` [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform Baolin Wang @ 2017-12-07 10:44 ` Daniel Lezcano 2017-12-07 10:52 ` Baolin Wang 0 siblings, 1 reply; 4+ messages in thread From: Daniel Lezcano @ 2017-12-07 10:44 UTC (permalink / raw) To: Baolin Wang, tglx, robh+dt, mark.rutland Cc: devicetree, linux-kernel, broonie, baolin.wang On 28/11/2017 03:45, Baolin Wang wrote: > The Spreadtrum SC9860 platform will use the architected timers as local > clock events, but we also need a broadcast timer device to wakeup the > cpus when the cpus are in sleep mode. > > Thus this patch registers the timer0 to be a broadcast timer supporting > periodic and oneshot events. This changelog is inadequate. It is pointless to explain why you submit this driver. It would be much more interesting to describe the timer internals. > Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com> > --- > Changes since v1: > - Change to 32bit counter to avoid build warning. > --- > drivers/clocksource/Kconfig | 8 ++ > drivers/clocksource/Makefile | 1 + > drivers/clocksource/sprd-timer.c | 211 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 220 insertions(+) > create mode 100644 drivers/clocksource/sprd-timer.c Even if it is not a general rule, can you rename this driver to timer-sprd.c ? I would like little by little to converge all the timer names to this pattern. > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index c729a88..48c8702 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -441,6 +441,14 @@ config MTK_TIMER > help > Support for Mediatek timer driver. > > +config SPRD_TIMER > + bool "Spreadtrum timer driver" > + depends on GENERIC_CLOCKEVENTS Remove GENERIC_CLOCKEVENTS because of: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/drivers/clocksource/Kconfig#n2 > + depends on ARCH_SPRD || COMPILE_TEST Remove the dependency on ARCH_SPRD, the arch's Kconfig must select SPRD_TIMER. Move the COMPILE_TEST as the other drivers. bool "Spreadtrum timer driver" if COMPILE_TEST > + select TIMER_O> + help > + Enables the support for the Spreadtrum timer driver. > + > config SYS_SUPPORTS_SH_MTU2 > bool > > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 72711f1..62bf264 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -54,6 +54,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o > obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o > obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o > obj-$(CONFIG_OWL_TIMER) += owl-timer.o > +obj-$(CONFIG_SPRD_TIMER) += sprd-timer.o > > obj-$(CONFIG_ARC_TIMERS) += arc_timer.o > obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o > diff --git a/drivers/clocksource/sprd-timer.c b/drivers/clocksource/sprd-timer.c > new file mode 100644 > index 0000000..e90f948 > --- /dev/null > +++ b/drivers/clocksource/sprd-timer.c > @@ -0,0 +1,211 @@ > +/* > + * Copyright (C) 2017 Spreadtrum Communications Inc. > + * > + * SPDX-License-Identifier: GPL-2.0 > + */ Please fix the above with proper format (one example in timer-of.c). > +#include <linux/clocksource.h> > +#include <linux/clockchips.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/sched_clock.h> Are you sure you need sched_clock and clocksource ? > +#include <linux/slab.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> These headers won't be needed anymore when using the timer-of API (see the init function comment below). > +#define TIMER_NAME "sprd_timer" > + > +#define TIMER_LOAD_LO 0x0 > +#define TIMER_LOAD_HI 0x4 > +#define TIMER_VALUE_LO 0x8 > +#define TIMER_VALUE_HI 0xc > + > +#define TIMER_CTL 0x10 > +#define TIMER_CTL_PERIOD_MODE BIT(0) > +#define TIMER_CTL_ENABLE BIT(1) > +#define TIMER_CTL_64BIT_WIDTH BIT(16) > + > +#define TIMER_INT 0x14 > +#define TIMER_INT_EN BIT(0) > +#define TIMER_INT_RAW_STS BIT(1) > +#define TIMER_INT_MASK_STS BIT(2) > +#define TIMER_INT_CLR BIT(3) > + > +#define TIMER_VALUE_SHDW_LO 0x18 > +#define TIMER_VALUE_SHDW_HI 0x1c > + > +#define TIMER_VALUE_LO_MASK GENMASK(31, 0) > + > +struct sprd_timer_device { > + struct clock_event_device ce; > + void __iomem *base; > + u32 freq; > + int irq; > +}; > + > +static inline struct sprd_timer_device * > +to_sprd_timer(struct clock_event_device *c) > +{ > + return container_of(c, struct sprd_timer_device, ce); > +} > + > +static void sprd_timer_enable(struct sprd_timer_device *timer, u32 flag) > +{ > + u32 val = readl_relaxed(timer->base + TIMER_CTL); > + > + val |= TIMER_CTL_ENABLE; > + if (flag & TIMER_CTL_64BIT_WIDTH) > + val |= TIMER_CTL_64BIT_WIDTH; > + else > + val &= ~TIMER_CTL_64BIT_WIDTH; > + > + if (flag & TIMER_CTL_PERIOD_MODE) > + val |= TIMER_CTL_PERIOD_MODE; > + else > + val &= ~TIMER_CTL_PERIOD_MODE; > + > + writel_relaxed(val, timer->base + TIMER_CTL); > +} > + > +static void sprd_timer_disable(struct sprd_timer_device *timer) > +{ > + u32 val = readl_relaxed(timer->base + TIMER_CTL); > + > + val &= ~TIMER_CTL_ENABLE; > + writel_relaxed(val, timer->base + TIMER_CTL); > +} > + > +static void sprd_timer_update_counter(struct sprd_timer_device *timer, > + unsigned long cycles) > +{ > + writel_relaxed(cycles & TIMER_VALUE_LO_MASK, > + timer->base + TIMER_LOAD_LO); > + writel_relaxed(0, timer->base + TIMER_LOAD_HI); > +} > + > +static void sprd_timer_enable_interrupt(struct sprd_timer_device *timer) > +{ > + writel_relaxed(TIMER_INT_EN, timer->base + TIMER_INT); > +} > + > +static void sprd_timer_clear_interrupt(struct sprd_timer_device *timer) > +{ > + u32 val = readl_relaxed(timer->base + TIMER_INT); > + > + val |= TIMER_INT_CLR; > + writel_relaxed(val, timer->base + TIMER_INT); > +} > + > +static int sprd_timer_set_next_event(unsigned long cycles, > + struct clock_event_device *ce) > +{ > + struct sprd_timer_device *timer = to_sprd_timer(ce); > + > + sprd_timer_disable(timer); > + sprd_timer_update_counter(timer, cycles); > + sprd_timer_enable(timer, 0); > + > + return 0; > +} > + > +static int sprd_timer_set_periodic(struct clock_event_device *ce) > +{ > + struct sprd_timer_device *timer = to_sprd_timer(ce); > + unsigned long cycles = DIV_ROUND_UP(timer->freq, HZ); > + > + sprd_timer_disable(timer); > + sprd_timer_update_counter(timer, cycles); > + sprd_timer_enable(timer, TIMER_CTL_PERIOD_MODE); > + > + return 0; > +} > + > +static int sprd_timer_shutdown(struct clock_event_device *ce) > +{ > + struct sprd_timer_device *timer = to_sprd_timer(ce); > + > + sprd_timer_disable(timer); > + return 0; > +} > + > +static irqreturn_t sprd_timer_interrupt(int irq, void *dev_id) > +{ > + struct sprd_timer_device *timer = dev_id; > + > + sprd_timer_clear_interrupt(timer); > + > + if (clockevent_state_oneshot(&timer->ce)) > + sprd_timer_disable(timer); > + > + timer->ce.event_handler(&timer->ce); > + return IRQ_HANDLED; > +} > + > +static void __init sprd_timer_clkevt_init(struct sprd_timer_device *timer) > +{ > + timer->ce.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_PERIODIC | > + CLOCK_EVT_FEAT_ONESHOT; > + timer->ce.set_next_event = sprd_timer_set_next_event; > + timer->ce.set_state_periodic = sprd_timer_set_periodic; > + timer->ce.set_state_shutdown = sprd_timer_shutdown; > + timer->ce.name = TIMER_NAME; > + timer->ce.rating = 300; > + timer->ce.irq = timer->irq; > + timer->ce.cpumask = cpu_possible_mask; > + > + sprd_timer_enable_interrupt(timer); > + clockevents_config_and_register(&timer->ce, timer->freq, 1, UINT_MAX); > +} > + > +static int __init sprd_timer_init(struct device_node *np) > +{ > + struct sprd_timer_device *timer; > + int ret; > + > + timer = kzalloc(sizeof(*timer), GFP_KERNEL); > + if (!timer) > + return -ENOMEM; > + > + ret = of_property_read_u32(np, "clock-frequency", &timer->freq); > + if (ret) { > + pr_err("failed to get clock frequency\n"); > + goto err_freq; > + } > + > + timer->base = of_iomap(np, 0); > + if (!timer->base) { > + pr_err("%s: unable to map resource\n", np->name); > + ret = -ENXIO; > + goto err_freq; > + } > + > + timer->irq = irq_of_parse_and_map(np, 0); > + if (timer->irq < 0) { > + pr_crit("%s: unable to parse timer irq\n", np->name); > + ret = timer->irq; > + goto err_map_irq; > + } > + > + ret = request_irq(timer->irq, sprd_timer_interrupt, IRQF_TIMER, > + TIMER_NAME, timer); > + if (ret) { > + pr_err("failed to setup irq %d\n", timer->irq); > + goto err_request_irq; > + } > + sprd_timer_clkevt_init(timer); Please use the timer-of API, that will save you all these operations and checks. One example at: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/sun4i_timer.c#n143 And then use the function: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/sun4i_timer.c#n174 -- <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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform 2017-12-07 10:44 ` Daniel Lezcano @ 2017-12-07 10:52 ` Baolin Wang 0 siblings, 0 replies; 4+ messages in thread From: Baolin Wang @ 2017-12-07 10:52 UTC (permalink / raw) To: Daniel Lezcano Cc: Baolin Wang, Thomas Gleixner, Rob Herring, Mark Rutland, DTML, LKML, Mark Brown Hi Daniel, On 7 December 2017 at 18:44, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 28/11/2017 03:45, Baolin Wang wrote: >> The Spreadtrum SC9860 platform will use the architected timers as local >> clock events, but we also need a broadcast timer device to wakeup the >> cpus when the cpus are in sleep mode. >> >> Thus this patch registers the timer0 to be a broadcast timer supporting >> periodic and oneshot events. > > This changelog is inadequate. It is pointless to explain why you submit > this driver. > > It would be much more interesting to describe the timer internals. Sure. > >> Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com> >> --- >> Changes since v1: >> - Change to 32bit counter to avoid build warning. >> --- >> drivers/clocksource/Kconfig | 8 ++ >> drivers/clocksource/Makefile | 1 + >> drivers/clocksource/sprd-timer.c | 211 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 220 insertions(+) >> create mode 100644 drivers/clocksource/sprd-timer.c > > Even if it is not a general rule, can you rename this driver to > timer-sprd.c ? I would like little by little to converge all the timer > names to this pattern. OK. > >> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >> index c729a88..48c8702 100644 >> --- a/drivers/clocksource/Kconfig >> +++ b/drivers/clocksource/Kconfig >> @@ -441,6 +441,14 @@ config MTK_TIMER >> help >> Support for Mediatek timer driver. >> >> +config SPRD_TIMER >> + bool "Spreadtrum timer driver" >> + depends on GENERIC_CLOCKEVENTS > > Remove GENERIC_CLOCKEVENTS because of: > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/drivers/clocksource/Kconfig#n2 Yes, here GENERIC_CLOCKEVENTS is redundant. > >> + depends on ARCH_SPRD || COMPILE_TEST > > Remove the dependency on ARCH_SPRD, the arch's Kconfig must select > SPRD_TIMER. > > Move the COMPILE_TEST as the other drivers. OK. > > bool "Spreadtrum timer driver" if COMPILE_TEST > >> + select TIMER_O> + help >> + Enables the support for the Spreadtrum timer driver. >> + >> config SYS_SUPPORTS_SH_MTU2 >> bool >> >> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >> index 72711f1..62bf264 100644 >> --- a/drivers/clocksource/Makefile >> +++ b/drivers/clocksource/Makefile >> @@ -54,6 +54,7 @@ obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o >> obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o >> obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o >> obj-$(CONFIG_OWL_TIMER) += owl-timer.o >> +obj-$(CONFIG_SPRD_TIMER) += sprd-timer.o >> >> obj-$(CONFIG_ARC_TIMERS) += arc_timer.o >> obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o >> diff --git a/drivers/clocksource/sprd-timer.c b/drivers/clocksource/sprd-timer.c >> new file mode 100644 >> index 0000000..e90f948 >> --- /dev/null >> +++ b/drivers/clocksource/sprd-timer.c >> @@ -0,0 +1,211 @@ >> +/* >> + * Copyright (C) 2017 Spreadtrum Communications Inc. >> + * >> + * SPDX-License-Identifier: GPL-2.0 >> + */ > > Please fix the above with proper format (one example in timer-of.c). OK. > >> +#include <linux/clocksource.h> >> +#include <linux/clockchips.h> >> +#include <linux/init.h> >> +#include <linux/interrupt.h> >> +#include <linux/sched_clock.h> > > Are you sure you need sched_clock and clocksource ? Sorry, I forgot to remove them. > >> +#include <linux/slab.h> >> +#include <linux/of.h> >> +#include <linux/of_address.h> >> +#include <linux/of_irq.h> > > These headers won't be needed anymore when using the timer-of API (see > the init function comment below). OK. > >> +#define TIMER_NAME "sprd_timer" >> + >> +#define TIMER_LOAD_LO 0x0 >> +#define TIMER_LOAD_HI 0x4 >> +#define TIMER_VALUE_LO 0x8 >> +#define TIMER_VALUE_HI 0xc >> + >> +#define TIMER_CTL 0x10 >> +#define TIMER_CTL_PERIOD_MODE BIT(0) >> +#define TIMER_CTL_ENABLE BIT(1) >> +#define TIMER_CTL_64BIT_WIDTH BIT(16) >> + >> +#define TIMER_INT 0x14 >> +#define TIMER_INT_EN BIT(0) >> +#define TIMER_INT_RAW_STS BIT(1) >> +#define TIMER_INT_MASK_STS BIT(2) >> +#define TIMER_INT_CLR BIT(3) >> + >> +#define TIMER_VALUE_SHDW_LO 0x18 >> +#define TIMER_VALUE_SHDW_HI 0x1c >> + >> +#define TIMER_VALUE_LO_MASK GENMASK(31, 0) >> + >> +struct sprd_timer_device { >> + struct clock_event_device ce; >> + void __iomem *base; >> + u32 freq; >> + int irq; >> +}; >> + >> +static inline struct sprd_timer_device * >> +to_sprd_timer(struct clock_event_device *c) >> +{ >> + return container_of(c, struct sprd_timer_device, ce); >> +} >> + >> +static void sprd_timer_enable(struct sprd_timer_device *timer, u32 flag) >> +{ >> + u32 val = readl_relaxed(timer->base + TIMER_CTL); >> + >> + val |= TIMER_CTL_ENABLE; >> + if (flag & TIMER_CTL_64BIT_WIDTH) >> + val |= TIMER_CTL_64BIT_WIDTH; >> + else >> + val &= ~TIMER_CTL_64BIT_WIDTH; >> + >> + if (flag & TIMER_CTL_PERIOD_MODE) >> + val |= TIMER_CTL_PERIOD_MODE; >> + else >> + val &= ~TIMER_CTL_PERIOD_MODE; >> + >> + writel_relaxed(val, timer->base + TIMER_CTL); >> +} >> + >> +static void sprd_timer_disable(struct sprd_timer_device *timer) >> +{ >> + u32 val = readl_relaxed(timer->base + TIMER_CTL); >> + >> + val &= ~TIMER_CTL_ENABLE; >> + writel_relaxed(val, timer->base + TIMER_CTL); >> +} >> + >> +static void sprd_timer_update_counter(struct sprd_timer_device *timer, >> + unsigned long cycles) >> +{ >> + writel_relaxed(cycles & TIMER_VALUE_LO_MASK, >> + timer->base + TIMER_LOAD_LO); >> + writel_relaxed(0, timer->base + TIMER_LOAD_HI); >> +} >> + >> +static void sprd_timer_enable_interrupt(struct sprd_timer_device *timer) >> +{ >> + writel_relaxed(TIMER_INT_EN, timer->base + TIMER_INT); >> +} >> + >> +static void sprd_timer_clear_interrupt(struct sprd_timer_device *timer) >> +{ >> + u32 val = readl_relaxed(timer->base + TIMER_INT); >> + >> + val |= TIMER_INT_CLR; >> + writel_relaxed(val, timer->base + TIMER_INT); >> +} >> + >> +static int sprd_timer_set_next_event(unsigned long cycles, >> + struct clock_event_device *ce) >> +{ >> + struct sprd_timer_device *timer = to_sprd_timer(ce); >> + >> + sprd_timer_disable(timer); >> + sprd_timer_update_counter(timer, cycles); >> + sprd_timer_enable(timer, 0); >> + >> + return 0; >> +} >> + >> +static int sprd_timer_set_periodic(struct clock_event_device *ce) >> +{ >> + struct sprd_timer_device *timer = to_sprd_timer(ce); >> + unsigned long cycles = DIV_ROUND_UP(timer->freq, HZ); >> + >> + sprd_timer_disable(timer); >> + sprd_timer_update_counter(timer, cycles); >> + sprd_timer_enable(timer, TIMER_CTL_PERIOD_MODE); >> + >> + return 0; >> +} >> + >> +static int sprd_timer_shutdown(struct clock_event_device *ce) >> +{ >> + struct sprd_timer_device *timer = to_sprd_timer(ce); >> + >> + sprd_timer_disable(timer); >> + return 0; >> +} >> + >> +static irqreturn_t sprd_timer_interrupt(int irq, void *dev_id) >> +{ >> + struct sprd_timer_device *timer = dev_id; >> + >> + sprd_timer_clear_interrupt(timer); >> + >> + if (clockevent_state_oneshot(&timer->ce)) >> + sprd_timer_disable(timer); >> + >> + timer->ce.event_handler(&timer->ce); >> + return IRQ_HANDLED; >> +} >> + >> +static void __init sprd_timer_clkevt_init(struct sprd_timer_device *timer) >> +{ >> + timer->ce.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_PERIODIC | >> + CLOCK_EVT_FEAT_ONESHOT; >> + timer->ce.set_next_event = sprd_timer_set_next_event; >> + timer->ce.set_state_periodic = sprd_timer_set_periodic; >> + timer->ce.set_state_shutdown = sprd_timer_shutdown; >> + timer->ce.name = TIMER_NAME; >> + timer->ce.rating = 300; >> + timer->ce.irq = timer->irq; >> + timer->ce.cpumask = cpu_possible_mask; >> + >> + sprd_timer_enable_interrupt(timer); >> + clockevents_config_and_register(&timer->ce, timer->freq, 1, UINT_MAX); >> +} >> + >> +static int __init sprd_timer_init(struct device_node *np) >> +{ >> + struct sprd_timer_device *timer; >> + int ret; >> + >> + timer = kzalloc(sizeof(*timer), GFP_KERNEL); >> + if (!timer) >> + return -ENOMEM; >> + >> + ret = of_property_read_u32(np, "clock-frequency", &timer->freq); >> + if (ret) { >> + pr_err("failed to get clock frequency\n"); >> + goto err_freq; >> + } >> + >> + timer->base = of_iomap(np, 0); >> + if (!timer->base) { >> + pr_err("%s: unable to map resource\n", np->name); >> + ret = -ENXIO; >> + goto err_freq; >> + } >> + >> + timer->irq = irq_of_parse_and_map(np, 0); >> + if (timer->irq < 0) { >> + pr_crit("%s: unable to parse timer irq\n", np->name); >> + ret = timer->irq; >> + goto err_map_irq; >> + } >> + >> + ret = request_irq(timer->irq, sprd_timer_interrupt, IRQF_TIMER, >> + TIMER_NAME, timer); >> + if (ret) { >> + pr_err("failed to setup irq %d\n", timer->irq); >> + goto err_request_irq; >> + } >> + sprd_timer_clkevt_init(timer); > > Please use the timer-of API, that will save you all these operations and > checks. > > One example at: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/sun4i_timer.c#n143 > > And then use the function: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/sun4i_timer.c#n174 > I missed timer-of APIs, and I will check it and use timer-of APIs. Really appreciated for your useful comments. -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-07 10:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-28 2:45 [PATCH v2 1/2] dt-bindings: clocksource: Add Spreadtrum SC9860 timer Baolin Wang 2017-11-28 2:45 ` [PATCH v2 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform Baolin Wang 2017-12-07 10:44 ` Daniel Lezcano 2017-12-07 10:52 ` Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®