| Message ID | 1438005755-27051-1-git-send-email-govindraj.raja@imgtec.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 07/27/2015 04:02 PM, Govindraj Raja wrote: > From: Ezequiel Garcia <ezequiel.garcia@imgtec.com> > > The Pistachio SoC provides four general purpose timers, and allow > to implement a clocksource driver. > > This driver can be used as a replacement for the MIPS GIC and MIPS R4K > clocksources and sched clocks, which are clocked from the CPU clock. > > Given the general purpose timers are clocked from an independent clock, > this new clocksource driver will be useful to introduce CPUFreq support > for Pistachio machines. > > Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> > --- > drivers/clocksource/Kconfig | 4 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/time-pistachio.c | 194 +++++++++++++++++++++++++++++++++++ > 3 files changed, 199 insertions(+) > create mode 100644 drivers/clocksource/time-pistachio.c > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 4e57730..74e002e 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -111,6 +111,10 @@ config CLKSRC_LPC32XX > select CLKSRC_MMIO > select CLKSRC_OF > > +config CLKSRC_PISTACHIO > + bool > + select CLKSRC_OF > + > config CLKSRC_STM32 > bool "Clocksource for STM32 SoCs" if !ARCH_STM32 > depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index f228354..066337e 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -44,6 +44,7 @@ obj-$(CONFIG_FSL_FTM_TIMER) += fsl_ftm_timer.o > obj-$(CONFIG_VF_PIT_TIMER) += vf_pit_timer.o > obj-$(CONFIG_CLKSRC_QCOM) += qcom-timer.o > obj-$(CONFIG_MTK_TIMER) += mtk_timer.o > +obj-$(CONFIG_CLKSRC_PISTACHIO) += time-pistachio.o > > obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o > obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o > diff --git a/drivers/clocksource/time-pistachio.c b/drivers/clocksource/time-pistachio.c > new file mode 100644 > index 0000000..d461bd1 > --- /dev/null > +++ b/drivers/clocksource/time-pistachio.c > @@ -0,0 +1,194 @@ > +/* > + * Pistachio clocksource based on general-purpose timers > + * > + * Copyright (C) 2015 Imagination Technologies > + * > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + */ > + > +#define pr_fmt(fmt) "%s: " fmt, __func__ > + > +#include <linux/clk.h> > +#include <linux/clocksource.h> > +#include <linux/clockchips.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/spinlock.h> > +#include <linux/mfd/syscon.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/sched_clock.h> > +#include <linux/time.h> > + > +/* Top level reg */ > +#define CR_TIMER_CTRL_CFG 0x00 > +#define TIMER_ME_GLOBAL BIT(0) extra space. > +#define CR_TIMER_REV 0x10 > + > +/* Timer specific registers */ > +#define TIMER_CFG 0x20 > +#define TIMER_ME_LOCAL BIT(0) extra space. > +#define TIMER_RELOAD_VALUE 0x24 > +#define TIMER_CURRENT_VALUE 0x28 > +#define TIMER_CURRENT_OVERFLOW_VALUE 0x2C > +#define TIMER_IRQ_STATUS 0x30 > +#define TIMER_IRQ_CLEAR 0x34 > +#define TIMER_IRQ_MASK 0x38 > + > +#define PERIP_TIMER_CONTROL 0x90 > + > +/* Timer specific configuration Values */ > +#define RELOAD_VALUE 0xffffffff > + > +static void __iomem *timer_base; > +static DEFINE_RAW_SPINLOCK(lock); > + > +static inline u32 gpt_readl(u32 offset, u32 gpt_id) > +{ > + return readl(timer_base + 0x20 * gpt_id + offset); > +} > + > +static inline void gpt_writel(u32 value, u32 offset, u32 gpt_id) > +{ > + writel(value, timer_base + 0x20 * gpt_id + offset); > +} > + > +static cycle_t pistachio_clocksource_read_cycles(struct clocksource *cs) > +{ > + u32 counter, overflw; > + unsigned long flags; > + > + raw_spin_lock_irqsave(&lock, flags); > + overflw = gpt_readl(TIMER_CURRENT_OVERFLOW_VALUE, 0); Why do you need to read 'overflw' here ? It is not used. > + counter = gpt_readl(TIMER_CURRENT_VALUE, 0); > + raw_spin_unlock_irqrestore(&lock, flags); > + > + return ~(cycle_t)counter; > +} > + > +static u64 notrace pistachio_read_sched_clock(void) > +{ > + return pistachio_clocksource_read_cycles(NULL); > +} > + > +static void pistachio_clksrc_enable(int timeridx) > +{ > + u32 val; > + > + /* Disable GPT local before loading reload value */ > + val = gpt_readl(TIMER_CFG, timeridx); > + val &= ~TIMER_ME_LOCAL; > + gpt_writel(val, TIMER_CFG, timeridx); > + > + gpt_writel(RELOAD_VALUE, TIMER_RELOAD_VALUE, timeridx); > + > + val = gpt_readl(TIMER_CFG, timeridx); > + val |= TIMER_ME_LOCAL; > + gpt_writel(val, TIMER_CFG, timeridx); > +} > + > +static void pistachio_clksrc_disable(int timeridx) > +{ > + u32 val; > + > + /* Disable GPT local */ > + val = gpt_readl(TIMER_CFG, timeridx); > + val &= ~TIMER_ME_LOCAL; > + gpt_writel(val, TIMER_CFG, timeridx); > +} Duplicate code with 'pistachio_clksrc_enable', please reuse this function in the enable one. > + > +static int pistachio_clocksource_enable(struct clocksource *cs) > +{ > + pistachio_clksrc_enable(0); > + return 0; > +} > + > +static void pistachio_clocksource_disable(struct clocksource *cs) > +{ > + pistachio_clksrc_disable(0); > +} It will be better if you don't wrap these function but use container_of to retrieve the timer_base from the clocksource structure. > +/* Desirable clock source for pistachio platform */ > +static struct clocksource pistachio_clocksource_gpt = { > + .name = "gptimer", > + .rating = 300, > + .enable = pistachio_clocksource_enable, > + .disable = pistachio_clocksource_disable, > + .read = pistachio_clocksource_read_cycles, > + .mask = CLOCKSOURCE_MASK(32), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | > + CLOCK_SOURCE_SUSPEND_NONSTOP, > +}; > + > +static void __init pistachio_clksrc_of_init(struct device_node *node) > +{ > + struct clk *sys_clk, *fast_clk; > + struct regmap *periph_regs; > + unsigned long rate; > + int ret; > + > + timer_base = of_iomap(node, 0); > + if (!timer_base) { > + pr_err("cannot iomap\n"); > + return; > + } > + > + periph_regs = syscon_regmap_lookup_by_phandle(node, "img,cr-periph"); > + if (IS_ERR(periph_regs)) { > + pr_err("cannot get peripheral regmap (%lu)\n", > + PTR_ERR(periph_regs)); > + return; > + } > + > + /* Switch to using the fast counter clock */ > + ret = regmap_update_bits(periph_regs, PERIP_TIMER_CONTROL, > + 0xf, 0x0); > + if (ret) > + return; > + > + sys_clk = of_clk_get_by_name(node, "sys"); > + if (IS_ERR(sys_clk)) { > + pr_err("clock get failed (%lu)\n", PTR_ERR(sys_clk)); > + return; > + } > + > + fast_clk = of_clk_get_by_name(node, "fast"); > + if (IS_ERR(fast_clk)) { > + pr_err("clock get failed (%lu)\n", PTR_ERR(fast_clk)); > + return; > + } > + > + ret = clk_prepare_enable(sys_clk); > + if (ret < 0) { > + pr_err("failed to enable clock (%d)\n", ret); > + return; > + } > + > + ret = clk_prepare_enable(fast_clk); > + if (ret < 0) { > + pr_err("failed to enable clock (%d)\n", ret); > + clk_disable_unprepare(sys_clk); > + return; > + } > + > + rate = clk_get_rate(fast_clk); > + > + /* Disable irq's for clocksource usage */ > + gpt_writel(0, TIMER_IRQ_MASK, 0); > + gpt_writel(0, TIMER_IRQ_MASK, 1); > + gpt_writel(0, TIMER_IRQ_MASK, 2); > + gpt_writel(0, TIMER_IRQ_MASK, 3); > + > + /* Enable timer block */ > + writel(TIMER_ME_GLOBAL, timer_base); > + > + sched_clock_register(pistachio_read_sched_clock, 32, rate); > + clocksource_register_hz(&pistachio_clocksource_gpt, rate); > +} > +CLOCKSOURCE_OF_DECLARE(pistachio_gptimer, "img,pistachio-gptimer", > + pistachio_clksrc_of_init); >
Hi Daniel, Thanks for the review! On 4 August 2015 at 06:21, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 07/27/2015 04:02 PM, Govindraj Raja wrote: >> >> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >> >> The Pistachio SoC provides four general purpose timers, and allow >> to implement a clocksource driver. >> >> This driver can be used as a replacement for the MIPS GIC and MIPS R4K >> clocksources and sched clocks, which are clocked from the CPU clock. >> >> Given the general purpose timers are clocked from an independent clock, >> this new clocksource driver will be useful to introduce CPUFreq support >> for Pistachio machines. >> >> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >> --- >> drivers/clocksource/Kconfig | 4 + >> drivers/clocksource/Makefile | 1 + >> drivers/clocksource/time-pistachio.c | 194 >> +++++++++++++++++++++++++++++++++++ >> 3 files changed, 199 insertions(+) >> create mode 100644 drivers/clocksource/time-pistachio.c >> >> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >> index 4e57730..74e002e 100644 >> --- a/drivers/clocksource/Kconfig >> +++ b/drivers/clocksource/Kconfig >> @@ -111,6 +111,10 @@ config CLKSRC_LPC32XX >> select CLKSRC_MMIO >> select CLKSRC_OF >> >> +config CLKSRC_PISTACHIO >> + bool >> + select CLKSRC_OF >> + >> config CLKSRC_STM32 >> bool "Clocksource for STM32 SoCs" if !ARCH_STM32 >> depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) >> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >> index f228354..066337e 100644 >> --- a/drivers/clocksource/Makefile >> +++ b/drivers/clocksource/Makefile >> @@ -44,6 +44,7 @@ obj-$(CONFIG_FSL_FTM_TIMER) += fsl_ftm_timer.o >> obj-$(CONFIG_VF_PIT_TIMER) += vf_pit_timer.o >> obj-$(CONFIG_CLKSRC_QCOM) += qcom-timer.o >> obj-$(CONFIG_MTK_TIMER) += mtk_timer.o >> +obj-$(CONFIG_CLKSRC_PISTACHIO) += time-pistachio.o >> >> obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o >> obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o >> diff --git a/drivers/clocksource/time-pistachio.c >> b/drivers/clocksource/time-pistachio.c >> new file mode 100644 >> index 0000000..d461bd1 >> --- /dev/null >> +++ b/drivers/clocksource/time-pistachio.c >> @@ -0,0 +1,194 @@ >> +/* >> + * Pistachio clocksource based on general-purpose timers >> + * >> + * Copyright (C) 2015 Imagination Technologies >> + * >> + * This file is subject to the terms and conditions of the GNU General >> Public >> + * License. See the file "COPYING" in the main directory of this archive >> + * for more details. >> + */ >> + >> +#define pr_fmt(fmt) "%s: " fmt, __func__ >> + >> +#include <linux/clk.h> >> +#include <linux/clocksource.h> >> +#include <linux/clockchips.h> >> +#include <linux/delay.h> >> +#include <linux/err.h> >> +#include <linux/init.h> >> +#include <linux/spinlock.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/of.h> >> +#include <linux/of_address.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> +#include <linux/sched_clock.h> >> +#include <linux/time.h> >> + >> +/* Top level reg */ >> +#define CR_TIMER_CTRL_CFG 0x00 >> +#define TIMER_ME_GLOBAL BIT(0) > > > extra space. > >> +#define CR_TIMER_REV 0x10 >> + >> +/* Timer specific registers */ >> +#define TIMER_CFG 0x20 >> +#define TIMER_ME_LOCAL BIT(0) > > > extra space. > These are not extra spaces, but they are there to separate register definitions from bit definitions. Same thing is done on time-armada-370-xp.c. > >> +#define TIMER_RELOAD_VALUE 0x24 >> +#define TIMER_CURRENT_VALUE 0x28 >> +#define TIMER_CURRENT_OVERFLOW_VALUE 0x2C >> +#define TIMER_IRQ_STATUS 0x30 >> +#define TIMER_IRQ_CLEAR 0x34 >> +#define TIMER_IRQ_MASK 0x38 >> + >> +#define PERIP_TIMER_CONTROL 0x90 >> + >> +/* Timer specific configuration Values */ >> +#define RELOAD_VALUE 0xffffffff >> + >> +static void __iomem *timer_base; >> +static DEFINE_RAW_SPINLOCK(lock); >> + >> +static inline u32 gpt_readl(u32 offset, u32 gpt_id) >> +{ >> + return readl(timer_base + 0x20 * gpt_id + offset); >> +} >> + >> +static inline void gpt_writel(u32 value, u32 offset, u32 gpt_id) >> +{ >> + writel(value, timer_base + 0x20 * gpt_id + offset); >> +} >> + >> +static cycle_t pistachio_clocksource_read_cycles(struct clocksource *cs) >> +{ >> + u32 counter, overflw; >> + unsigned long flags; >> + >> + raw_spin_lock_irqsave(&lock, flags); >> + overflw = gpt_readl(TIMER_CURRENT_OVERFLOW_VALUE, 0); > > > Why do you need to read 'overflw' here ? It is not used. > The counter value is only refreshed after the overflow value is read. And they must be read in strict order, hence the ugly raw spin lock. Without both of these, the CPU locks up completely when reading the counter. Maybe a comment explaining this would help. > >> + counter = gpt_readl(TIMER_CURRENT_VALUE, 0); >> + raw_spin_unlock_irqrestore(&lock, flags); >> + >> + return ~(cycle_t)counter; >> +} >> + >> +static u64 notrace pistachio_read_sched_clock(void) >> +{ >> + return pistachio_clocksource_read_cycles(NULL); >> +} >> + >> +static void pistachio_clksrc_enable(int timeridx) >> +{ >> + u32 val; >> + >> + /* Disable GPT local before loading reload value */ >> + val = gpt_readl(TIMER_CFG, timeridx); >> + val &= ~TIMER_ME_LOCAL; >> + gpt_writel(val, TIMER_CFG, timeridx); >> + >> + gpt_writel(RELOAD_VALUE, TIMER_RELOAD_VALUE, timeridx); >> + >> + val = gpt_readl(TIMER_CFG, timeridx); >> + val |= TIMER_ME_LOCAL; >> + gpt_writel(val, TIMER_CFG, timeridx); >> +} >> + >> +static void pistachio_clksrc_disable(int timeridx) >> +{ >> + u32 val; >> + >> + /* Disable GPT local */ >> + val = gpt_readl(TIMER_CFG, timeridx); >> + val &= ~TIMER_ME_LOCAL; >> + gpt_writel(val, TIMER_CFG, timeridx); >> +} > > > Duplicate code with 'pistachio_clksrc_enable', please reuse this function in > the enable one. > OK. >> + >> +static int pistachio_clocksource_enable(struct clocksource *cs) >> +{ >> + pistachio_clksrc_enable(0); >> + return 0; >> +} >> + >> +static void pistachio_clocksource_disable(struct clocksource *cs) >> +{ >> + pistachio_clksrc_disable(0); >> +} > > > It will be better if you don't wrap these function but use container_of to > retrieve the timer_base from the clocksource structure. > I'm not sure I understand what you mean. We are not using clocksource_mmio and there's no driver-specific structure. > >> +/* Desirable clock source for pistachio platform */ >> +static struct clocksource pistachio_clocksource_gpt = { >> + .name = "gptimer", >> + .rating = 300, >> + .enable = pistachio_clocksource_enable, >> + .disable = pistachio_clocksource_disable, >> + .read = pistachio_clocksource_read_cycles, >> + .mask = CLOCKSOURCE_MASK(32), >> + .flags = CLOCK_SOURCE_IS_CONTINUOUS | >> + CLOCK_SOURCE_SUSPEND_NONSTOP, >> +}; >> + >> +static void __init pistachio_clksrc_of_init(struct device_node *node) >> +{ >> + struct clk *sys_clk, *fast_clk; >> + struct regmap *periph_regs; >> + unsigned long rate; >> + int ret; >> + >> + timer_base = of_iomap(node, 0); >> + if (!timer_base) { >> + pr_err("cannot iomap\n"); >> + return; >> + } >> + >> + periph_regs = syscon_regmap_lookup_by_phandle(node, >> "img,cr-periph"); >> + if (IS_ERR(periph_regs)) { >> + pr_err("cannot get peripheral regmap (%lu)\n", >> + PTR_ERR(periph_regs)); >> + return; >> + } >> + >> + /* Switch to using the fast counter clock */ >> + ret = regmap_update_bits(periph_regs, PERIP_TIMER_CONTROL, >> + 0xf, 0x0); >> + if (ret) >> + return; >> + >> + sys_clk = of_clk_get_by_name(node, "sys"); >> + if (IS_ERR(sys_clk)) { >> + pr_err("clock get failed (%lu)\n", PTR_ERR(sys_clk)); >> + return; >> + } >> + >> + fast_clk = of_clk_get_by_name(node, "fast"); >> + if (IS_ERR(fast_clk)) { >> + pr_err("clock get failed (%lu)\n", PTR_ERR(fast_clk)); >> + return; >> + } >> + >> + ret = clk_prepare_enable(sys_clk); >> + if (ret < 0) { >> + pr_err("failed to enable clock (%d)\n", ret); >> + return; >> + } >> + >> + ret = clk_prepare_enable(fast_clk); >> + if (ret < 0) { >> + pr_err("failed to enable clock (%d)\n", ret); >> + clk_disable_unprepare(sys_clk); >> + return; >> + } >> + >> + rate = clk_get_rate(fast_clk); >> + >> + /* Disable irq's for clocksource usage */ >> + gpt_writel(0, TIMER_IRQ_MASK, 0); >> + gpt_writel(0, TIMER_IRQ_MASK, 1); >> + gpt_writel(0, TIMER_IRQ_MASK, 2); >> + gpt_writel(0, TIMER_IRQ_MASK, 3); >> + >> + /* Enable timer block */ >> + writel(TIMER_ME_GLOBAL, timer_base); >> + >> + sched_clock_register(pistachio_read_sched_clock, 32, rate); >> + clocksource_register_hz(&pistachio_clocksource_gpt, rate); >> +} >> +CLOCKSOURCE_OF_DECLARE(pistachio_gptimer, "img,pistachio-gptimer", >> + pistachio_clksrc_of_init); >> > > > -- > <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 >
On 08/04/2015 01:37 PM, Ezequiel Garcia wrote: > Hi Daniel, > > Thanks for the review! > > On 4 August 2015 at 06:21, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: >> On 07/27/2015 04:02 PM, Govindraj Raja wrote: >>> >>> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >>> >>> The Pistachio SoC provides four general purpose timers, and allow >>> to implement a clocksource driver. >>> >>> This driver can be used as a replacement for the MIPS GIC and MIPS R4K >>> clocksources and sched clocks, which are clocked from the CPU clock. >>> >>> Given the general purpose timers are clocked from an independent clock, >>> this new clocksource driver will be useful to introduce CPUFreq support >>> for Pistachio machines. >>> >>> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com> >>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com> >>> --- >>> drivers/clocksource/Kconfig | 4 + >>> drivers/clocksource/Makefile | 1 + >>> drivers/clocksource/time-pistachio.c | 194 >>> +++++++++++++++++++++++++++++++++++ >>> 3 files changed, 199 insertions(+) >>> create mode 100644 drivers/clocksource/time-pistachio.c >>> >>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >>> index 4e57730..74e002e 100644 >>> --- a/drivers/clocksource/Kconfig >>> +++ b/drivers/clocksource/Kconfig >>> @@ -111,6 +111,10 @@ config CLKSRC_LPC32XX >>> select CLKSRC_MMIO >>> select CLKSRC_OF >>> >>> +config CLKSRC_PISTACHIO >>> + bool >>> + select CLKSRC_OF >>> + >>> config CLKSRC_STM32 >>> bool "Clocksource for STM32 SoCs" if !ARCH_STM32 >>> depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) >>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >>> index f228354..066337e 100644 >>> --- a/drivers/clocksource/Makefile >>> +++ b/drivers/clocksource/Makefile >>> @@ -44,6 +44,7 @@ obj-$(CONFIG_FSL_FTM_TIMER) += fsl_ftm_timer.o >>> obj-$(CONFIG_VF_PIT_TIMER) += vf_pit_timer.o >>> obj-$(CONFIG_CLKSRC_QCOM) += qcom-timer.o >>> obj-$(CONFIG_MTK_TIMER) += mtk_timer.o >>> +obj-$(CONFIG_CLKSRC_PISTACHIO) += time-pistachio.o >>> >>> obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o >>> obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o >>> diff --git a/drivers/clocksource/time-pistachio.c >>> b/drivers/clocksource/time-pistachio.c >>> new file mode 100644 >>> index 0000000..d461bd1 >>> --- /dev/null >>> +++ b/drivers/clocksource/time-pistachio.c >>> @@ -0,0 +1,194 @@ >>> +/* >>> + * Pistachio clocksource based on general-purpose timers >>> + * >>> + * Copyright (C) 2015 Imagination Technologies >>> + * >>> + * This file is subject to the terms and conditions of the GNU General >>> Public >>> + * License. See the file "COPYING" in the main directory of this archive >>> + * for more details. >>> + */ >>> + >>> +#define pr_fmt(fmt) "%s: " fmt, __func__ >>> + >>> +#include <linux/clk.h> >>> +#include <linux/clocksource.h> >>> +#include <linux/clockchips.h> >>> +#include <linux/delay.h> >>> +#include <linux/err.h> >>> +#include <linux/init.h> >>> +#include <linux/spinlock.h> >>> +#include <linux/mfd/syscon.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/regmap.h> >>> +#include <linux/sched_clock.h> >>> +#include <linux/time.h> >>> + >>> +/* Top level reg */ >>> +#define CR_TIMER_CTRL_CFG 0x00 >>> +#define TIMER_ME_GLOBAL BIT(0) >> >> >> extra space. >> >>> +#define CR_TIMER_REV 0x10 >>> + >>> +/* Timer specific registers */ >>> +#define TIMER_CFG 0x20 >>> +#define TIMER_ME_LOCAL BIT(0) >> >> >> extra space. >> > > These are not extra spaces, but they are there > to separate register definitions from bit definitions. > Same thing is done on time-armada-370-xp.c. > >> >>> +#define TIMER_RELOAD_VALUE 0x24 >>> +#define TIMER_CURRENT_VALUE 0x28 >>> +#define TIMER_CURRENT_OVERFLOW_VALUE 0x2C >>> +#define TIMER_IRQ_STATUS 0x30 >>> +#define TIMER_IRQ_CLEAR 0x34 >>> +#define TIMER_IRQ_MASK 0x38 >>> + >>> +#define PERIP_TIMER_CONTROL 0x90 >>> + >>> +/* Timer specific configuration Values */ >>> +#define RELOAD_VALUE 0xffffffff >>> + >>> +static void __iomem *timer_base; >>> +static DEFINE_RAW_SPINLOCK(lock); >>> + >>> +static inline u32 gpt_readl(u32 offset, u32 gpt_id) >>> +{ >>> + return readl(timer_base + 0x20 * gpt_id + offset); >>> +} >>> + >>> +static inline void gpt_writel(u32 value, u32 offset, u32 gpt_id) >>> +{ >>> + writel(value, timer_base + 0x20 * gpt_id + offset); >>> +} >>> + >>> +static cycle_t pistachio_clocksource_read_cycles(struct clocksource *cs) >>> +{ >>> + u32 counter, overflw; >>> + unsigned long flags; >>> + >>> + raw_spin_lock_irqsave(&lock, flags); >>> + overflw = gpt_readl(TIMER_CURRENT_OVERFLOW_VALUE, 0); >> >> >> Why do you need to read 'overflw' here ? It is not used. >> > > The counter value is only refreshed after the overflow value is read. > And they must be read in strict order, hence the ugly raw spin lock. > Without both of these, the CPU locks up completely when reading > the counter. > > Maybe a comment explaining this would help. > >> >>> + counter = gpt_readl(TIMER_CURRENT_VALUE, 0); >>> + raw_spin_unlock_irqrestore(&lock, flags); >>> + >>> + return ~(cycle_t)counter; >>> +} >>> + >>> +static u64 notrace pistachio_read_sched_clock(void) >>> +{ >>> + return pistachio_clocksource_read_cycles(NULL); >>> +} >>> + >>> +static void pistachio_clksrc_enable(int timeridx) >>> +{ >>> + u32 val; >>> + >>> + /* Disable GPT local before loading reload value */ >>> + val = gpt_readl(TIMER_CFG, timeridx); >>> + val &= ~TIMER_ME_LOCAL; >>> + gpt_writel(val, TIMER_CFG, timeridx); >>> + >>> + gpt_writel(RELOAD_VALUE, TIMER_RELOAD_VALUE, timeridx); >>> + >>> + val = gpt_readl(TIMER_CFG, timeridx); >>> + val |= TIMER_ME_LOCAL; >>> + gpt_writel(val, TIMER_CFG, timeridx); >>> +} >>> + >>> +static void pistachio_clksrc_disable(int timeridx) >>> +{ >>> + u32 val; >>> + >>> + /* Disable GPT local */ >>> + val = gpt_readl(TIMER_CFG, timeridx); >>> + val &= ~TIMER_ME_LOCAL; >>> + gpt_writel(val, TIMER_CFG, timeridx); >>> +} >> >> >> Duplicate code with 'pistachio_clksrc_enable', please reuse this function in >> the enable one. >> > > OK. > >>> + >>> +static int pistachio_clocksource_enable(struct clocksource *cs) >>> +{ >>> + pistachio_clksrc_enable(0); >>> + return 0; >>> +} >>> + >>> +static void pistachio_clocksource_disable(struct clocksource *cs) >>> +{ >>> + pistachio_clksrc_disable(0); >>> +} >> >> >> It will be better if you don't wrap these function but use container_of to >> retrieve the timer_base from the clocksource structure. >> > > I'm not sure I understand what you mean. > We are not using clocksource_mmio and there's no driver-specific > structure. struct pistachio_clocksource { void __iomem *base; raw_spin_lock lock; struct clocksource cs; }; struct pistachio_clocksource to_pistachio_clocksource( struct clocksource *cs) { return container_of(cs, struct pistachio_clocksource, cs); } static void pistachio_clocksource_disable(struct clocksource *cs) { struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs); u32 val; /* Disable GPT local before loading reload value */ val = gpt_readl(pcs->base, TIMER_CFG, timeridx); ... ... etc ... } >> >>> +/* Desirable clock source for pistachio platform */ >>> +static struct clocksource pistachio_clocksource_gpt = { >>> + .name = "gptimer", >>> + .rating = 300, >>> + .enable = pistachio_clocksource_enable, >>> + .disable = pistachio_clocksource_disable, >>> + .read = pistachio_clocksource_read_cycles, >>> + .mask = CLOCKSOURCE_MASK(32), >>> + .flags = CLOCK_SOURCE_IS_CONTINUOUS | >>> + CLOCK_SOURCE_SUSPEND_NONSTOP, >>> +}; >>> + >>> +static void __init pistachio_clksrc_of_init(struct device_node *node) >>> +{ >>> + struct clk *sys_clk, *fast_clk; >>> + struct regmap *periph_regs; >>> + unsigned long rate; >>> + int ret; >>> + >>> + timer_base = of_iomap(node, 0); >>> + if (!timer_base) { >>> + pr_err("cannot iomap\n"); >>> + return; >>> + } >>> + >>> + periph_regs = syscon_regmap_lookup_by_phandle(node, >>> "img,cr-periph"); >>> + if (IS_ERR(periph_regs)) { >>> + pr_err("cannot get peripheral regmap (%lu)\n", >>> + PTR_ERR(periph_regs)); >>> + return; >>> + } >>> + >>> + /* Switch to using the fast counter clock */ >>> + ret = regmap_update_bits(periph_regs, PERIP_TIMER_CONTROL, >>> + 0xf, 0x0); >>> + if (ret) >>> + return; >>> + >>> + sys_clk = of_clk_get_by_name(node, "sys"); >>> + if (IS_ERR(sys_clk)) { >>> + pr_err("clock get failed (%lu)\n", PTR_ERR(sys_clk)); >>> + return; >>> + } >>> + >>> + fast_clk = of_clk_get_by_name(node, "fast"); >>> + if (IS_ERR(fast_clk)) { >>> + pr_err("clock get failed (%lu)\n", PTR_ERR(fast_clk)); >>> + return; >>> + } >>> + >>> + ret = clk_prepare_enable(sys_clk); >>> + if (ret < 0) { >>> + pr_err("failed to enable clock (%d)\n", ret); >>> + return; >>> + } >>> + >>> + ret = clk_prepare_enable(fast_clk); >>> + if (ret < 0) { >>> + pr_err("failed to enable clock (%d)\n", ret); >>> + clk_disable_unprepare(sys_clk); >>> + return; >>> + } >>> + >>> + rate = clk_get_rate(fast_clk); >>> + >>> + /* Disable irq's for clocksource usage */ >>> + gpt_writel(0, TIMER_IRQ_MASK, 0); >>> + gpt_writel(0, TIMER_IRQ_MASK, 1); >>> + gpt_writel(0, TIMER_IRQ_MASK, 2); >>> + gpt_writel(0, TIMER_IRQ_MASK, 3); >>> + >>> + /* Enable timer block */ >>> + writel(TIMER_ME_GLOBAL, timer_base); >>> + >>> + sched_clock_register(pistachio_read_sched_clock, 32, rate); >>> + clocksource_register_hz(&pistachio_clocksource_gpt, rate); >>> +} >>> +CLOCKSOURCE_OF_DECLARE(pistachio_gptimer, "img,pistachio-gptimer", >>> + pistachio_clksrc_of_init);
diff --git a/Documentation/devicetree/bindings/timer/img,pistachio-gptimer.txt b/Documentation/devicetree/bindings/timer/img,pistachio-gptimer.txt new file mode 100644 index 0000000..7afce80 --- /dev/null +++ b/Documentation/devicetree/bindings/timer/img,pistachio-gptimer.txt @@ -0,0 +1,28 @@ +* Pistachio general-purpose timer based clocksource + +Required properties: + - compatible: "img,pistachio-gptimer". + - reg: Address range of the timer registers. + - interrupts: An interrupt for each of the four timers + - clocks: Should contain a clock specifier for each entry in clock-names + - clock-names: Should contain the following entries: + "sys", interface clock + "slow", slow counter clock + "fast", fast counter clock + - img,cr-periph: Must contain a phandle to the peripheral control + syscon node. + +Example: + timer: timer@18102000 { + compatible = "img,pistachio-gptimer"; + reg = <0x18102000 0x100>; + interrupts = <GIC_SHARED 60 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 61 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 62 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 63 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_periph PERIPH_CLK_COUNTER_FAST>, + <&clk_periph PERIPH_CLK_COUNTER_SLOW>, + <&cr_periph SYS_CLK_TIMER>; + clock-names = "fast", "slow", "sys"; + img,cr-periph = <&cr_periph>; + };