Re: [PATCH 2/2] m68k: m5441x: Add DMA timer support
From: Jean-Michel Hautbois
Date: Sun Dec 22 2024 - 09:31:01 EST
Hi Greg,
On 22/12/2024 14:41, Greg Ungerer wrote:
Hi JM,
On 2/12/24 19:29, Jean-Michel Hautbois wrote:
Introduce support for the DMA Timer (DTIM) modules found in ColdFire
processors, covering DTIM0 to DTIM3. These are 32-bits timers with 8ns
resolution at 125MHz (fsys/2).
We use it as a sched_clock as well.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>
---
MAINTAINERS | 6 +
arch/m68k/include/asm/m5441xsim.h | 18 +++
drivers/clocksource/Kconfig | 9 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/mcf_dma_timer.c | 240 ++++++++++++++++++++++++++
++++++++++
5 files changed, 274 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index
1e930c7a58b13d8bbe6bf133ba7b36aa24c2b5e0..21e8646f29e2ee726798bbb28a31e921bcaf6011 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9047,6 +9047,12 @@ S: Maintained
F: drivers/mmc/host/sdhci-esdhc-mcf.c
F: include/linux/platform_data/mmc-esdhc-mcf.h
+FREESCALE COLDFIRE M5441X DMA TIMER DRIVER
+M: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>
+L: linux-m68k@xxxxxxxxxxxxxxx
+S: Maintained
+F: drivers/clocksource/mcf_dma_timer.c
+
FREESCALE DIU FRAMEBUFFER DRIVER
M: Timur Tabi <timur@xxxxxxxxxx>
L: linux-fbdev@xxxxxxxxxxxxxxx
diff --git a/arch/m68k/include/asm/m5441xsim.h b/arch/m68k/include/
asm/m5441xsim.h
index
f48cf63bd7822fd53c33788128f984585c0c421a..c46f874bf4a24137bc45300cb3013cb13b47f5ec 100644
--- a/arch/m68k/include/asm/m5441xsim.h
+++ b/arch/m68k/include/asm/m5441xsim.h
@@ -101,6 +101,24 @@
#define MCFINT2_PIT3 16
#define MCFINT2_RTC 26
+/*
+ * DMA timer module.
+ */
+#define MCFDMATIMER_BASE0 0xFC070000 /* Base address of DMA
timer 0 */
+#define MCFDMATIMER_BASE1 0xFC074000 /* Base address of DMA
timer 1 */
+#define MCFDMATIMER_BASE2 0xFC078000 /* Base address of DMA
timer 2 */
+#define MCFDMATIMER_BASE3 0xFC07C000 /* Base address of DMA
timer 3 */
+
+#define MCFDMATIMER_IRQ_DTIM0 (MCFINT0_VECBASE + MCFINT0_TIMER0)
+#define MCFDMATIMER_IRQ_DTIM1 (MCFINT0_VECBASE + MCFINT0_TIMER1)
+#define MCFDMATIMER_IRQ_DTIM2 (MCFINT0_VECBASE + MCFINT0_TIMER2)
+#define MCFDMATIMER_IRQ_DTIM3 (MCFINT0_VECBASE + MCFINT0_TIMER3)
+
+#define MCFDMATIMER_IRQ_PRIO0 (MCFINTC0_ICR0 + MCFINT0_TIMER0)
+#define MCFDMATIMER_IRQ_PRIO1 (MCFINTC0_ICR0 + MCFINT0_TIMER1)
+#define MCFDMATIMER_IRQ_PRIO2 (MCFINTC0_ICR0 + MCFINT0_TIMER2)
+#define MCFDMATIMER_IRQ_PRIO3 (MCFINTC0_ICR0 + MCFINT0_TIMER3)
None of these are actually used directly in the driver code below.
Maybe separate them out into a separate patch.
Sure, along with a DMA timer in device.c, this would make more sense I
suppose ?
+
/*
* PIT timer module.
*/
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index
487c8525996724fbf9c6e9726dabb478d86513b9..3cb3237ef788170f1c2d2bf5db3d47adbeca5664 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -763,4 +763,13 @@ config RALINK_TIMER
Enables support for system tick counter present on
Ralink SoCs RT3352 and MT7620.
+config COLDFIRE_DMA_TIMERS
+ bool "Coldfire DMA timer clocksource"
+ depends on M5441x
+ select GENERIC_SCHED_CLOCK
+ help
+ Enables support for the Coldfire M5441x DMA timers as
+ a clocksource and a clockevent. It supports oneshot and
+ high resolution.
+
endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index
43ef16a4efa6a33bf69ca05a8d66d682826841c9..3a740cd5d52a6f5e68c4e8a0831a2f3306f56941 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -92,3 +92,4 @@ obj-$(CONFIG_GXP_TIMER) += timer-gxp.o
obj-$(CONFIG_CLKSRC_LOONGSON1_PWM) += timer-loongson1-pwm.o
obj-$(CONFIG_EP93XX_TIMER) += timer-ep93xx.o
obj-$(CONFIG_RALINK_TIMER) += timer-ralink.o
+obj-$(CONFIG_COLDFIRE_DMA_TIMERS) += mcf_dma_timer.o
diff --git a/drivers/clocksource/mcf_dma_timer.c b/drivers/
clocksource/mcf_dma_timer.c
new file mode 100644
index
0000000000000000000000000000000000000000..f5cb0f92fcecffcd51a6e73ca38e5a8d8bade0a9
--- /dev/null
+++ b/drivers/clocksource/mcf_dma_timer.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mcf_dma_timer.c -- Freescale ColdFire DMA Timer.
+ *
+ * Copyright (C) 2024, Jean-Michel Hautbois
<jeanmichel.hautbois@xxxxxxxxxx>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/sched/clock.h>
+#include <linux/sched_clock.h>
+
+#include <asm/machdep.h>
+#include <asm/coldfire.h>
+#include <asm/mcfpit.h>
+#include <asm/mcfsim.h>
+
+struct dmatmr_regs {
+ u16 *dtmr;
+ u8 *dtxmr;
+ u8 *dter;
+ u32 *dtrr;
+ u32 *dtcr;
+ u32 *dtcn;
+};
+
+struct dmatmr_priv {
+ void __iomem *base;
+ struct clk *clk;
+ struct platform_device *pdev;
+ unsigned long rate;
+ raw_spinlock_t lock;
+ struct clock_event_device ced;
+ struct clocksource cs;
+ int id;
+ struct dmatmr_regs regs;
+};
+
+static u64 sched_dtim_clk_val;
+static void __iomem *dmatmr_sched_clk_counter;
+
+static struct dmatmr_priv *ced_to_priv(struct clock_event_device *ced)
+{
+ return container_of(ced, struct dmatmr_priv, ced);
+}
+
+static void sys_dtim_init(struct dmatmr_priv *priv)
+{
+ __raw_writel((priv->rate / HZ) - 1, priv->regs.dtrr);
+ __raw_writew(BIT(4) | BIT(1) | BIT(0), priv->regs.dtmr);
+ __raw_writeb(0, priv->regs.dtxmr);
+ __raw_writeb(1, priv->regs.dter);
+
+ dev_info(&priv->pdev->dev, "Initialized for sched_clock at %d
hz", HZ);
+}
+
+static inline u64 notrace read_dtcn(void)
+{
+ return __raw_readl(dmatmr_sched_clk_counter);
+}
+
+static u64 notrace sys_dtim_read(void)
+{
+ return sched_dtim_clk_val + read_dtcn();
+}
+
+static u64 cfv4_read_dtimvalue(struct clocksource *cs)
+{
+ return sys_dtim_read();
+}
+
+static int cfv4_set_next_event(unsigned long delta,
+ struct clock_event_device *dev)
+{
+ struct dmatmr_priv *priv = ced_to_priv(dev);
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&priv->lock, flags);
+ /* read timer value */
+ sched_dtim_clk_val += read_dtcn();
+ raw_spin_unlock_irqrestore(&priv->lock, flags);
+
+ /* reset timer with delta cycle */
+ __raw_writew(0, priv->regs.dtmr);
+ __raw_writel(delta, priv->regs.dtrr);
+ __raw_writew(BIT(4) | BIT(1) | BIT(0), priv->regs.dtmr);
+
+ return 0;
+}
+
+static int cfv4_set_oneshot(struct clock_event_device *dev)
+{
+ struct dmatmr_priv *priv = ced_to_priv(dev);
+ unsigned long flags;
+
+ /* read timer value */
+ raw_spin_lock_irqsave(&priv->lock, flags);
+ sched_dtim_clk_val += read_dtcn();
+ raw_spin_unlock_irqrestore(&priv->lock, flags);
+
+ __raw_writew(0, priv->regs.dtmr);
+ return 0;
+}
+
+static irqreturn_t coldfire_dtim_clk_irq(int irq, void *dev)
+{
+ struct dmatmr_priv *priv = dev;
+ unsigned long flags;
+
+ /* acknowledge the IRQ */
+ __raw_writeb(BIT(0) | BIT(1), priv->regs.dter);
+
+ /* read timer value */
+ raw_spin_lock_irqsave(&priv->lock, flags);
+ sched_dtim_clk_val += read_dtcn();
+ raw_spin_unlock_irqrestore(&priv->lock, flags);
+
+ /* restart counter */
+ __raw_writel(0, dmatmr_sched_clk_counter);
+
+ priv->ced.event_handler(&priv->ced);
+
+ return IRQ_HANDLED;
+}
+
+static void mcf_dma_register_clocksource(struct dmatmr_priv *priv)
+{
+ struct clocksource *cs = &priv->cs;
+
+ cs->name = dev_name(&priv->pdev->dev);
+ cs->rating = 250;
+ cs->mask = CLOCKSOURCE_MASK(30);
+ cs->read = cfv4_read_dtimvalue;
+ cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
+
+ dev_info(&priv->pdev->dev, "registering clocksource\n");
+
+ clocksource_register_hz(cs, priv->rate);
+}
+
+static void mcf_dma_register_clockevent(struct dmatmr_priv *priv)
+{
+ struct clock_event_device *ced = &priv->ced;
+
+ ced->name = dev_name(&priv->pdev->dev);
+ ced->features = CLOCK_EVT_FEAT_ONESHOT;
+ ced->rating = 250;
+ ced->shift = 20;
+ ced->cpumask = cpumask_of(smp_processor_id());
+ ced->set_state_oneshot = cfv4_set_oneshot;
+ ced->set_next_event = cfv4_set_next_event;
+
+ dev_info(&priv->pdev->dev, "registering clockevent\n");
+
+ clockevents_config_and_register(ced, priv->rate, 2, 0xffffffff);
+}
+
+static void mcf_dma_init_registers(struct dmatmr_priv *priv)
+{
+ struct dmatmr_regs *regs = &priv->regs;
+
+ regs->dtmr = priv->base + 0x0;
+ regs->dtxmr = priv->base + 0x2;
+ regs->dter = priv->base + 0x3;
+ regs->dtrr = priv->base + 0x4;
+ regs->dtcr = priv->base + 0x8;
+ regs->dtcn = priv->base + 0xc;
Why define pointers for each register like this and not define
the offsets and use those directly with the read/write calls?
Something like this:
#define DTER 3
__raw_writeb(BIT(0) | BIT(1), priv->base + DTER)
For one thing the setup of variables here is losing the "void __iomem *"
type.
I guess you get away with in this case because the __raw_read/__raw_write
macros force volatile and casting and directly access.
Thanks, I will do that.
I also removed the raw_spin_locks as we always are in a IRQ disabled
context when I use those. And in
sys_dtim_read() I changed it to:
return READ_ONCE(sched_dtim_clk_val) + read_dtcn();
If it makes sense ?
I sometimes have the issue I mentionned in
https://lore.kernel.org/linux-m68k/125431ab-e486-43a7-a903-76c831da4985@xxxxxxxxxx/
: using cyclictest with high prio and short interval with a very high
cpu usage and network with iperf3 or hping3 I get an unexpected IRQ vector.
I don't know how to debug this, but I think it is related to this
driver, as I don't think I observed those in another context.
Thanks !
JM
Regards
Greg
+}
+
+static int __init mcf_dma_timer_probe(struct platform_device *pdev)
+{
+ struct dmatmr_priv *priv;
+ struct resource *prio_reg;
+ int irq, ret;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->pdev = pdev;
+ platform_set_drvdata(pdev, priv);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ priv->id = pdev->id;
+
+ ret = devm_request_irq(&pdev->dev, irq, coldfire_dtim_clk_irq,
IRQF_TIMER,
+ dev_name(&pdev->dev), priv);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request irq %d\n", irq);
+ return ret;
+ }
+
+ prio_reg = platform_get_resource_byname(pdev, IORESOURCE_REG,
"prio_reg");
+ if (prio_reg) {
+ /* Enhance the interrupt priority */
+ __raw_writeb(5, prio_reg->start);
+ }
+
+ priv->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "failed to get clock\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ priv->rate = clk_get_rate(priv->clk);
+
+ raw_spin_lock_init(&priv->lock);
+
+ mcf_dma_init_registers(priv);
+ dmatmr_sched_clk_counter = priv->regs.dtcn;
+
+ mcf_dma_register_clockevent(priv);
+ mcf_dma_register_clocksource(priv);
+
+ /* initialize the system timer */
+ sys_dtim_init(priv);
+
+ sched_clock_register(sys_dtim_read, 32, priv->rate);
+
+ return 0;
+}
+
+static struct platform_driver mcf_platform_driver = {
+ .driver = {
+ .name = "mcftmr",
+ },
+};
+
+builtin_platform_driver_probe(mcf_platform_driver, mcf_dma_timer_probe);