| Message ID | 20180611155001.3506-5-peron.clem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Reintroduce i.MX EPIT Timer | expand |
On 11/06/2018 17:50, Clément Péron wrote: > From: Colin Didier <colin.didier@devialet.com> > > Add driver for NXP's EPIT timer used in i.MX SoC. Give a description on how works this timer. > Signed-off-by: Colin Didier <colin.didier@devialet.com> > Signed-off-by: Clément Peron <clement.peron@devialet.com> > Reviewed-by: Vladimir Zapolskiy <vz@mleia.com> > Tested-by: Vladimir Zapolskiy <vz@mleia.com> > --- > drivers/clocksource/Kconfig | 11 ++ > drivers/clocksource/Makefile | 1 + > drivers/clocksource/timer-imx-epit.c | 265 +++++++++++++++++++++++++++ > 3 files changed, 277 insertions(+) > create mode 100644 drivers/clocksource/timer-imx-epit.c > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 8e8a09755d10..790478afd02c 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -576,6 +576,17 @@ config H8300_TPU > This enables the clocksource for the H8300 platform with the > H8S2678 cpu. > > +config CLKSRC_IMX_EPIT > + bool "Clocksource using i.MX EPIT" > + depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST) > + select CLKSRC_MMIO > + help > + This enables EPIT support available on some i.MX platforms. > + Normally you don't have a reason to do so as the EPIT has > + the same features and uses the same clocks as the GPT. > + Anyway, on some systems the GPT may be in use for other > + purposes. > + > config CLKSRC_IMX_GPT > bool "Clocksource using i.MX GPT" if COMPILE_TEST > depends on ARM && CLKDEV_LOOKUP > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 00caf37e52f9..d9426f69ec69 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER) += timer-integrator-ap.o > obj-$(CONFIG_CLKSRC_VERSATILE) += versatile.o > obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o > obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o > +obj-$(CONFIG_CLKSRC_IMX_EPIT) += timer-imx-epit.o > obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o > obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o > obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o > diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c > new file mode 100644 > index 000000000000..7f1c8e2e00aa > --- /dev/null > +++ b/drivers/clocksource/timer-imx-epit.c > @@ -0,0 +1,265 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * i.MX EPIT Timer > + * > + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de> > + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com> > + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/clockchips.h> > +#include <linux/interrupt.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/sched_clock.h> > +#include <linux/slab.h> > + > +#define EPITCR 0x00 > +#define EPITSR 0x04 > +#define EPITLR 0x08 > +#define EPITCMPR 0x0c > +#define EPITCNR 0x10 > + > +#define EPITCR_EN BIT(0) > +#define EPITCR_ENMOD BIT(1) > +#define EPITCR_OCIEN BIT(2) > +#define EPITCR_RLD BIT(3) > +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4) > +#define EPITCR_SWR BIT(16) > +#define EPITCR_IOVW BIT(17) > +#define EPITCR_DBGEN BIT(18) > +#define EPITCR_WAITEN BIT(19) > +#define EPITCR_RES BIT(20) > +#define EPITCR_STOPEN BIT(21) > +#define EPITCR_OM_DISCON (0 << 22) > +#define EPITCR_OM_TOGGLE (1 << 22) > +#define EPITCR_OM_CLEAR (2 << 22) > +#define EPITCR_OM_SET (3 << 22) > +#define EPITCR_CLKSRC_OFF (0 << 24) > +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24) > +#define EPITCR_CLKSRC_REF_HIGH (2 << 24) > +#define EPITCR_CLKSRC_REF_LOW (3 << 24) > + > +#define EPITSR_OCIF BIT(0) > + > +struct epit_timer { > + void __iomem *base; > + int irq; > + struct clk *clk; > + struct clock_event_device ced; > + struct irqaction act; > +}; > + > +static void __iomem *sched_clock_reg; > + > +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced) > +{ > + return container_of(ced, struct epit_timer, ced); > +} > + > +static inline void epit_irq_disable(struct epit_timer *epittm) > +{ > + u32 val; > + > + val = readl_relaxed(epittm->base + EPITCR); > + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR); > +} > + > +static inline void epit_irq_enable(struct epit_timer *epittm) > +{ > + u32 val; > + > + val = readl_relaxed(epittm->base + EPITCR); > + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR); > +} > + > +static void epit_irq_acknowledge(struct epit_timer *epittm) > +{ > + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR); > +} > + > +static u64 notrace epit_read_sched_clock(void) > +{ > + return ~readl_relaxed(sched_clock_reg); > +} > + > +static int epit_set_next_event(unsigned long cycles, > + struct clock_event_device *ced) > +{ > + struct epit_timer *epittm = to_epit_timer(ced); > + unsigned long tcmp; > + > + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles; > + writel_relaxed(tcmp, epittm->base + EPITCMPR); > + > + return 0; > +} > + > +/* Left event sources disabled, no more interrupts appear */ > +static int epit_shutdown(struct clock_event_device *ced) > +{ > + struct epit_timer *epittm = to_epit_timer(ced); > + unsigned long flags; > + > + /* > + * The timer interrupt generation is disabled at least > + * for enough time to call epit_set_next_event() > + */ > + local_irq_save(flags); This is not necessary. This function is called with interrupt disabled. > + /* Disable interrupt in EPIT module */ > + epit_irq_disable(epittm); > + > + /* Clear pending interrupt */ > + epit_irq_acknowledge(epittm); No irq ack is needed here. Neither disabling the interrupt. Why not just stop the counter ? > + local_irq_restore(flags); > + > + return 0; > +} > + > +static int epit_set_oneshot(struct clock_event_device *ced) > +{ > + struct epit_timer *epittm = to_epit_timer(ced); > + unsigned long flags; > + > + /* > + * The timer interrupt generation is disabled at least > + * for enough time to call epit_set_next_event() > + */ > + local_irq_save(flags); This is not necessary. This function is called with interrupt disabled. > + /* Disable interrupt in EPIT module */ > + epit_irq_disable(epittm); > + > + /* Clear pending interrupt, only while switching mode */ > + if (!clockevent_state_oneshot(ced)) > + epit_irq_acknowledge(epittm); Why do you need to ack the interrupt here ? > + /* > + * Do not put overhead of interrupt enable/disable into > + * epit_set_next_event(), the core has about 4 minutes > + * to call epit_set_next_event() or shutdown clock after > + * mode switching > + */ > + epit_irq_enable(epittm); > + local_irq_restore(flags); > + > + return 0; > +} > + > +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id) > +{ > + struct clock_event_device *ced = dev_id; > + struct epit_timer *epittm = to_epit_timer(ced); > + > + epit_irq_acknowledge(epittm); > + > + ced->event_handler(ced); > + > + return IRQ_HANDLED; > +} > + > +static int __init epit_clocksource_init(struct epit_timer *epittm) > +{ > + unsigned int c = clk_get_rate(epittm->clk); > + > + sched_clock_reg = epittm->base + EPITCNR; > + sched_clock_register(epit_read_sched_clock, 32, c); > + > + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32, > + clocksource_mmio_readl_down); > +} > + > +static int __init epit_clockevent_init(struct epit_timer *epittm) > +{ > + struct clock_event_device *ced = &epittm->ced; > + struct irqaction *act = &epittm->act; > + > + ced->name = "epit"; > + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ; > + ced->set_state_shutdown = epit_shutdown; > + ced->tick_resume = epit_shutdown; > + ced->set_state_oneshot = epit_set_oneshot; > + ced->set_next_event = epit_set_next_event; > + ced->rating = 200; > + ced->cpumask = cpumask_of(0); > + ced->irq = epittm->irq; > + clockevents_config_and_register(ced, clk_get_rate(epittm->clk), > + 0xff, 0xfffffffe); > + > + act->name = "i.MX EPIT Timer Tick", > + act->flags = IRQF_TIMER | IRQF_IRQPOLL; > + act->handler = epit_timer_interrupt; > + act->dev_id = ced; > + > + /* Make irqs happen */ > + return setup_irq(epittm->irq, act); > +} > + > +static int __init epit_timer_init(struct device_node *np) > +{ > + struct epit_timer *epittm; > + int ret; > + > + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); > + if (!epittm) > + return -ENOMEM; > + > + epittm->base = of_iomap(np, 0); > + if (!epittm->base) { > + ret = -ENXIO; > + goto out_kfree; > + } > + > + epittm->irq = irq_of_parse_and_map(np, 0); > + if (!epittm->irq) { > + ret = -EINVAL; > + goto out_iounmap; > + } > + > + /* Get EPIT clock */ > + epittm->clk = of_clk_get(np, 0); > + if (IS_ERR(epittm->clk)) { > + pr_err("i.MX EPIT: unable to get clk\n"); > + ret = PTR_ERR(epittm->clk); > + goto out_iounmap; > + } > + > + ret = clk_prepare_enable(epittm->clk); > + if (ret) { > + pr_err("i.MX EPIT: unable to prepare+enable clk\n"); > + goto out_iounmap; > + } Please replace all the above with the timer-of API as: https://patchwork.kernel.org/patch/10510443/ > + /* Initialise to a known state (all timers off, and timing reset) */ > + writel_relaxed(0x0, epittm->base + EPITCR); > + writel_relaxed(0xffffffff, epittm->base + EPITLR); > + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN, > + epittm->base + EPITCR); > + > + ret = epit_clocksource_init(epittm); > + if (ret) { > + pr_err("i.MX EPIT: failed to init clocksource\n"); > + goto out_clk_disable; > + } > + > + ret = epit_clockevent_init(epittm); > + if (ret) { > + pr_err("i.MX EPIT: failed to init clockevent\n"); > + goto out_clk_disable; > + } > + > + return 0; > + > +out_clk_disable: > + clk_disable_unprepare(epittm->clk); > +out_iounmap: > + iounmap(epittm->base); > +out_kfree: > + kfree(epittm); > + > + return ret; > +} > +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init); >
Hi Daniel, Thanks for the review and sorry for the delay. On Tue, 10 Jul 2018 at 18:20, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 11/06/2018 17:50, Clément Péron wrote: > > From: Colin Didier <colin.didier@devialet.com> > > > > Add driver for NXP's EPIT timer used in i.MX SoC. > > Give a description on how works this timer. Ok, > > > > Signed-off-by: Colin Didier <colin.didier@devialet.com> > > Signed-off-by: Clément Peron <clement.peron@devialet.com> > > Reviewed-by: Vladimir Zapolskiy <vz@mleia.com> > > Tested-by: Vladimir Zapolskiy <vz@mleia.com> > > --- > > drivers/clocksource/Kconfig | 11 ++ > > drivers/clocksource/Makefile | 1 + > > drivers/clocksource/timer-imx-epit.c | 265 +++++++++++++++++++++++++++ > > 3 files changed, 277 insertions(+) > > create mode 100644 drivers/clocksource/timer-imx-epit.c > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > index 8e8a09755d10..790478afd02c 100644 > > --- a/drivers/clocksource/Kconfig > > +++ b/drivers/clocksource/Kconfig > > @@ -576,6 +576,17 @@ config H8300_TPU > > This enables the clocksource for the H8300 platform with the > > H8S2678 cpu. > > > > +config CLKSRC_IMX_EPIT > > + bool "Clocksource using i.MX EPIT" > > + depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST) > > + select CLKSRC_MMIO > > + help > > + This enables EPIT support available on some i.MX platforms. > > + Normally you don't have a reason to do so as the EPIT has > > + the same features and uses the same clocks as the GPT. > > + Anyway, on some systems the GPT may be in use for other > > + purposes. > > + > > config CLKSRC_IMX_GPT > > bool "Clocksource using i.MX GPT" if COMPILE_TEST > > depends on ARM && CLKDEV_LOOKUP > > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > > index 00caf37e52f9..d9426f69ec69 100644 > > --- a/drivers/clocksource/Makefile > > +++ b/drivers/clocksource/Makefile > > @@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER) += timer-integrator-ap.o > > obj-$(CONFIG_CLKSRC_VERSATILE) += versatile.o > > obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o > > obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o > > +obj-$(CONFIG_CLKSRC_IMX_EPIT) += timer-imx-epit.o > > obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o > > obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o > > obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o > > diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c > > new file mode 100644 > > index 000000000000..7f1c8e2e00aa > > --- /dev/null > > +++ b/drivers/clocksource/timer-imx-epit.c > > @@ -0,0 +1,265 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * i.MX EPIT Timer > > + * > > + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de> > > + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com> > > + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com> > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/clockchips.h> > > +#include <linux/interrupt.h> > > +#include <linux/of_address.h> > > +#include <linux/of_irq.h> > > +#include <linux/sched_clock.h> > > +#include <linux/slab.h> > > + > > +#define EPITCR 0x00 > > +#define EPITSR 0x04 > > +#define EPITLR 0x08 > > +#define EPITCMPR 0x0c > > +#define EPITCNR 0x10 > > + > > +#define EPITCR_EN BIT(0) > > +#define EPITCR_ENMOD BIT(1) > > +#define EPITCR_OCIEN BIT(2) > > +#define EPITCR_RLD BIT(3) > > +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4) > > +#define EPITCR_SWR BIT(16) > > +#define EPITCR_IOVW BIT(17) > > +#define EPITCR_DBGEN BIT(18) > > +#define EPITCR_WAITEN BIT(19) > > +#define EPITCR_RES BIT(20) > > +#define EPITCR_STOPEN BIT(21) > > +#define EPITCR_OM_DISCON (0 << 22) > > +#define EPITCR_OM_TOGGLE (1 << 22) > > +#define EPITCR_OM_CLEAR (2 << 22) > > +#define EPITCR_OM_SET (3 << 22) > > +#define EPITCR_CLKSRC_OFF (0 << 24) > > +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24) > > +#define EPITCR_CLKSRC_REF_HIGH (2 << 24) > > +#define EPITCR_CLKSRC_REF_LOW (3 << 24) > > + > > +#define EPITSR_OCIF BIT(0) > > + > > +struct epit_timer { > > + void __iomem *base; > > + int irq; > > + struct clk *clk; > > + struct clock_event_device ced; > > + struct irqaction act; > > +}; > > + > > +static void __iomem *sched_clock_reg; > > + > > +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced) > > +{ > > + return container_of(ced, struct epit_timer, ced); > > +} > > + > > +static inline void epit_irq_disable(struct epit_timer *epittm) > > +{ > > + u32 val; > > + > > + val = readl_relaxed(epittm->base + EPITCR); > > + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR); > > +} > > + > > +static inline void epit_irq_enable(struct epit_timer *epittm) > > +{ > > + u32 val; > > + > > + val = readl_relaxed(epittm->base + EPITCR); > > + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR); > > +} > > + > > +static void epit_irq_acknowledge(struct epit_timer *epittm) > > +{ > > + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR); > > +} > > + > > +static u64 notrace epit_read_sched_clock(void) > > +{ > > + return ~readl_relaxed(sched_clock_reg); > > +} > > + > > +static int epit_set_next_event(unsigned long cycles, > > + struct clock_event_device *ced) > > +{ > > + struct epit_timer *epittm = to_epit_timer(ced); > > + unsigned long tcmp; > > + > > + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles; > > + writel_relaxed(tcmp, epittm->base + EPITCMPR); > > + > > + return 0; > > +} > > + > > +/* Left event sources disabled, no more interrupts appear */ > > +static int epit_shutdown(struct clock_event_device *ced) > > +{ > > + struct epit_timer *epittm = to_epit_timer(ced); > > + unsigned long flags; > > + > > + /* > > + * The timer interrupt generation is disabled at least > > + * for enough time to call epit_set_next_event() > > + */ > > + local_irq_save(flags); > > This is not necessary. This function is called with interrupt disabled. I took this from the i.MX GPT Timer : https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/timer-imx-gpt.c#L208 Do you think i should also fix on the imx gpt timer ? > > > + /* Disable interrupt in EPIT module */ > > + epit_irq_disable(epittm); > > + > > + /* Clear pending interrupt */ > > + epit_irq_acknowledge(epittm); > > No irq ack is needed here. Neither disabling the interrupt. Is it done by the framework ? I took also this from the IMX GPT driver > > Why not just stop the counter ? I will check the datasheet. > > > + local_irq_restore(flags); > > + > > + return 0; > > +} > > + > > +static int epit_set_oneshot(struct clock_event_device *ced) > > +{ > > + struct epit_timer *epittm = to_epit_timer(ced); > > + unsigned long flags; > > + > > + /* > > + * The timer interrupt generation is disabled at least > > + * for enough time to call epit_set_next_event() > > + */ > > + local_irq_save(flags); > > This is not necessary. This function is called with interrupt disabled. > > > + /* Disable interrupt in EPIT module */ > > + epit_irq_disable(epittm); > > + > > + /* Clear pending interrupt, only while switching mode */ > > + if (!clockevent_state_oneshot(ced)) > > + epit_irq_acknowledge(epittm); > > Why do you need to ack the interrupt here ? > > > + /* > > + * Do not put overhead of interrupt enable/disable into > > + * epit_set_next_event(), the core has about 4 minutes > > + * to call epit_set_next_event() or shutdown clock after > > + * mode switching > > + */ > > + epit_irq_enable(epittm); > > + local_irq_restore(flags); > > + > > + return 0; > > +} > > + > > +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id) > > +{ > > + struct clock_event_device *ced = dev_id; > > + struct epit_timer *epittm = to_epit_timer(ced); > > + > > + epit_irq_acknowledge(epittm); > > + > > + ced->event_handler(ced); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int __init epit_clocksource_init(struct epit_timer *epittm) > > +{ > > + unsigned int c = clk_get_rate(epittm->clk); > > + > > + sched_clock_reg = epittm->base + EPITCNR; > > + sched_clock_register(epit_read_sched_clock, 32, c); > > + > > + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32, > > + clocksource_mmio_readl_down); > > +} > > + > > +static int __init epit_clockevent_init(struct epit_timer *epittm) > > +{ > > + struct clock_event_device *ced = &epittm->ced; > > + struct irqaction *act = &epittm->act; > > + > > + ced->name = "epit"; > > + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ; > > + ced->set_state_shutdown = epit_shutdown; > > + ced->tick_resume = epit_shutdown; > > + ced->set_state_oneshot = epit_set_oneshot; > > + ced->set_next_event = epit_set_next_event; > > + ced->rating = 200; > > + ced->cpumask = cpumask_of(0); > > + ced->irq = epittm->irq; > > + clockevents_config_and_register(ced, clk_get_rate(epittm->clk), > > + 0xff, 0xfffffffe); > > + > > + act->name = "i.MX EPIT Timer Tick", > > + act->flags = IRQF_TIMER | IRQF_IRQPOLL; > > + act->handler = epit_timer_interrupt; > > + act->dev_id = ced; > > + > > + /* Make irqs happen */ > > + return setup_irq(epittm->irq, act); > > +} > > + > > +static int __init epit_timer_init(struct device_node *np) > > +{ > > + struct epit_timer *epittm; > > + int ret; > > + > > + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); > > + if (!epittm) > > + return -ENOMEM; > > + > > + epittm->base = of_iomap(np, 0); > > + if (!epittm->base) { > > + ret = -ENXIO; > > + goto out_kfree; > > + } > > + > > + epittm->irq = irq_of_parse_and_map(np, 0); > > + if (!epittm->irq) { > > + ret = -EINVAL; > > + goto out_iounmap; > > + } > > + > > + /* Get EPIT clock */ > > + epittm->clk = of_clk_get(np, 0); > > + if (IS_ERR(epittm->clk)) { > > + pr_err("i.MX EPIT: unable to get clk\n"); > > + ret = PTR_ERR(epittm->clk); > > + goto out_iounmap; > > + } > > + > > + ret = clk_prepare_enable(epittm->clk); > > + if (ret) { > > + pr_err("i.MX EPIT: unable to prepare+enable clk\n"); > > + goto out_iounmap; > > + } > > Please replace all the above with the timer-of API as: Ok I will, Thanks Clement > > https://patchwork.kernel.org/patch/10510443/ > > > > + /* Initialise to a known state (all timers off, and timing reset) */ > > + writel_relaxed(0x0, epittm->base + EPITCR); > > + writel_relaxed(0xffffffff, epittm->base + EPITLR); > > + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN, > > + epittm->base + EPITCR); > > + > > + ret = epit_clocksource_init(epittm); > > + if (ret) { > > + pr_err("i.MX EPIT: failed to init clocksource\n"); > > + goto out_clk_disable; > > + } > > + > > + ret = epit_clockevent_init(epittm); > > + if (ret) { > > + pr_err("i.MX EPIT: failed to init clockevent\n"); > > + goto out_clk_disable; > > + } > > + > > + return 0; > > + > > +out_clk_disable: > > + clk_disable_unprepare(epittm->clk); > > +out_iounmap: > > + iounmap(epittm->base); > > +out_kfree: > > + kfree(epittm); > > + > > + return ret; > > +} > > +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_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 >
Hi Clément, On 04/11/2018 17:07, Clément Péron wrote: > Hi Daniel, > > Thanks for the review and sorry for the delay. no problem. > On Tue, 10 Jul 2018 at 18:20, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: >> >> On 11/06/2018 17:50, Clément Péron wrote: >>> From: Colin Didier <colin.didier@devialet.com> >>> >>> Add driver for NXP's EPIT timer used in i.MX SoC. >> >> Give a description on how works this timer. > Ok, > >> >> >>> Signed-off-by: Colin Didier <colin.didier@devialet.com> >>> Signed-off-by: Clément Peron <clement.peron@devialet.com> >>> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com> >>> Tested-by: Vladimir Zapolskiy <vz@mleia.com> >>> --- >>> drivers/clocksource/Kconfig | 11 ++ >>> drivers/clocksource/Makefile | 1 + >>> drivers/clocksource/timer-imx-epit.c | 265 +++++++++++++++++++++++++++ >>> 3 files changed, 277 insertions(+) >>> create mode 100644 drivers/clocksource/timer-imx-epit.c >>> >>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >>> index 8e8a09755d10..790478afd02c 100644 >>> --- a/drivers/clocksource/Kconfig >>> +++ b/drivers/clocksource/Kconfig >>> @@ -576,6 +576,17 @@ config H8300_TPU >>> This enables the clocksource for the H8300 platform with the >>> H8S2678 cpu. >>> >>> +config CLKSRC_IMX_EPIT >>> + bool "Clocksource using i.MX EPIT" >>> + depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST) >>> + select CLKSRC_MMIO >>> + help >>> + This enables EPIT support available on some i.MX platforms. >>> + Normally you don't have a reason to do so as the EPIT has >>> + the same features and uses the same clocks as the GPT. >>> + Anyway, on some systems the GPT may be in use for other >>> + purposes. >>> + >>> config CLKSRC_IMX_GPT >>> bool "Clocksource using i.MX GPT" if COMPILE_TEST >>> depends on ARM && CLKDEV_LOOKUP >>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >>> index 00caf37e52f9..d9426f69ec69 100644 >>> --- a/drivers/clocksource/Makefile >>> +++ b/drivers/clocksource/Makefile >>> @@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER) += timer-integrator-ap.o >>> obj-$(CONFIG_CLKSRC_VERSATILE) += versatile.o >>> obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o >>> obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o >>> +obj-$(CONFIG_CLKSRC_IMX_EPIT) += timer-imx-epit.o >>> obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o >>> obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o >>> obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o >>> diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c >>> new file mode 100644 >>> index 000000000000..7f1c8e2e00aa >>> --- /dev/null >>> +++ b/drivers/clocksource/timer-imx-epit.c >>> @@ -0,0 +1,265 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * i.MX EPIT Timer >>> + * >>> + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de> >>> + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com> >>> + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com> >>> + */ >>> + >>> +#include <linux/clk.h> >>> +#include <linux/clockchips.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_irq.h> >>> +#include <linux/sched_clock.h> >>> +#include <linux/slab.h> >>> + >>> +#define EPITCR 0x00 >>> +#define EPITSR 0x04 >>> +#define EPITLR 0x08 >>> +#define EPITCMPR 0x0c >>> +#define EPITCNR 0x10 >>> + >>> +#define EPITCR_EN BIT(0) >>> +#define EPITCR_ENMOD BIT(1) >>> +#define EPITCR_OCIEN BIT(2) >>> +#define EPITCR_RLD BIT(3) >>> +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4) >>> +#define EPITCR_SWR BIT(16) >>> +#define EPITCR_IOVW BIT(17) >>> +#define EPITCR_DBGEN BIT(18) >>> +#define EPITCR_WAITEN BIT(19) >>> +#define EPITCR_RES BIT(20) >>> +#define EPITCR_STOPEN BIT(21) >>> +#define EPITCR_OM_DISCON (0 << 22) >>> +#define EPITCR_OM_TOGGLE (1 << 22) >>> +#define EPITCR_OM_CLEAR (2 << 22) >>> +#define EPITCR_OM_SET (3 << 22) >>> +#define EPITCR_CLKSRC_OFF (0 << 24) >>> +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24) >>> +#define EPITCR_CLKSRC_REF_HIGH (2 << 24) >>> +#define EPITCR_CLKSRC_REF_LOW (3 << 24) >>> + >>> +#define EPITSR_OCIF BIT(0) >>> + >>> +struct epit_timer { >>> + void __iomem *base; >>> + int irq; >>> + struct clk *clk; >>> + struct clock_event_device ced; >>> + struct irqaction act; >>> +}; >>> + >>> +static void __iomem *sched_clock_reg; >>> + >>> +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced) >>> +{ >>> + return container_of(ced, struct epit_timer, ced); >>> +} >>> + >>> +static inline void epit_irq_disable(struct epit_timer *epittm) >>> +{ >>> + u32 val; >>> + >>> + val = readl_relaxed(epittm->base + EPITCR); >>> + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR); >>> +} >>> + >>> +static inline void epit_irq_enable(struct epit_timer *epittm) >>> +{ >>> + u32 val; >>> + >>> + val = readl_relaxed(epittm->base + EPITCR); >>> + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR); >>> +} >>> + >>> +static void epit_irq_acknowledge(struct epit_timer *epittm) >>> +{ >>> + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR); >>> +} >>> + >>> +static u64 notrace epit_read_sched_clock(void) >>> +{ >>> + return ~readl_relaxed(sched_clock_reg); >>> +} >>> + >>> +static int epit_set_next_event(unsigned long cycles, >>> + struct clock_event_device *ced) >>> +{ >>> + struct epit_timer *epittm = to_epit_timer(ced); >>> + unsigned long tcmp; >>> + >>> + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles; >>> + writel_relaxed(tcmp, epittm->base + EPITCMPR); >>> + >>> + return 0; >>> +} >>> + >>> +/* Left event sources disabled, no more interrupts appear */ >>> +static int epit_shutdown(struct clock_event_device *ced) >>> +{ >>> + struct epit_timer *epittm = to_epit_timer(ced); >>> + unsigned long flags; >>> + >>> + /* >>> + * The timer interrupt generation is disabled at least >>> + * for enough time to call epit_set_next_event() >>> + */ >>> + local_irq_save(flags); >> >> This is not necessary. This function is called with interrupt disabled. > > I took this from the i.MX GPT Timer : > https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/timer-imx-gpt.c#L208 > > Do you think i should also fix on the imx gpt timer ? Yes, the pointed code is 10 years old, so if you are willing to do the changes in a separate patchset, that would be nice. >>> + /* Disable interrupt in EPIT module */ >>> + epit_irq_disable(epittm); >>> + >>> + /* Clear pending interrupt */ >>> + epit_irq_acknowledge(epittm); >> >> No irq ack is needed here. Neither disabling the interrupt. > Is it done by the framework ? > > I took also this from the IMX GPT driver If we reach this point of code with a pending timer interrupt, that means it should be processed after the clockevent shutdown path is finished. Otherwise it is like we cancel the timer on the back of the system. >> Why not just stop the counter ? > I will check the datasheet. >>> + local_irq_restore(flags); >>> + >>> + return 0; >>> +} >>> + >>> +static int epit_set_oneshot(struct clock_event_device *ced) >>> +{ >>> + struct epit_timer *epittm = to_epit_timer(ced); >>> + unsigned long flags; >>> + >>> + /* >>> + * The timer interrupt generation is disabled at least >>> + * for enough time to call epit_set_next_event() >>> + */ >>> + local_irq_save(flags); >> >> This is not necessary. This function is called with interrupt disabled. >> >>> + /* Disable interrupt in EPIT module */ >>> + epit_irq_disable(epittm); >>> + >>> + /* Clear pending interrupt, only while switching mode */ >>> + if (!clockevent_state_oneshot(ced)) >>> + epit_irq_acknowledge(epittm); >> >> Why do you need to ack the interrupt here ? >> >>> + /* >>> + * Do not put overhead of interrupt enable/disable into >>> + * epit_set_next_event(), the core has about 4 minutes >>> + * to call epit_set_next_event() or shutdown clock after >>> + * mode switching >>> + */ >>> + epit_irq_enable(epittm); >>> + local_irq_restore(flags); >>> + >>> + return 0; >>> +} >>> + >>> +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id) >>> +{ >>> + struct clock_event_device *ced = dev_id; >>> + struct epit_timer *epittm = to_epit_timer(ced); >>> + >>> + epit_irq_acknowledge(epittm); >>> + >>> + ced->event_handler(ced); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static int __init epit_clocksource_init(struct epit_timer *epittm) >>> +{ >>> + unsigned int c = clk_get_rate(epittm->clk); >>> + >>> + sched_clock_reg = epittm->base + EPITCNR; >>> + sched_clock_register(epit_read_sched_clock, 32, c); >>> + >>> + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32, >>> + clocksource_mmio_readl_down); >>> +} >>> + >>> +static int __init epit_clockevent_init(struct epit_timer *epittm) >>> +{ >>> + struct clock_event_device *ced = &epittm->ced; >>> + struct irqaction *act = &epittm->act; >>> + >>> + ced->name = "epit"; >>> + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ; >>> + ced->set_state_shutdown = epit_shutdown; >>> + ced->tick_resume = epit_shutdown; >>> + ced->set_state_oneshot = epit_set_oneshot; >>> + ced->set_next_event = epit_set_next_event; >>> + ced->rating = 200; >>> + ced->cpumask = cpumask_of(0); >>> + ced->irq = epittm->irq; >>> + clockevents_config_and_register(ced, clk_get_rate(epittm->clk), >>> + 0xff, 0xfffffffe); >>> + >>> + act->name = "i.MX EPIT Timer Tick", >>> + act->flags = IRQF_TIMER | IRQF_IRQPOLL; >>> + act->handler = epit_timer_interrupt; >>> + act->dev_id = ced; >>> + >>> + /* Make irqs happen */ >>> + return setup_irq(epittm->irq, act); >>> +} >>> + >>> +static int __init epit_timer_init(struct device_node *np) >>> +{ >>> + struct epit_timer *epittm; >>> + int ret; >>> + >>> + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); >>> + if (!epittm) >>> + return -ENOMEM; >>> + >>> + epittm->base = of_iomap(np, 0); >>> + if (!epittm->base) { >>> + ret = -ENXIO; >>> + goto out_kfree; >>> + } >>> + >>> + epittm->irq = irq_of_parse_and_map(np, 0); >>> + if (!epittm->irq) { >>> + ret = -EINVAL; >>> + goto out_iounmap; >>> + } >>> + >>> + /* Get EPIT clock */ >>> + epittm->clk = of_clk_get(np, 0); >>> + if (IS_ERR(epittm->clk)) { >>> + pr_err("i.MX EPIT: unable to get clk\n"); >>> + ret = PTR_ERR(epittm->clk); >>> + goto out_iounmap; >>> + } >>> + >>> + ret = clk_prepare_enable(epittm->clk); >>> + if (ret) { >>> + pr_err("i.MX EPIT: unable to prepare+enable clk\n"); >>> + goto out_iounmap; >>> + } >> >> Please replace all the above with the timer-of API as: > Ok I will, > > Thanks > Clement > >> >> https://patchwork.kernel.org/patch/10510443/ >> >> >>> + /* Initialise to a known state (all timers off, and timing reset) */ >>> + writel_relaxed(0x0, epittm->base + EPITCR); >>> + writel_relaxed(0xffffffff, epittm->base + EPITLR); >>> + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN, >>> + epittm->base + EPITCR); >>> + >>> + ret = epit_clocksource_init(epittm); >>> + if (ret) { >>> + pr_err("i.MX EPIT: failed to init clocksource\n"); >>> + goto out_clk_disable; >>> + } >>> + >>> + ret = epit_clockevent_init(epittm); >>> + if (ret) { >>> + pr_err("i.MX EPIT: failed to init clockevent\n"); >>> + goto out_clk_disable; >>> + } >>> + >>> + return 0; >>> + >>> +out_clk_disable: >>> + clk_disable_unprepare(epittm->clk); >>> +out_iounmap: >>> + iounmap(epittm->base); >>> +out_kfree: >>> + kfree(epittm); >>> + >>> + return ret; >>> +} >>> +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_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 >>
Hi Daniel, On Mon, 5 Nov 2018 at 12:48, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > > Hi Clément, > > On 04/11/2018 17:07, Clément Péron wrote: > > Hi Daniel, > > > > Thanks for the review and sorry for the delay. > > no problem. > > > On Tue, 10 Jul 2018 at 18:20, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > >> > >> On 11/06/2018 17:50, Clément Péron wrote: > >>> From: Colin Didier <colin.didier@devialet.com> > >>> > >>> Add driver for NXP's EPIT timer used in i.MX SoC. > >> > >> Give a description on how works this timer. > > Ok, > > > >> > >> > >>> Signed-off-by: Colin Didier <colin.didier@devialet.com> > >>> Signed-off-by: Clément Peron <clement.peron@devialet.com> > >>> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com> > >>> Tested-by: Vladimir Zapolskiy <vz@mleia.com> > >>> --- > >>> drivers/clocksource/Kconfig | 11 ++ > >>> drivers/clocksource/Makefile | 1 + > >>> drivers/clocksource/timer-imx-epit.c | 265 +++++++++++++++++++++++++++ > >>> 3 files changed, 277 insertions(+) > >>> create mode 100644 drivers/clocksource/timer-imx-epit.c > >>> > >>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > >>> index 8e8a09755d10..790478afd02c 100644 > >>> --- a/drivers/clocksource/Kconfig > >>> +++ b/drivers/clocksource/Kconfig > >>> @@ -576,6 +576,17 @@ config H8300_TPU > >>> This enables the clocksource for the H8300 platform with the > >>> H8S2678 cpu. > >>> > >>> +config CLKSRC_IMX_EPIT > >>> + bool "Clocksource using i.MX EPIT" > >>> + depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST) > >>> + select CLKSRC_MMIO > >>> + help > >>> + This enables EPIT support available on some i.MX platforms. > >>> + Normally you don't have a reason to do so as the EPIT has > >>> + the same features and uses the same clocks as the GPT. > >>> + Anyway, on some systems the GPT may be in use for other > >>> + purposes. > >>> + > >>> config CLKSRC_IMX_GPT > >>> bool "Clocksource using i.MX GPT" if COMPILE_TEST > >>> depends on ARM && CLKDEV_LOOKUP > >>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > >>> index 00caf37e52f9..d9426f69ec69 100644 > >>> --- a/drivers/clocksource/Makefile > >>> +++ b/drivers/clocksource/Makefile > >>> @@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER) += timer-integrator-ap.o > >>> obj-$(CONFIG_CLKSRC_VERSATILE) += versatile.o > >>> obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o > >>> obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o > >>> +obj-$(CONFIG_CLKSRC_IMX_EPIT) += timer-imx-epit.o > >>> obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o > >>> obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o > >>> obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o > >>> diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c > >>> new file mode 100644 > >>> index 000000000000..7f1c8e2e00aa > >>> --- /dev/null > >>> +++ b/drivers/clocksource/timer-imx-epit.c > >>> @@ -0,0 +1,265 @@ > >>> +// SPDX-License-Identifier: GPL-2.0 > >>> +/* > >>> + * i.MX EPIT Timer > >>> + * > >>> + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de> > >>> + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com> > >>> + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com> > >>> + */ > >>> + > >>> +#include <linux/clk.h> > >>> +#include <linux/clockchips.h> > >>> +#include <linux/interrupt.h> > >>> +#include <linux/of_address.h> > >>> +#include <linux/of_irq.h> > >>> +#include <linux/sched_clock.h> > >>> +#include <linux/slab.h> > >>> + > >>> +#define EPITCR 0x00 > >>> +#define EPITSR 0x04 > >>> +#define EPITLR 0x08 > >>> +#define EPITCMPR 0x0c > >>> +#define EPITCNR 0x10 > >>> + > >>> +#define EPITCR_EN BIT(0) > >>> +#define EPITCR_ENMOD BIT(1) > >>> +#define EPITCR_OCIEN BIT(2) > >>> +#define EPITCR_RLD BIT(3) > >>> +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4) > >>> +#define EPITCR_SWR BIT(16) > >>> +#define EPITCR_IOVW BIT(17) > >>> +#define EPITCR_DBGEN BIT(18) > >>> +#define EPITCR_WAITEN BIT(19) > >>> +#define EPITCR_RES BIT(20) > >>> +#define EPITCR_STOPEN BIT(21) > >>> +#define EPITCR_OM_DISCON (0 << 22) > >>> +#define EPITCR_OM_TOGGLE (1 << 22) > >>> +#define EPITCR_OM_CLEAR (2 << 22) > >>> +#define EPITCR_OM_SET (3 << 22) > >>> +#define EPITCR_CLKSRC_OFF (0 << 24) > >>> +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24) > >>> +#define EPITCR_CLKSRC_REF_HIGH (2 << 24) > >>> +#define EPITCR_CLKSRC_REF_LOW (3 << 24) > >>> + > >>> +#define EPITSR_OCIF BIT(0) > >>> + > >>> +struct epit_timer { > >>> + void __iomem *base; > >>> + int irq; > >>> + struct clk *clk; > >>> + struct clock_event_device ced; > >>> + struct irqaction act; > >>> +}; > >>> + > >>> +static void __iomem *sched_clock_reg; > >>> + > >>> +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced) > >>> +{ > >>> + return container_of(ced, struct epit_timer, ced); > >>> +} > >>> + > >>> +static inline void epit_irq_disable(struct epit_timer *epittm) > >>> +{ > >>> + u32 val; > >>> + > >>> + val = readl_relaxed(epittm->base + EPITCR); > >>> + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR); > >>> +} > >>> + > >>> +static inline void epit_irq_enable(struct epit_timer *epittm) > >>> +{ > >>> + u32 val; > >>> + > >>> + val = readl_relaxed(epittm->base + EPITCR); > >>> + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR); > >>> +} > >>> + > >>> +static void epit_irq_acknowledge(struct epit_timer *epittm) > >>> +{ > >>> + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR); > >>> +} > >>> + > >>> +static u64 notrace epit_read_sched_clock(void) > >>> +{ > >>> + return ~readl_relaxed(sched_clock_reg); > >>> +} > >>> + > >>> +static int epit_set_next_event(unsigned long cycles, > >>> + struct clock_event_device *ced) > >>> +{ > >>> + struct epit_timer *epittm = to_epit_timer(ced); > >>> + unsigned long tcmp; > >>> + > >>> + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles; > >>> + writel_relaxed(tcmp, epittm->base + EPITCMPR); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +/* Left event sources disabled, no more interrupts appear */ > >>> +static int epit_shutdown(struct clock_event_device *ced) > >>> +{ > >>> + struct epit_timer *epittm = to_epit_timer(ced); > >>> + unsigned long flags; > >>> + > >>> + /* > >>> + * The timer interrupt generation is disabled at least > >>> + * for enough time to call epit_set_next_event() > >>> + */ > >>> + local_irq_save(flags); > >> > >> This is not necessary. This function is called with interrupt disabled. > > > > I took this from the i.MX GPT Timer : > > https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/timer-imx-gpt.c#L208 > > > > Do you think i should also fix on the imx gpt timer ? > > Yes, the pointed code is 10 years old, so if you are willing to do the > changes in a separate patchset, that would be nice. > > >>> + /* Disable interrupt in EPIT module */ > >>> + epit_irq_disable(epittm); > >>> + > >>> + /* Clear pending interrupt */ > >>> + epit_irq_acknowledge(epittm); > >> > >> No irq ack is needed here. Neither disabling the interrupt. > > Is it done by the framework ? > > > > I took also this from the IMX GPT driver > > If we reach this point of code with a pending timer interrupt, that > means it should be processed after the clockevent shutdown path is > finished. Otherwise it is like we cancel the timer on the back of the > system. Thanks for pointing out that. This is actually the first timer driver that I touch. I found all this a bit crappy and I have lazily copy/paste from imx-gpt driver. for me it should more propre to have something like the "timer-npcm7xx.c" driver. I'm looking to understand a bit more the framework before cleaning this. Could you point me any documentation about the framework, Specially what should set_state_**, and tick_resume should do. I'm septic about this "ced->tick_resume = epit_shutdown;" Also do you have some test that i could run to confirm the driver works fine ? Thanks, Clement > > >> Why not just stop the counter ? > > I will check the datasheet. > >>> + local_irq_restore(flags); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int epit_set_oneshot(struct clock_event_device *ced) > >>> +{ > >>> + struct epit_timer *epittm = to_epit_timer(ced); > >>> + unsigned long flags; > >>> + > >>> + /* > >>> + * The timer interrupt generation is disabled at least > >>> + * for enough time to call epit_set_next_event() > >>> + */ > >>> + local_irq_save(flags); > >> > >> This is not necessary. This function is called with interrupt disabled. > >> > >>> + /* Disable interrupt in EPIT module */ > >>> + epit_irq_disable(epittm); > >>> + > >>> + /* Clear pending interrupt, only while switching mode */ > >>> + if (!clockevent_state_oneshot(ced)) > >>> + epit_irq_acknowledge(epittm); > >> > >> Why do you need to ack the interrupt here ? > >> > >>> + /* > >>> + * Do not put overhead of interrupt enable/disable into > >>> + * epit_set_next_event(), the core has about 4 minutes > >>> + * to call epit_set_next_event() or shutdown clock after > >>> + * mode switching > >>> + */ > >>> + epit_irq_enable(epittm); > >>> + local_irq_restore(flags); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id) > >>> +{ > >>> + struct clock_event_device *ced = dev_id; > >>> + struct epit_timer *epittm = to_epit_timer(ced); > >>> + > >>> + epit_irq_acknowledge(epittm); > >>> + > >>> + ced->event_handler(ced); > >>> + > >>> + return IRQ_HANDLED; > >>> +} > >>> + > >>> +static int __init epit_clocksource_init(struct epit_timer *epittm) > >>> +{ > >>> + unsigned int c = clk_get_rate(epittm->clk); > >>> + > >>> + sched_clock_reg = epittm->base + EPITCNR; > >>> + sched_clock_register(epit_read_sched_clock, 32, c); > >>> + > >>> + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32, > >>> + clocksource_mmio_readl_down); > >>> +} > >>> + > >>> +static int __init epit_clockevent_init(struct epit_timer *epittm) > >>> +{ > >>> + struct clock_event_device *ced = &epittm->ced; > >>> + struct irqaction *act = &epittm->act; > >>> + > >>> + ced->name = "epit"; > >>> + ced->features = CLOCK_set_state_shutdownEVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ; > >>> + ced->set_state_shutdown = epit_shutdown; > >>> + ced->tick_resume = epit_shutdown; > >>> + ced->set_state_oneshot = epit_set_oneshot; > >>> + ced->set_next_event = epit_set_next_event; > >>> + ced->rating = 200; > >>> + ced->cpumask = cpumask_of(0); > >>> + ced->irq = epittm->irq; > >>> + clockevents_config_and_register(ced, clk_get_rate(epittm->clk), > >>> + 0xff, 0xfffffffe); > >>> + > >>> + act->name = "i.MX EPIT Timer Tick", > >>> + act->flags = IRQF_TIMER | IRQF_IRQPOLL; > >>> + act->handler = epit_timer_interrupt; > >>> + act->dev_id = ced; > >>> + > >>> + /* Make irqs happen */ > >>> + return setup_irq(epittm->irq, act); > >>> +} > >>> + > >>> +static int __init epit_timer_init(struct device_node *np) > >>> +{ > >>> + struct epit_timer *epittm; > >>> + int ret; > >>> + > >>> + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); > >>> + if (!epittm) > >>> + return -ENOMEM; > >>> + > >>> + epittm->base = of_iomap(np, 0); > >>> + if (!epittm->base) { > >>> + ret = -ENXIO; > >>> + goto out_kfree; > >>> + } > >>> + > >>> + epittm->irq = irq_of_parse_and_map(np, 0); > >>> + if (!epittm->irq) { > >>> + ret = -EINVAL; > >>> + goto out_iounmap; > >>> + } > >>> + > >>> + /* Get EPIT clock */ > >>> + epittm->clk = of_clk_get(np, 0); > >>> + if (IS_ERR(epittm->clk)) { > >>> + pr_err("i.MX EPIT: unable to get clk\n"); > >>> + ret = PTR_ERR(epittm->clk); > >>> + goto out_iounmap; > >>> + } > >>> + > >>> + ret = clk_prepare_enable(epittm->clk); > >>> + if (ret) { > >>> + pr_err("i.MX EPIT: unable to prepare+enable clk\n"); > >>> + goto out_iounmap; > >>> + } > >> > >> Please replace all the above with the timer-of API as: > > Ok I will, > > > > Thanks > > Clement > > > >> > >> https://patchwork.kernel.org/patch/10510443/ > >> > >> > >>> + /* Initialise to a known state (all timers off, and timing reset) */ > >>> + writel_relaxed(0x0, epittm->base + EPITCR); > >>> + writel_relaxed(0xffffffff, epittm->base + EPITLR); > >>> + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN, > >>> + epittm->base + EPITCR); > >>> + > >>> + ret = epit_clocksource_init(epittm); > >>> + if (ret) { > >>> + pr_err("i.MX EPIT: failed to init clocksource\n"); > >>> + goto out_clk_disable; > >>> + } > >>> + > >>> + ret = epit_clockevent_init(epittm); > >>> + if (ret) { > >>> + pr_err("i.MX EPIT: failed to init clockevent\n"); > >>> + goto out_clk_disable; > >>> + } > >>> + > >>> + return 0; > >>> + > >>> +out_clk_disable: > >>> + clk_disable_unprepare(epittm->clk); > >>> +out_iounmap: > >>> + iounmap(epittm->base); > >>> +out_kfree: > >>> + kfree(epittm); > >>> + > >>> + return ret; > >>> +} > >>> +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_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 > >> > > > -- > <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 >
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 8e8a09755d10..790478afd02c 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -576,6 +576,17 @@ config H8300_TPU This enables the clocksource for the H8300 platform with the H8S2678 cpu. +config CLKSRC_IMX_EPIT + bool "Clocksource using i.MX EPIT" + depends on CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST) + select CLKSRC_MMIO + help + This enables EPIT support available on some i.MX platforms. + Normally you don't have a reason to do so as the EPIT has + the same features and uses the same clocks as the GPT. + Anyway, on some systems the GPT may be in use for other + purposes. + config CLKSRC_IMX_GPT bool "Clocksource using i.MX GPT" if COMPILE_TEST depends on ARM && CLKDEV_LOOKUP diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 00caf37e52f9..d9426f69ec69 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -69,6 +69,7 @@ obj-$(CONFIG_INTEGRATOR_AP_TIMER) += timer-integrator-ap.o obj-$(CONFIG_CLKSRC_VERSATILE) += versatile.o obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o +obj-$(CONFIG_CLKSRC_IMX_EPIT) += timer-imx-epit.o obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o obj-$(CONFIG_CLKSRC_IMX_TPM) += timer-imx-tpm.o obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o diff --git a/drivers/clocksource/timer-imx-epit.c b/drivers/clocksource/timer-imx-epit.c new file mode 100644 index 000000000000..7f1c8e2e00aa --- /dev/null +++ b/drivers/clocksource/timer-imx-epit.c @@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * i.MX EPIT Timer + * + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de> + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com> + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com> + */ + +#include <linux/clk.h> +#include <linux/clockchips.h> +#include <linux/interrupt.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/sched_clock.h> +#include <linux/slab.h> + +#define EPITCR 0x00 +#define EPITSR 0x04 +#define EPITLR 0x08 +#define EPITCMPR 0x0c +#define EPITCNR 0x10 + +#define EPITCR_EN BIT(0) +#define EPITCR_ENMOD BIT(1) +#define EPITCR_OCIEN BIT(2) +#define EPITCR_RLD BIT(3) +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4) +#define EPITCR_SWR BIT(16) +#define EPITCR_IOVW BIT(17) +#define EPITCR_DBGEN BIT(18) +#define EPITCR_WAITEN BIT(19) +#define EPITCR_RES BIT(20) +#define EPITCR_STOPEN BIT(21) +#define EPITCR_OM_DISCON (0 << 22) +#define EPITCR_OM_TOGGLE (1 << 22) +#define EPITCR_OM_CLEAR (2 << 22) +#define EPITCR_OM_SET (3 << 22) +#define EPITCR_CLKSRC_OFF (0 << 24) +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24) +#define EPITCR_CLKSRC_REF_HIGH (2 << 24) +#define EPITCR_CLKSRC_REF_LOW (3 << 24) + +#define EPITSR_OCIF BIT(0) + +struct epit_timer { + void __iomem *base; + int irq; + struct clk *clk; + struct clock_event_device ced; + struct irqaction act; +}; + +static void __iomem *sched_clock_reg; + +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced) +{ + return container_of(ced, struct epit_timer, ced); +} + +static inline void epit_irq_disable(struct epit_timer *epittm) +{ + u32 val; + + val = readl_relaxed(epittm->base + EPITCR); + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR); +} + +static inline void epit_irq_enable(struct epit_timer *epittm) +{ + u32 val; + + val = readl_relaxed(epittm->base + EPITCR); + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR); +} + +static void epit_irq_acknowledge(struct epit_timer *epittm) +{ + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR); +} + +static u64 notrace epit_read_sched_clock(void) +{ + return ~readl_relaxed(sched_clock_reg); +} + +static int epit_set_next_event(unsigned long cycles, + struct clock_event_device *ced) +{ + struct epit_timer *epittm = to_epit_timer(ced); + unsigned long tcmp; + + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles; + writel_relaxed(tcmp, epittm->base + EPITCMPR); + + return 0; +} + +/* Left event sources disabled, no more interrupts appear */ +static int epit_shutdown(struct clock_event_device *ced) +{ + struct epit_timer *epittm = to_epit_timer(ced); + unsigned long flags; + + /* + * The timer interrupt generation is disabled at least + * for enough time to call epit_set_next_event() + */ + local_irq_save(flags); + + /* Disable interrupt in EPIT module */ + epit_irq_disable(epittm); + + /* Clear pending interrupt */ + epit_irq_acknowledge(epittm); + + local_irq_restore(flags); + + return 0; +} + +static int epit_set_oneshot(struct clock_event_device *ced) +{ + struct epit_timer *epittm = to_epit_timer(ced); + unsigned long flags; + + /* + * The timer interrupt generation is disabled at least + * for enough time to call epit_set_next_event() + */ + local_irq_save(flags); + + /* Disable interrupt in EPIT module */ + epit_irq_disable(epittm); + + /* Clear pending interrupt, only while switching mode */ + if (!clockevent_state_oneshot(ced)) + epit_irq_acknowledge(epittm); + + /* + * Do not put overhead of interrupt enable/disable into + * epit_set_next_event(), the core has about 4 minutes + * to call epit_set_next_event() or shutdown clock after + * mode switching + */ + epit_irq_enable(epittm); + local_irq_restore(flags); + + return 0; +} + +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id) +{ + struct clock_event_device *ced = dev_id; + struct epit_timer *epittm = to_epit_timer(ced); + + epit_irq_acknowledge(epittm); + + ced->event_handler(ced); + + return IRQ_HANDLED; +} + +static int __init epit_clocksource_init(struct epit_timer *epittm) +{ + unsigned int c = clk_get_rate(epittm->clk); + + sched_clock_reg = epittm->base + EPITCNR; + sched_clock_register(epit_read_sched_clock, 32, c); + + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32, + clocksource_mmio_readl_down); +} + +static int __init epit_clockevent_init(struct epit_timer *epittm) +{ + struct clock_event_device *ced = &epittm->ced; + struct irqaction *act = &epittm->act; + + ced->name = "epit"; + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ; + ced->set_state_shutdown = epit_shutdown; + ced->tick_resume = epit_shutdown; + ced->set_state_oneshot = epit_set_oneshot; + ced->set_next_event = epit_set_next_event; + ced->rating = 200; + ced->cpumask = cpumask_of(0); + ced->irq = epittm->irq; + clockevents_config_and_register(ced, clk_get_rate(epittm->clk), + 0xff, 0xfffffffe); + + act->name = "i.MX EPIT Timer Tick", + act->flags = IRQF_TIMER | IRQF_IRQPOLL; + act->handler = epit_timer_interrupt; + act->dev_id = ced; + + /* Make irqs happen */ + return setup_irq(epittm->irq, act); +} + +static int __init epit_timer_init(struct device_node *np) +{ + struct epit_timer *epittm; + int ret; + + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); + if (!epittm) + return -ENOMEM; + + epittm->base = of_iomap(np, 0); + if (!epittm->base) { + ret = -ENXIO; + goto out_kfree; + } + + epittm->irq = irq_of_parse_and_map(np, 0); + if (!epittm->irq) { + ret = -EINVAL; + goto out_iounmap; + } + + /* Get EPIT clock */ + epittm->clk = of_clk_get(np, 0); + if (IS_ERR(epittm->clk)) { + pr_err("i.MX EPIT: unable to get clk\n"); + ret = PTR_ERR(epittm->clk); + goto out_iounmap; + } + + ret = clk_prepare_enable(epittm->clk); + if (ret) { + pr_err("i.MX EPIT: unable to prepare+enable clk\n"); + goto out_iounmap; + } + + /* Initialise to a known state (all timers off, and timing reset) */ + writel_relaxed(0x0, epittm->base + EPITCR); + writel_relaxed(0xffffffff, epittm->base + EPITLR); + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN, + epittm->base + EPITCR); + + ret = epit_clocksource_init(epittm); + if (ret) { + pr_err("i.MX EPIT: failed to init clocksource\n"); + goto out_clk_disable; + } + + ret = epit_clockevent_init(epittm); + if (ret) { + pr_err("i.MX EPIT: failed to init clockevent\n"); + goto out_clk_disable; + } + + return 0; + +out_clk_disable: + clk_disable_unprepare(epittm->clk); +out_iounmap: + iounmap(epittm->base); +out_kfree: + kfree(epittm); + + return ret; +} +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);