2018-05-30 12:05:26

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 0/5] Reintroduce i.MX EPIT Timer

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.

Changes since v3:
- Clean Kconfig
- Rename imx6q-epit to imx31-epit
- Update doc and bindings
- Indent and fix

Changes since v2 (Thanks Fabio Estevam):
- Removed unused ckil clock
- Add out_iounmap
- Check and handle if clk_prepare_enable failed
- Fix comment typo

Changes since v1 (Thanks Vladimir Zapolskiy):
- Add OF dependency in Kconfig
- Sort header
- Use BIT macro
- Remove useless comments
- Fix incorrect indent
- Fix memory leak
- Add check and handle possible returned error

Clément Peron (2):
ARM: imx: remove inexistant EPIT timer init
Documentation: DT: add i.MX EPIT timer binding

Colin Didier (3):
clk: imx6: 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/timer/fsl,imxepit.txt | 24 ++
arch/arm/boot/dts/imx6qdl.dtsi | 10 +
arch/arm/mach-imx/common.h | 1 -
drivers/clk/imx/clk-imx6q.c | 2 +
drivers/clocksource/Kconfig | 11 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-imx-epit.c | 281 ++++++++++++++++++
include/dt-bindings/clock/imx6qdl-clock.h | 4 +-
8 files changed, 332 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt
create mode 100644 drivers/clocksource/timer-imx-epit.c

--
2.17.0



2018-05-30 12:04:51

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 3/5] Documentation: DT: add i.MX EPIT timer binding

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/timer/fsl,imxepit.txt | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt

diff --git a/Documentation/devicetree/bindings/timer/fsl,imxepit.txt b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
new file mode 100644
index 000000000000..90112d58af10
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
@@ -0,0 +1,24 @@
+Binding for the i.MX EPIT timer
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible: should be "fsl,imx31-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 : should include entries "ipg", "per"
+
+Example for i.MX6QDL:
+ epit1: epit@20d0000 {
+ compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
+ reg = <0x020d0000 0x4000>;
+ interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_EPIT1>;
+ clock-names = "ipg", "per";
+ };
--
2.17.0


2018-05-30 12:05:08

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT

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]>
Reviewed-by: Fabio Estevam <[email protected]>
---
arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index c003e62bf290..0feec516847a 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -844,13 +844,23 @@
};

epit1: epit@20d0000 { /* EPIT1 */
+ compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
reg = <0x020d0000 0x4000>;
interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_EPIT1>;
+ clock-names = "ipg", "per";
+ status = "disabled";
};

epit2: epit@20d4000 { /* EPIT2 */
+ compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
reg = <0x020d4000 0x4000>;
interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6QDL_CLK_IPG_PER>,
+ <&clks IMX6QDL_CLK_EPIT2>;
+ clock-names = "ipg", "per";
+ status = "disabled";
};

src: src@20d8000 {
--
2.17.0


2018-05-30 12:05:35

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 4/5] clocksource: add driver for i.MX EPIT timer

From: Colin Didier <[email protected]>

Add driver for NXP's EPIT timer used in i.MX 6 family of SoC.

Signed-off-by: Colin Didier <[email protected]>
Signed-off-by: Clément Peron <[email protected]>
---
drivers/clocksource/Kconfig | 11 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-imx-epit.c | 281 +++++++++++++++++++++++++++
3 files changed, 293 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..7e92fcab10d3
--- /dev/null
+++ b/drivers/clocksource/timer-imx-epit.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX EPIT Timer
+ *
+ * Copyright (C) 2010 Sascha Hauer <[email protected]>
+ * Copyright (C) 2018 Colin Didier <[email protected]>
+ * Copyright (C) 2018 Clément Péron <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/err.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_per;
+ 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_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);
+}
+
+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;
+ 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;
+ }
+
+ clk_ipg = of_clk_get_by_name(np, "ipg");
+ if (IS_ERR(clk_ipg)) {
+ pr_err("i.MX EPIT: unable to get clk_ipg\n");
+ ret = PTR_ERR(clk_ipg);
+ goto out_iounmap;
+ }
+
+ ret = clk_prepare_enable(clk_ipg);
+ if (ret) {
+ pr_err("i.MX EPIT: unable to prepare+enable clk_ipg\n");
+ goto out_iounmap;
+ }
+
+ epittm->clk_per = of_clk_get_by_name(np, "per");
+ if (IS_ERR(epittm->clk_per)) {
+ pr_err("i.MX EPIT: unable to get clk_per\n");
+ ret = PTR_ERR(epittm->clk_per);
+ goto out_clk_ipg_disable;
+ }
+
+ ret = clk_prepare_enable(epittm->clk_per);
+ if (ret) {
+ pr_err("i.MX EPIT: unable to prepare+enable clk_per\n");
+ goto out_clk_ipg_disable;
+ }
+
+ /* 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_per_disable;
+ }
+
+ ret = epit_clockevent_init(epittm);
+ if (ret) {
+ pr_err("i.MX EPIT: failed to init clockevent\n");
+ goto out_clk_per_disable;
+ }
+
+ return 0;
+
+out_clk_per_disable:
+ clk_disable_unprepare(epittm->clk_per);
+out_clk_ipg_disable:
+ clk_disable_unprepare(clk_ipg);
+out_iounmap:
+ iounmap(epittm->base);
+out_kfree:
+ kfree(epittm);
+
+ return ret;
+}
+TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);
--
2.17.0


2018-05-30 12:07:00

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 2/5] clk: imx6: add EPIT clock support

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]>
Reviewed-by: Fabio Estevam <[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


2018-05-30 12:07:29

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 1/5] ARM: imx: remove inexistant EPIT timer init

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]>
Reviewed-by: Fabio Estevam <[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


2018-05-31 03:08:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] clk: imx6: add EPIT clock support

On Wed, May 30, 2018 at 02:03:24PM +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]>
> Reviewed-by: Fabio Estevam <[email protected]>
> ---
> drivers/clk/imx/clk-imx6q.c | 2 ++
> include/dt-bindings/clock/imx6qdl-clock.h | 4 +++-

Acked-by: Rob Herring <[email protected]>

> 2 files changed, 5 insertions(+), 1 deletion(-)

2018-05-31 03:12:10

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] Documentation: DT: add i.MX EPIT timer binding

On Wed, May 30, 2018 at 02:03:25PM +0200, Cl?ment P?ron wrote:
> From: Cl?ment Peron <[email protected]>

"dt-bindings: timer: ..." is the preferred subject prefix.

>
> Add devicetree binding document for NXP's i.MX SoC specific
> EPIT timer driver.
>
> Signed-off-by: Cl?ment Peron <[email protected]>
> ---
> .../devicetree/bindings/timer/fsl,imxepit.txt | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxepit.txt b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> new file mode 100644
> index 000000000000..90112d58af10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> @@ -0,0 +1,24 @@
> +Binding for the i.MX EPIT timer
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt

No need to state this. Listing "clocks" tells us that.

> +
> +Required properties:
> +- compatible: should be "fsl,imx31-epit"

What about imx6q listed below?

> +- 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 : should include entries "ipg", "per"
> +
> +Example for i.MX6QDL:
> + epit1: epit@20d0000 {

timer@...

> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> + reg = <0x020d0000 0x4000>;
> + interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT1>;
> + clock-names = "ipg", "per";
> + };
> --
> 2.17.0
>

2018-05-31 08:28:57

by Vladimir Zapolskiy

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] Documentation: DT: add i.MX EPIT timer binding

Hi Clément,

On 05/30/2018 03:03 PM, Clément Péron wrote:
> 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/timer/fsl,imxepit.txt | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxepit.txt b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> new file mode 100644
> index 000000000000..90112d58af10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> @@ -0,0 +1,24 @@
> +Binding for the i.MX EPIT timer
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +

no, this leftover reference to clock-bindings.txt is invalid, please remove it.

Instead you may add a simple description of the timer module.

> +Required properties:
> +- compatible: should be "fsl,imx31-epit"

To satisfy compatibles with multiple SoCs, apparently you may follow a model,
which is used with other Freescale controllers, for instance

gpio/fsl-imx-gpio.txt - compatible : Should be "fsl,<soc>-gpio"
mmc/fsl-imx-esdhc.txt - compatible : Should be "fsl,<chip>-esdhc"
serial/fsl-imx-uart.txt - compatible : Should be "fsl,<soc>-uart"
timer/fsl,imxgpt.txt - compatible : should be "fsl,<soc>-gpt"

and so on, I hope it would cover Rob's ask.

> +- 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 : should include entries "ipg", "per"
> +
> +Example for i.MX6QDL:
> + epit1: epit@20d0000 {
> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> + reg = <0x020d0000 0x4000>;
> + interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT1>;
> + clock-names = "ipg", "per";
> + };
>

--
With best wishes,
Vladimir

2018-05-31 08:34:37

by Vladimir Zapolskiy

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT

On 05/30/2018 03:03 PM, Clément Péron wrote:
> 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]>
> Reviewed-by: Fabio Estevam <[email protected]>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index c003e62bf290..0feec516847a 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -844,13 +844,23 @@
> };
>
> epit1: epit@20d0000 { /* EPIT1 */

epit1: timer@20d0000 { ...

And /* EPIT1 */ comment can be removed, it is quite clear from the same
line context.

Formally it is a subject to another patch, but I think this can be
accepted as a part of this one.

> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> reg = <0x020d0000 0x4000>;
> interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT1>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> };
>
> epit2: epit@20d4000 { /* EPIT2 */

Same as above.

> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> reg = <0x020d4000 0x4000>;
> interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT2>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> };
>
> src: src@20d8000 {
>

--
With best wishes,
Vladimir

2018-05-31 08:38:38

by Vladimir Zapolskiy

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] clocksource: add driver for i.MX EPIT timer

Hi Clément,

On 05/30/2018 03:03 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.
>
> Signed-off-by: Colin Didier <[email protected]>
> Signed-off-by: Clément Peron <[email protected]>
> ---

[snip]

> +++ b/drivers/clocksource/timer-imx-epit.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX EPIT Timer
> + *
> + * Copyright (C) 2010 Sascha Hauer <[email protected]>
> + * Copyright (C) 2018 Colin Didier <[email protected]>
> + * Copyright (C) 2018 Clément Péron <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/err.h>

The included header above still can be removed.

I have no more comments about the code, I will try to find time to
test the driver, but please don't take it as a promise.

--
With best wishes,
Vladimir

2018-05-31 08:43:26

by Clément Péron

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT

Hi Vladimir,

On Thu, 31 May 2018 at 10:33, Vladimir Zapolskiy
<[email protected]> wrote:
>
> On 05/30/2018 03:03 PM, Clément Péron wrote:
> > 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]>
> > Reviewed-by: Fabio Estevam <[email protected]>
> > ---
> > arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index c003e62bf290..0feec516847a 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -844,13 +844,23 @@
> > };
> >
> > epit1: epit@20d0000 { /* EPIT1 */
>
> epit1: timer@20d0000 { ...
>
> And /* EPIT1 */ comment can be removed, it is quite clear from the same
> line context.
>
> Formally it is a subject to another patch, but I think this can be
> accepted as a part of this one.

Should I also update other boards ?
I only did it for imx6qdl.dtsi, but the EPIT is present in other boards
but i can't test it myself.

>
> > + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> > reg = <0x020d0000 0x4000>;
> > interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> > + <&clks IMX6QDL_CLK_EPIT1>;
> > + clock-names = "ipg", "per";
> > + status = "disabled";
> > };
> >
> > epit2: epit@20d4000 { /* EPIT2 */
>
> Same as above.
>
> > + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> > reg = <0x020d4000 0x4000>;
> > interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> > + <&clks IMX6QDL_CLK_EPIT2>;
> > + clock-names = "ipg", "per";
> > + status = "disabled";
> > };
> >
> > src: src@20d8000 {
> >
>
> --
> With best wishes,
> Vladimir

2018-05-31 08:54:55

by Vladimir Zapolskiy

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT

Hi Clément,

On 05/31/2018 11:41 AM, Clément Péron wrote:
> Hi Vladimir,
>
> On Thu, 31 May 2018 at 10:33, Vladimir Zapolskiy
> <[email protected]> wrote:
>>
>> On 05/30/2018 03:03 PM, Clément Péron wrote:
>>> 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]>
>>> Reviewed-by: Fabio Estevam <[email protected]>
>>> ---
>>> arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
>>> index c003e62bf290..0feec516847a 100644
>>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
>>> @@ -844,13 +844,23 @@
>>> };
>>>
>>> epit1: epit@20d0000 { /* EPIT1 */
>>
>> epit1: timer@20d0000 { ...
>>
>> And /* EPIT1 */ comment can be removed, it is quite clear from the same
>> line context.
>>
>> Formally it is a subject to another patch, but I think this can be
>> accepted as a part of this one.
>
> Should I also update other boards ?
> I only did it for imx6qdl.dtsi, but the EPIT is present in other boards
> but i can't test it myself.
>

Sure, please do it, why not, it is quite a safe modification.

One change per one dtsi file will suffice, and I see that imx25.dtsi
already contains the requested change, however probably you may
want to update its compatible = "fsl,imx25-epit" line.

Regarding compatibles for other imx6* SoCs, I think all of them should
be documented in fsl,imxepit.txt and then added to the correspondent
dtsi files one per SoC.

And I forgot the outcome of one former discussion with Uwe Kleine-König,
but if my bad memory serves me, we agreed that i.MX25 was released later
than i.MX31, so the most generic (the last value in the list) compatible
should be a compatible with i.MX31 like in

imx25.dtsi:367: compatible = "fsl,imx25-gpt", "fsl,imx31-gpt";

--
With best wishes,
Vladimir

2018-06-04 09:11:18

by Clément Péron

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT

Hi Vladimir,

On Thu, 31 May 2018 at 10:54, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Clément,
>
> On 05/31/2018 11:41 AM, Clément Péron wrote:
> > Hi Vladimir,
> >
> > On Thu, 31 May 2018 at 10:33, Vladimir Zapolskiy
> > <[email protected]> wrote:
> >>
> >> On 05/30/2018 03:03 PM, Clément Péron wrote:
> >>> 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]>
> >>> Reviewed-by: Fabio Estevam <[email protected]>
> >>> ---
> >>> arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
> >>> 1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> >>> index c003e62bf290..0feec516847a 100644
> >>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> >>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> >>> @@ -844,13 +844,23 @@
> >>> };
> >>>
> >>> epit1: epit@20d0000 { /* EPIT1 */
> >>
> >> epit1: timer@20d0000 { ...
> >>
> >> And /* EPIT1 */ comment can be removed, it is quite clear from the same
> >> line context.
> >>
> >> Formally it is a subject to another patch, but I think this can be
> >> accepted as a part of this one.
> >
> > Should I also update other boards ?
> > I only did it for imx6qdl.dtsi, but the EPIT is present in other boards
> > but i can't test it myself.
> >
>
> Sure, please do it, why not, it is quite a safe modification.
>
> One change per one dtsi file will suffice, and I see that imx25.dtsi
> already contains the requested change, however probably you may
> want to update its compatible = "fsl,imx25-epit" line.
>
> Regarding compatibles for other imx6* SoCs, I think all of them should
> be documented in fsl,imxepit.txt and then added to the correspondent
> dtsi files one per SoC.

Nvidia UART doc did this :
- For other Tegra, must contain '"nvidia,<chip>-uart",
"nvidia,tegra20-uart"' where <chip> is tegra30, tegra114, tegra124,

I will follow this.

>
> And I forgot the outcome of one former discussion with Uwe Kleine-König,
> but if my bad memory serves me, we agreed that i.MX25 was released later
> than i.MX31, so the most generic (the last value in the list) compatible
> should be a compatible with i.MX31 like in
>
> imx25.dtsi:367: compatible = "fsl,imx25-gpt", "fsl,imx31-gpt";
>
> --
> With best wishes,
> Vladimir

Regards,
Clement

2018-06-04 09:23:26

by Clément Péron

[permalink] [raw]
Subject: Re: [PATCH v4 4/5] clocksource: add driver for i.MX EPIT timer

Hi Vladimir,

On Thu, 31 May 2018 at 10:36, Vladimir Zapolskiy
<[email protected]> wrote:
>
> Hi Clément,
>
> On 05/30/2018 03:03 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.
> >
> > Signed-off-by: Colin Didier <[email protected]>
> > Signed-off-by: Clément Peron <[email protected]>
> > ---
>
> [snip]
>
> > +++ b/drivers/clocksource/timer-imx-epit.c
> > @@ -0,0 +1,281 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * i.MX EPIT Timer
> > + *
> > + * Copyright (C) 2010 Sascha Hauer <[email protected]>
> > + * Copyright (C) 2018 Colin Didier <[email protected]>
> > + * Copyright (C) 2018 Clément Péron <[email protected]>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/err.h>
>
> The included header above still can be removed.

Ok.

>
> I have no more comments about the code, I will try to find time to
> test the driver, but please don't take it as a promise.

Regarding the clks, i think the management of the ipg clk in the
driver is useless
has it is already handled by the imx clk driver.

I remove the ipg clk and test it on i.MX6Q.

My test is limited to disabled the GPT and enabled the EPIT in the device-tree
&gpt {
status = "disabled";
};

&epit1 {
status = "okay";
};

[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 4.17.0-rc6 (cperon@cperon-Latitude-7490)
(gcc version 6.4.1 20170707 (Linaro GCC 6.4-2017.08)) #1 SMP PREEMPT
Mon Jun 4 11:13:41 CEST 2018
[ 0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d
[ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[ 0.000000] OF: fdt: Machine model: Devialet Aerobase
[ 0.000000] Memory policy: Data cache writealloc
[ 0.000000] On node 0 totalpages: 262144
[ 0.000000] Normal zone: 2048 pages used for memmap
[ 0.000000] Normal zone: 0 pages reserved
[ 0.000000] Normal zone: 262144 pages, LIFO batch:31
[ 0.000000] random: get_random_bytes called from
start_kernel+0x80/0x398 with crng_init=0
[ 0.000000] percpu: Embedded 16 pages/cpu @(ptrval) s35084 r8192
d22260 u65536
[ 0.000000] pcpu-alloc: s35084 r8192 d22260 u65536 alloc=16*4096
[ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 260096
[ 0.000000] Kernel command line: console=ttymxc0,115200
root=/dev/nfs rw nfsroot=192.168.0.4:/opt/nfsroot,v3,tcp ip=dhcp
[ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[ 0.000000] Memory: 1029544K/1048576K available (6144K kernel code,
194K rwdata, 1312K rodata, 1024K init, 224K bss, 19032K reserved, 0K
cma-reserved)
[ 0.000000] Virtual kernel memory layout:
[ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[ 0.000000] Preemptible hierarchical RCU implementation.
[ 0.000000] Tasks RCU enabled.\x00
[ 0.000000] NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
[ 0.000000] L2C-310 errata 752271 769419 enabled
[ 0.000000] L2C-310 enabling early BRESP for Cortex-A9
[ 0.000000] L2C-310 full line of zeros enabled for Cortex-A9
[ 0.000000] L2C-310 ID prefetch enabled, offset 16 lines
[ 0.000000] L2C-310 dynamic clock gating enabled, standby mode enabled
[ 0.000000] L2C-310 cache controller enabled, 16 ways, 1024 kB
[ 0.000000] L2C-310: CACHE_ID 0x410000c7, AUX_CTRL 0x76470001
[ 0.000012] sched_clock: 32 bits at 66MHz, resolution 15ns, wraps
every 32537631224ns
[ 0.000032] clocksource: epit: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 28958491609 ns
[ 0.001231] Calibrating delay loop... 1560.57 BogoMIPS (lpj=780288)
[ 0.008161] pid_max: default: 32768 minimum: 301
[ 0.008336] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.008360] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.009042] CPU: Testing write buffer coherency: ok
[ 0.009506] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[ 0.014238] Setting up static identity map for 0x10100000 - 0x10100060
[ 0.015207] Hierarchical SRCU implementation.
[ 0.017208] smp: Bringing up secondary CPUs ...
[ 0.029147] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[ 0.041146] CPU2: thread -1, cpu 2, socket 0, mpidr 80000002
[ 0.053146] CPU3: thread -1, cpu 3, socket 0, mpidr 80000003
[ 0.053321] smp: Brought up 1 node, 4 CPUs
[ 0.053340] SMP: Total of 4 processors activated (6303.74 BogoMIPS).
[ 0.053349] CPU: All CPU(s) started in SVC mode.
[ 0.054441] devtmpfs: initialized
[ 0.063770] VFP support v0.3: implementor 41 architecture 3 part 30
variant 9 rev 4
[ 0.064454] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 1911260446275000 ns
[ 0.064477] futex hash table entries: 1024 (order: 4, 65536 bytes)
[ 0.064742] pinctrl core: initialized pinctrl subsystem
[ 0.066693] NET: Registered protocol family 16
[ 0.068623] DMA: preallocated 256 KiB pool for atomic coherent allocations
[ 0.071654] cpuidle: using governor menu
[ 0.071802] CPU identified as i.MX6Q, silicon rev 1.2


>
> --
> With best wishes,
> Vladimir