From: Clément Peron <[email protected]>
As suggested in the commit message we have added the device tree support,
proper bindings and we moved the driver into the correct folder.
Moreover we made some changes like use of relaxed IO accesor,
implement sched_clock, delay_timer and reduce the clockevents min_delta.
Signed-off-by: Clément Peron <[email protected]>
---
Clément Peron (2):
ARM: imx: remove inexistant EPIT timer init
Documentation: DT: add i.MX EPIT timer binding
Colin Didier (3):
ARM: clk-imx6q: add EPIT clock support
clocksource: add driver for i.MX EPIT timer
ARM: dts: imx6qdl: add missing compatible and clock properties for
EPIT
.../devicetree/bindings/clock/imx6q,epit.txt | 25 ++
arch/arm/boot/dts/imx6qdl.dtsi | 12 +
arch/arm/mach-imx/common.h | 1 -
drivers/clk/imx/clk-imx6q.c | 2 +
drivers/clocksource/Kconfig | 12 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-imx-epit.c | 254 ++++++++++++++++++
include/dt-bindings/clock/imx6qdl-clock.h | 4 +-
8 files changed, 309 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx6q,epit.txt
create mode 100644 drivers/clocksource/timer-imx-epit.c
--
2.17.0
From: Clément Peron <[email protected]>
Add devicetree binding document for NXP's i.MX SoC specific
EPIT timer driver.
Signed-off-by: Clément Peron <[email protected]>
---
.../devicetree/bindings/clock/imx6q,epit.txt | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/imx6q,epit.txt
diff --git a/Documentation/devicetree/bindings/clock/imx6q,epit.txt b/Documentation/devicetree/bindings/clock/imx6q,epit.txt
new file mode 100644
index 000000000000..d54d455cebdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx6q,epit.txt
@@ -0,0 +1,25 @@
+Binding for the i.MX6 EPIT timer
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible: should be "fsl,imx6q-epit"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- interrupts: Should contain EPIT controller interrupt
+- clocks: list of clock specifiers, must contain an entry for each required
+ entry in clock-names
+- clock-names : as described in the clock bindings
+
+Example:
+ epit1: epit@20d0000 {
+ compatible = "fsl,imx6q-epit";
+ reg = <0x020d0000 0x4000>;
+ interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_EPIT1>,
+ <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_CKIL>;
+ clock-names = "ipg", "per", "ckil";
+ };
--
2.17.0
From: Colin Didier <[email protected]>
Add driver for NXP's EPIT timer used in i.MX 6 family of SoC.
Signed-off-by: Clément Peron <[email protected]>
---
drivers/clocksource/Kconfig | 12 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-imx-epit.c | 254 +++++++++++++++++++++++++++
3 files changed, 267 insertions(+)
create mode 100644 drivers/clocksource/timer-imx-epit.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 8e8a09755d10..cc1ed592fa6f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -576,6 +576,18 @@ 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 ARM && CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST)
+ select CLKSRC_OF if OF
+ 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..96eb6435a9c3
--- /dev/null
+++ b/drivers/clocksource/timer-imx-epit.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX EPIT Timer
+ *
+ * Copyright (C) 2010 Sascha Hauer <[email protected]>
+ * Copyright (C) 2018 Colin Didier <[email protected]>
+ */
+
+#define EPITCR 0x00
+#define EPITSR 0x04
+#define EPITLR 0x08
+#define EPITCMPR 0x0c
+#define EPITCNR 0x10
+
+#define EPITCR_EN (1 << 0)
+#define EPITCR_ENMOD (1 << 1)
+#define EPITCR_OCIEN (1 << 2)
+#define EPITCR_RLD (1 << 3)
+#define EPITCR_PRESC(x) (((x) & 0xfff) << 4)
+#define EPITCR_SWR (1 << 16)
+#define EPITCR_IOVW (1 << 17)
+#define EPITCR_DBGEN (1 << 18)
+#define EPITCR_WAITEN (1 << 19)
+#define EPITCR_RES (1 << 20)
+#define EPITCR_STOPEN (1 << 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 (1 << 0)
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/clockchips.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+
+struct epit_timer {
+ void __iomem *base;
+ int irq;
+ struct clk *clk_per;
+ struct clock_event_device ced;
+ struct irqaction act;
+};
+
+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 void __iomem *sched_clock_reg;
+
+static u64 notrace epit_read_sched_clock(void)
+{
+ return ~readl_relaxed(sched_clock_reg);
+}
+
+static int __init epit_clocksource_init(struct epit_timer *epittm)
+{
+ unsigned int c = clk_get_rate(epittm->clk_per);
+
+ 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);
+}
+
+/* clock event */
+
+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;
+}
+
+/*
+ * IRQ handler for the timer
+ */
+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_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_per),
+ 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;
+ struct clk *clk_ipg;
+
+ epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
+ if (!epittm)
+ return -ENOMEM;
+
+ epittm->base = of_iomap(np, 0);
+ if (!epittm->base)
+ return -ENXIO;
+
+ epittm->irq = irq_of_parse_and_map(np, 0);
+ if (epittm->irq <= 0)
+ return -EINVAL;
+
+ epittm->clk_per = of_clk_get_by_name(np, "per");
+
+ clk_ipg = of_clk_get_by_name(np, "ipg");
+ if (!IS_ERR(clk_ipg))
+ clk_prepare_enable(clk_ipg);
+
+ if (IS_ERR(epittm->clk_per)) {
+ pr_err("i.MX EPIT: unable to get clk\n");
+ return PTR_ERR(epittm->clk_per);
+ }
+ clk_prepare_enable(epittm->clk_per);
+
+ /*
+ * 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);
+
+ /* init and register the timer to the framework */
+ epit_clocksource_init(epittm);
+ epit_clockevent_init(epittm);
+
+ return 0;
+}
+CLOCKSOURCE_OF_DECLARE(mx6q_timer, "fsl,imx6q-epit", epit_timer_init);
--
2.17.0
From: Colin Didier <[email protected]>
Add EPIT clock support to the i.MX6Q clocking infrastructure.
Signed-off-by: Colin Didier <[email protected]>
Signed-off-by: Clément Peron <[email protected]>
---
drivers/clk/imx/clk-imx6q.c | 2 ++
include/dt-bindings/clock/imx6qdl-clock.h | 4 +++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 8d518ad5dc13..b9ea7037e193 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -753,6 +753,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
else
clk[IMX6Q_CLK_ECSPI5] = imx_clk_gate2("ecspi5", "ecspi_root", base + 0x6c, 8);
clk[IMX6QDL_CLK_ENET] = imx_clk_gate2("enet", "ipg", base + 0x6c, 10);
+ clk[IMX6QDL_CLK_EPIT1] = imx_clk_gate2("epit1", "ipg", base + 0x6c, 12);
+ clk[IMX6QDL_CLK_EPIT2] = imx_clk_gate2("epit2", "ipg", base + 0x6c, 14);
clk[IMX6QDL_CLK_ESAI_EXTAL] = imx_clk_gate2_shared("esai_extal", "esai_podf", base + 0x6c, 16, &share_count_esai);
clk[IMX6QDL_CLK_ESAI_IPG] = imx_clk_gate2_shared("esai_ipg", "ahb", base + 0x6c, 16, &share_count_esai);
clk[IMX6QDL_CLK_ESAI_MEM] = imx_clk_gate2_shared("esai_mem", "ahb", base + 0x6c, 16, &share_count_esai);
diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
index da59fd9cdb5e..7ad171b8f3bf 100644
--- a/include/dt-bindings/clock/imx6qdl-clock.h
+++ b/include/dt-bindings/clock/imx6qdl-clock.h
@@ -271,6 +271,8 @@
#define IMX6QDL_CLK_PRE_AXI 258
#define IMX6QDL_CLK_MLB_SEL 259
#define IMX6QDL_CLK_MLB_PODF 260
-#define IMX6QDL_CLK_END 261
+#define IMX6QDL_CLK_EPIT1 261
+#define IMX6QDL_CLK_EPIT2 262
+#define IMX6QDL_CLK_END 263
#endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
--
2.17.0
From: Colin Didier <[email protected]>
Add missing compatible and clock properties for EPIT node.
Signed-off-by: Colin Didier <[email protected]>
Signed-off-by: Clément Peron <[email protected]>
---
arch/arm/boot/dts/imx6qdl.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index c003e62bf290..16fec147dcb8 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -844,13 +844,25 @@
};
epit1: epit@20d0000 { /* EPIT1 */
+ compatible = "fsl,imx6q-epit";
reg = <0x020d0000 0x4000>;
interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_EPIT1>,
+ <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_CKIL>;
+ clock-names = "ipg", "per", "ckil";
+ status = "disabled";
};
epit2: epit@20d4000 { /* EPIT2 */
+ compatible = "fsl,imx6q-epit";
reg = <0x020d4000 0x4000>;
interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_EPIT2>,
+ <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_CKIL>;
+ clock-names = "ipg", "per", "ckil";
+ status = "disabled";
};
src: src@20d8000 {
--
2.17.0
From: Clément Peron <[email protected]>
i.MX EPIT timer has been removed but not the init function declaration.
Signed-off-by: Clément Peron <[email protected]>
---
arch/arm/mach-imx/common.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index c8d68e918b2f..18aae76fa2da 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -38,7 +38,6 @@ void imx21_soc_init(void);
void imx27_soc_init(void);
void imx31_soc_init(void);
void imx35_soc_init(void);
-void epit_timer_init(void __iomem *base, int irq);
int mx21_clocks_init(unsigned long lref, unsigned long fref);
int mx27_clocks_init(unsigned long fref);
int mx31_clocks_init(unsigned long fref);
--
2.17.0
Hi Clément,
please find basic review comments below.
On 05/28/2018 08:34 PM, Clément Péron wrote:
> From: Colin Didier <[email protected]>
>
> Add driver for NXP's EPIT timer used in i.MX 6 family of SoC.
>
The first author's signed-off-by tag is missing.
> Signed-off-by: Clément Peron <[email protected]>
> ---
> drivers/clocksource/Kconfig | 12 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-imx-epit.c | 254 +++++++++++++++++++++++++++
> 3 files changed, 267 insertions(+)
> create mode 100644 drivers/clocksource/timer-imx-epit.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 8e8a09755d10..cc1ed592fa6f 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -576,6 +576,18 @@ 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 ARM && CLKDEV_LOOKUP && (ARCH_MXC || COMPILE_TEST)
> + select CLKSRC_OF if OF
The driver strictly depends on OF, and this has to be specified.
> + 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..96eb6435a9c3
> --- /dev/null
> +++ b/drivers/clocksource/timer-imx-epit.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX EPIT Timer
> + *
> + * Copyright (C) 2010 Sascha Hauer <[email protected]>
> + * Copyright (C) 2018 Colin Didier <[email protected]>
> + */
> +
> +#define EPITCR 0x00
> +#define EPITSR 0x04
> +#define EPITLR 0x08
> +#define EPITCMPR 0x0c
> +#define EPITCNR 0x10
> +
> +#define EPITCR_EN (1 << 0)
> +#define EPITCR_ENMOD (1 << 1)
> +#define EPITCR_OCIEN (1 << 2)
> +#define EPITCR_RLD (1 << 3)
> +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4)
> +#define EPITCR_SWR (1 << 16)
> +#define EPITCR_IOVW (1 << 17)
> +#define EPITCR_DBGEN (1 << 18)
> +#define EPITCR_WAITEN (1 << 19)
> +#define EPITCR_RES (1 << 20)
> +#define EPITCR_STOPEN (1 << 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 (1 << 0)
> +
Please place all macro definitions after the list of included header files.
Also for bit field definitions please use BIT() macro, and remove
(0 << x) macro, they are anyway unused in the code.
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/clockchips.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/sched_clock.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
Please sort out the list of headers alphabetically.
> +
> +
Surplus empty line, please remove it.
> +struct epit_timer {
> + void __iomem *base;
> + int irq;
> + struct clk *clk_per;
> + struct clock_event_device ced;
> + struct irqaction act;
> +};
> +
> +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 void __iomem *sched_clock_reg;
Please move this declaration up, place it right after struct epit_timer {}.
> +
> +static u64 notrace epit_read_sched_clock(void)
> +{
> + return ~readl_relaxed(sched_clock_reg);
> +}
> +
> +static int __init epit_clocksource_init(struct epit_timer *epittm)
> +{
> + unsigned int c = clk_get_rate(epittm->clk_per);
> +
> + 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);
> +}
> +
> +/* clock event */
> +
Surplus empty line above, please remove, and I would suggest to remove
the trivial comment as well.
> +static int epit_set_next_event(unsigned long cycles,
> + struct clock_event_device *ced)
Please align the indentation on the line above.
> +{
> + struct epit_timer *epittm = to_epit_timer(ced);
> + unsigned long tcmp;
> +
> + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles;
> +
I suppose you can remove the empty line above.
> + 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;
> +}
> +
> +/*
> + * IRQ handler for the timer
> + */
The comment above is trivial, please remove it.
> +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_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_per),
> + 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;
> + struct clk *clk_ipg;
> +
> + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
> + if (!epittm)
> + return -ENOMEM;
> +
> + epittm->base = of_iomap(np, 0);
> + if (!epittm->base)
> + return -ENXIO;
Dynamically allocated 'epittm' memory leak here and on every
error path below.
> +
> + epittm->irq = irq_of_parse_and_map(np, 0);
> + if (epittm->irq <= 0)
To handle a possible error you should check !epittm->irq
> + return -EINVAL;
iomapped 'epittm->base' is not iounmap()'ed here and on every
error path below.
> +
> + epittm->clk_per = of_clk_get_by_name(np, "per");
Move this line closer to clk_prepare_enable(epittm->clk_per) and
check for errors.
> +
> + clk_ipg = of_clk_get_by_name(np, "ipg");
> + if (!IS_ERR(clk_ipg))
> + clk_prepare_enable(clk_ipg);
Please don't use a check for success status, 'if (IS_ERR(clk_ipg)) ...'
> +
> + if (IS_ERR(epittm->clk_per)) {
> + pr_err("i.MX EPIT: unable to get clk\n");
> + return PTR_ERR(epittm->clk_per);
On error path "ipg" clock is left prepared/enabled.
> + }
> + clk_prepare_enable(epittm->clk_per);
> +
> + /*
> + * 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);
> +
> + /* init and register the timer to the framework */
The comment above is redundant, please remove it.
> + epit_clocksource_init(epittm);
Please check and handle a possible returned error.
> + epit_clockevent_init(epittm);
Please check and handle a possible returned error.
> +
> + return 0;
> +}
> +CLOCKSOURCE_OF_DECLARE(mx6q_timer, "fsl,imx6q-epit", epit_timer_init);
Please use TIMER_OF_DECLARE() macro.
--
With best wishes,
Vladimir
On Mon, May 28, 2018 at 07:34:08PM +0200, Cl?ment P?ron wrote:
> From: Cl?ment Peron <[email protected]>
>
> i.MX EPIT timer has been removed but not the init function declaration.
>
> Signed-off-by: Cl?ment Peron <[email protected]>
Applied, thanks.
On Mon, May 28, 2018 at 07:34:09PM +0200, Cl?ment P?ron wrote:
> From: Colin Didier <[email protected]>
>
> Add EPIT clock support to the i.MX6Q clocking infrastructure.
>
> Signed-off-by: Colin Didier <[email protected]>
> Signed-off-by: Cl?ment Peron <[email protected]>
This is a clock patch and should be prefixed with 'clk: imx6q: ...'
rather than 'ARM: ...' Otherwise,
Acked-by: Shawn Guo <[email protected]>
> ---
> drivers/clk/imx/clk-imx6q.c | 2 ++
> include/dt-bindings/clock/imx6qdl-clock.h | 4 +++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index 8d518ad5dc13..b9ea7037e193 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -753,6 +753,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
> else
> clk[IMX6Q_CLK_ECSPI5] = imx_clk_gate2("ecspi5", "ecspi_root", base + 0x6c, 8);
> clk[IMX6QDL_CLK_ENET] = imx_clk_gate2("enet", "ipg", base + 0x6c, 10);
> + clk[IMX6QDL_CLK_EPIT1] = imx_clk_gate2("epit1", "ipg", base + 0x6c, 12);
> + clk[IMX6QDL_CLK_EPIT2] = imx_clk_gate2("epit2", "ipg", base + 0x6c, 14);
> clk[IMX6QDL_CLK_ESAI_EXTAL] = imx_clk_gate2_shared("esai_extal", "esai_podf", base + 0x6c, 16, &share_count_esai);
> clk[IMX6QDL_CLK_ESAI_IPG] = imx_clk_gate2_shared("esai_ipg", "ahb", base + 0x6c, 16, &share_count_esai);
> clk[IMX6QDL_CLK_ESAI_MEM] = imx_clk_gate2_shared("esai_mem", "ahb", base + 0x6c, 16, &share_count_esai);
> diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
> index da59fd9cdb5e..7ad171b8f3bf 100644
> --- a/include/dt-bindings/clock/imx6qdl-clock.h
> +++ b/include/dt-bindings/clock/imx6qdl-clock.h
> @@ -271,6 +271,8 @@
> #define IMX6QDL_CLK_PRE_AXI 258
> #define IMX6QDL_CLK_MLB_SEL 259
> #define IMX6QDL_CLK_MLB_PODF 260
> -#define IMX6QDL_CLK_END 261
> +#define IMX6QDL_CLK_EPIT1 261
> +#define IMX6QDL_CLK_EPIT2 262
> +#define IMX6QDL_CLK_END 263
>
> #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
> --
> 2.17.0
>