[PATCH 0/5] m68k: add ColdFire watchdog driver

[PATCH 0/5] m68k: add ColdFire watchdog driver

[Angelo Dureghello]

This patch allows to reach 0 warning for the m68k
family. Watchdog driver was the last one producing the
"conversion to DM" warning,

Angelo Dureghello (5):
  drivers: watchdog: add mcf watchdog support
  m68k: move watchdog functions in mcf_wdt driver
  m68k: dts: add watchdog node
  configs: m68k: add watchdog driver
  MAINTAINERS: add myself as mcf_wdt.c maintainer

 MAINTAINERS                           |   1 +
 arch/m68k/cpu/mcf523x/cpu.c           |  42 -------
 arch/m68k/cpu/mcf52x2/cpu.c           |  47 +-------
 arch/m68k/cpu/mcf532x/cpu.c           |  44 -------
 arch/m68k/dts/M5208EVBE.dts           |   5 +
 arch/m68k/dts/mcf5208.dtsi            |   7 ++
 arch/m68k/dts/mcf523x.dtsi            |   7 ++
 arch/m68k/dts/mcf5271.dtsi            |   7 ++
 arch/m68k/dts/mcf5275.dtsi            |   7 ++
 arch/m68k/dts/mcf5282.dtsi            |   7 ++
 arch/m68k/dts/mcf5329.dtsi            |   7 ++
 arch/m68k/dts/mcf537x.dtsi            |   7 ++
 configs/M5208EVBE_defconfig           |   2 +
 configs/astro_mcf5373l_defconfig      |   4 +-
 configs/eb_cpu5282_defconfig          |   1 +
 configs/eb_cpu5282_internal_defconfig |   1 +
 drivers/watchdog/Kconfig              |   7 ++
 drivers/watchdog/Makefile             |   1 +
 drivers/watchdog/mcf_wdt.c            | 162 ++++++++++++++++++++++++++
 19 files changed, 232 insertions(+), 134 deletions(-)
 create mode 100644 drivers/watchdog/mcf_wdt.c

-- 
2.41.0

[PATCH 1/5] drivers: watchdog: add mcf watchdog support

[Angelo Dureghello]

This watchdog driver applies to the following
mcf families:

- mcf52x2 (5271 5275 5282)
- mcf532x (5329 5373)
- mcf523x (5235)

Cpu's not listed for each family does not have WDT module.

Note, after some attempts testing by qemu on 5208 i
finally abandoned, watchdog seems not implemented properly.

The driver has been tested in a real M5282EVM.

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 drivers/watchdog/Kconfig   |   7 ++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/mcf_wdt.c | 162 +++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 drivers/watchdog/mcf_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 646663528a..07fc4940e9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -178,6 +178,13 @@ config WDT_MAX6370
 	help
 	  Select this to enable max6370 watchdog timer.
 
+config WDT_MCF
+	bool "ColdFire family watchdog timer support"
+	depends on WDT
+	help
+	  Select this to enable ColdFire watchdog timer,
+	  which supports mcf52x2 mcf532x mcf523x families.
+
 config WDT_MESON_GXBB
 	bool "Amlogic watchdog timer support"
 	depends on WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index fd5d9c7376..eef786f5e7 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
 obj-$(CONFIG_WDT_FTWDT010) += ftwdt010_wdt.o
 obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
 obj-$(CONFIG_WDT_MAX6370) += max6370_wdt.o
+obj-$(CONFIG_WDT_MCF) += mcf_wdt.o
 obj-$(CONFIG_WDT_MESON_GXBB) += meson_gxbb_wdt.o
 obj-$(CONFIG_WDT_MPC8xxx) += mpc8xxx_wdt.o
 obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
diff --git a/drivers/watchdog/mcf_wdt.c b/drivers/watchdog/mcf_wdt.c
new file mode 100644
index 0000000000..03842135c5
--- /dev/null
+++ b/drivers/watchdog/mcf_wdt.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mcf_wdt.c - driver for ColdFire on-chip watchdog
+ *
+ * Author: Angelo Dureghello <angelo@kernel-space.org>
+ *
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <hang.h>
+#include <asm/io.h>
+#include <wdt.h>
+#include <linux/bitops.h>
+
+#define TIMEOUT_MAX	3360
+#define TIMEOUT_MIN	500
+
+#define DIVIDER_5XXX	4096
+#define DIVIDER_5282	8192
+
+#define WCR_EN          BIT(0)
+#define WCR_HALTED      BIT(1)
+#define WCR_DOZE        BIT(2)
+#define WCR_WAIT        BIT(3)
+
+struct watchdog_regs {
+	u16 wcr;        /* Control */
+	u16 wmr;        /* Service */
+	u16 wcntr;      /* Counter */
+	u16 wsr;        /* Reset Status */
+};
+
+#if defined(CONFIG_WDT_MCF)
+static void mcf_watchdog_reset(struct watchdog_regs *wdog)
+{
+#ifndef CONFIG_WATCHDOG_RESET_DISABLE
+	writew(0x5555, &wdog->wsr);
+	writew(0xaaaa, &wdog->wsr);
+#endif /* CONFIG_WATCHDOG_RESET_DISABLE*/
+}
+
+static void mcf_watchdog_init(struct watchdog_regs *wdog, u32 fixed_divider,
+			      u64 timeout_msecs)
+{
+	u32 wdog_module;
+
+	/*
+	 * The timer watchdog can be set between
+	 * 0.5 and 128 Seconds.
+	 */
+
+	/* set timeout and enable watchdog */
+	wdog_module = ((CFG_SYS_CLK / 1000) * timeout_msecs);
+
+	writew((u16)(wdog_module / fixed_divider), &wdog->wmr);
+	writew(WCR_EN, &wdog->wcr);
+
+	mcf_watchdog_reset(wdog);
+}
+
+#if !CONFIG_IS_ENABLED(WDT)
+void hw_watchdog_reset(void)
+{
+	struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
+
+	mcf_watchdog_reset(wdog);
+}
+
+void hw_watchdog_init(void)
+{
+	struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
+
+	if (IS_ENABLED(CONFIG_MCF5282))
+		writew(&wdog->wmr, wdog_module / DIVIDER_5282);
+	else
+		writew(&wdog->wmr, wdog_module / DIVIDER_5XXX);
+
+	mcf_watchdog_init(wdog, CONFIG_WATCHDOG_TIMEOUT_MSECS);
+}
+
+#else /* CONFIG_WDT */
+
+struct mcf_wdt_priv {
+	void __iomem *base;
+	u32 fixed_divider;
+};
+
+static int mcf_wdt_expire_now(struct udevice *dev, ulong flags)
+{
+	hang();
+
+	return 0;
+}
+
+static int mcf_wdt_reset(struct udevice *dev)
+{
+	struct mcf_wdt_priv *priv = dev_get_priv(dev);
+
+	mcf_watchdog_reset(priv->base);
+
+	return 0;
+}
+
+static int mcf_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
+{
+	struct mcf_wdt_priv *priv = dev_get_priv(dev);
+
+	/* Timeout from fdt is in second, driver works in msecs */
+	mcf_watchdog_init(priv->base, priv->fixed_divider,
+			  timeout * 1000);
+
+	return 0;
+}
+
+static int mcf_wdt_stop(struct udevice *dev)
+{
+	struct mcf_wdt_priv *priv = dev_get_priv(dev);
+	struct watchdog_regs *wdog = (struct watchdog_regs *)priv->base;
+
+	setbits_be16(&wdog->wcr, WCR_HALTED);
+
+	return 0;
+}
+
+static int mcf_wdt_probe(struct udevice *dev)
+{
+	struct mcf_wdt_priv *priv = dev_get_priv(dev);
+
+	priv->base = dev_read_addr_ptr(dev);
+	if (!priv->base)
+		return -ENOENT;
+
+	priv->fixed_divider = (u32)dev_get_driver_data(dev);
+
+	return 0;
+}
+
+static const struct wdt_ops mcf_wdt_ops = {
+	.start		= mcf_wdt_start,
+	.stop		= mcf_wdt_stop,
+	.reset		= mcf_wdt_reset,
+	.expire_now	= mcf_wdt_expire_now,
+};
+
+static const struct udevice_id mcf_wdt_ids[] = {
+	{ .compatible = "fsl,mcf5208-wdt", .data = DIVIDER_5XXX },
+	{ .compatible = "fsl,mcf5282-wdt", .data = DIVIDER_5282 },
+	{}
+};
+
+U_BOOT_DRIVER(mcf_wdt) = {
+	.name		= "mcf_wdt",
+	.id		= UCLASS_WDT,
+	.of_match	= mcf_wdt_ids,
+	.probe		= mcf_wdt_probe,
+	.ops		= &mcf_wdt_ops,
+	.priv_auto	= sizeof(struct mcf_wdt_priv),
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+#endif
+#endif
-- 
2.41.0

[PATCH 2/5] m68k: move watchdog functions in mcf_wdt driver

[Angelo Dureghello]

Move watchdog functions inside a separate watchdog driver.

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 arch/m68k/cpu/mcf523x/cpu.c | 42 ---------------------------------
 arch/m68k/cpu/mcf52x2/cpu.c | 47 +------------------------------------
 arch/m68k/cpu/mcf532x/cpu.c | 44 ----------------------------------
 3 files changed, 1 insertion(+), 132 deletions(-)

diff --git a/arch/m68k/cpu/mcf523x/cpu.c b/arch/m68k/cpu/mcf523x/cpu.c
index ba2c228911..bef67767b4 100644
--- a/arch/m68k/cpu/mcf523x/cpu.c
+++ b/arch/m68k/cpu/mcf523x/cpu.c
@@ -12,7 +12,6 @@
 #include <init.h>
 #include <net.h>
 #include <vsprintf.h>
-#include <watchdog.h>
 #include <command.h>
 #include <netdev.h>
 #include <asm/global_data.h>
@@ -62,47 +61,6 @@ int print_cpuinfo(void)
 };
 #endif /* CONFIG_DISPLAY_CPUINFO */
 
-#if defined(CONFIG_WATCHDOG)
-/* Called by macro WATCHDOG_RESET */
-void watchdog_reset(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-
-	/* Count register */
-	out_be16(&wdp->sr, 0x5555);
-	asm("nop");
-	out_be16(&wdp->sr, 0xaaaa);
-}
-
-int watchdog_disable(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-
-	/* UserManual, once the wdog is disabled, wdog cannot be re-enabled */
-	/* halted watchdog timer */
-	setbits_be16(&wdp->cr, WTM_WCR_HALTED);
-
-	puts("WATCHDOG:disabled\n");
-	return (0);
-}
-
-int watchdog_init(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-	u32 wdog_module = 0;
-
-	/* set timeout and enable watchdog */
-	wdog_module = ((CFG_SYS_CLK / CONFIG_SYS_HZ) * CONFIG_WATCHDOG_TIMEOUT_MSECS);
-	wdog_module |= (wdog_module / 8192);
-	out_be16(&wdp->mr, wdog_module);
-
-	out_be16(&wdp->cr, WTM_WCR_EN);
-	puts("WATCHDOG:enabled\n");
-
-	return (0);
-}
-#endif				/* CONFIG_WATCHDOG */
-
 #if defined(CONFIG_MCFFEC)
 /* Default initializations for MCFFEC controllers.  To override,
  * create a board-specific function called:
diff --git a/arch/m68k/cpu/mcf52x2/cpu.c b/arch/m68k/cpu/mcf52x2/cpu.c
index d7cbf11e25..5042a38b3e 100644
--- a/arch/m68k/cpu/mcf52x2/cpu.c
+++ b/arch/m68k/cpu/mcf52x2/cpu.c
@@ -17,7 +17,6 @@
 #include <init.h>
 #include <net.h>
 #include <vsprintf.h>
-#include <watchdog.h>
 #include <command.h>
 #include <asm/global_data.h>
 #include <asm/immap.h>
@@ -53,51 +52,7 @@ int print_cpuinfo(void)
 	return 0;
 };
 #endif /* CONFIG_DISPLAY_CPUINFO */
-
-#if defined(CONFIG_WATCHDOG)
-/* Called by macro WATCHDOG_RESET */
-void watchdog_reset(void)
-{
-	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
-
-	out_be16(&wdt->sr, 0x5555);
-	out_be16(&wdt->sr, 0xaaaa);
-}
-
-int watchdog_disable(void)
-{
-	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
-
-	/* reset watchdog counter */
-	out_be16(&wdt->sr, 0x5555);
-	out_be16(&wdt->sr, 0xaaaa);
-	/* disable watchdog timer */
-	out_be16(&wdt->cr, 0);
-
-	puts("WATCHDOG:disabled\n");
-	return (0);
-}
-
-int watchdog_init(void)
-{
-	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
-
-	/* disable watchdog */
-	out_be16(&wdt->cr, 0);
-
-	/* set timeout and enable watchdog */
-	out_be16(&wdt->mr,
-		(CONFIG_WATCHDOG_TIMEOUT_MSECS * CONFIG_SYS_HZ) / (32768 * 1000) - 1);
-
-	/* reset watchdog counter */
-	out_be16(&wdt->sr, 0x5555);
-	out_be16(&wdt->sr, 0xaaaa);
-
-	puts("WATCHDOG:enabled\n");
-	return (0);
-}
-#endif				/* #ifdef CONFIG_WATCHDOG */
-#endif				/* #ifdef CONFIG_M5208 */
+#endif /* #ifdef CONFIG_M5208 */
 
 #ifdef  CONFIG_M5271
 #if defined(CONFIG_DISPLAY_CPUINFO)
diff --git a/arch/m68k/cpu/mcf532x/cpu.c b/arch/m68k/cpu/mcf532x/cpu.c
index 548cbca36a..18d20a8926 100644
--- a/arch/m68k/cpu/mcf532x/cpu.c
+++ b/arch/m68k/cpu/mcf532x/cpu.c
@@ -12,7 +12,6 @@
 #include <init.h>
 #include <net.h>
 #include <vsprintf.h>
-#include <watchdog.h>
 #include <command.h>
 #include <netdev.h>
 #include <asm/global_data.h>
@@ -102,49 +101,6 @@ int print_cpuinfo(void)
 };
 #endif /* CONFIG_DISPLAY_CPUINFO */
 
-#if defined(CONFIG_WATCHDOG)
-/* Called by macro WATCHDOG_RESET */
-void watchdog_reset(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-
-	/* Count register */
-	out_be16(&wdp->sr, 0x5555);
-	out_be16(&wdp->sr, 0xaaaa);
-}
-
-int watchdog_disable(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-
-	/* UserManual, once the wdog is disabled, wdog cannot be re-enabled */
-	/* halted watchdog timer */
-	setbits_be16(&wdp->cr, WTM_WCR_HALTED);
-
-	puts("WATCHDOG:disabled\n");
-	return (0);
-}
-
-int watchdog_init(void)
-{
-	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
-	u32 wdog_module = 0;
-
-	/* set timeout and enable watchdog */
-	wdog_module = ((CFG_SYS_CLK / 1000) * CONFIG_WATCHDOG_TIMEOUT_MSECS);
-#ifdef CONFIG_M5329
-	out_be16(&wdp->mr, wdog_module / 8192);
-#else
-	out_be16(&wdp->mr, wdog_module / 4096);
-#endif
-
-	out_be16(&wdp->cr, WTM_WCR_EN);
-	puts("WATCHDOG:enabled\n");
-
-	return (0);
-}
-#endif				/* CONFIG_WATCHDOG */
-
 #if defined(CONFIG_MCFFEC)
 /* Default initializations for MCFFEC controllers.  To override,
  * create a board-specific function called:
-- 
2.41.0

[PATCH 3/5] m68k: dts: add watchdog node

[Angelo Dureghello]

Add watchdog node for the implemented mcf_wdt driver.

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 arch/m68k/dts/M5208EVBE.dts | 5 +++++
 arch/m68k/dts/mcf5208.dtsi  | 7 +++++++
 arch/m68k/dts/mcf523x.dtsi  | 7 +++++++
 arch/m68k/dts/mcf5271.dtsi  | 7 +++++++
 arch/m68k/dts/mcf5275.dtsi  | 7 +++++++
 arch/m68k/dts/mcf5282.dtsi  | 7 +++++++
 arch/m68k/dts/mcf5329.dtsi  | 7 +++++++
 arch/m68k/dts/mcf537x.dtsi  | 7 +++++++
 8 files changed, 54 insertions(+)

diff --git a/arch/m68k/dts/M5208EVBE.dts b/arch/m68k/dts/M5208EVBE.dts
index 1c32718af4..ec203e8b69 100644
--- a/arch/m68k/dts/M5208EVBE.dts
+++ b/arch/m68k/dts/M5208EVBE.dts
@@ -15,6 +15,11 @@
 	};
 };
 
+&wdog0 {
+	timeout-sec = <32>;
+	status = "okay";
+};
+
 &uart0 {
 	bootph-all;
 	status = "okay";
diff --git a/arch/m68k/dts/mcf5208.dtsi b/arch/m68k/dts/mcf5208.dtsi
index 9392facfa8..b06dc4bb26 100644
--- a/arch/m68k/dts/mcf5208.dtsi
+++ b/arch/m68k/dts/mcf5208.dtsi
@@ -16,6 +16,13 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		wdog0: watchdog@fc08c000 {
+			compatible = "fsl,mcf5208-wdt";
+			reg = <0xfc08c000 0x10>;
+			big-endian;
+			status = "disabled";
+		};
+
 		uart0: uart@fc060000 {
 			compatible = "fsl,mcf-uart";
 			reg = <0xfc060000 0x40>;
diff --git a/arch/m68k/dts/mcf523x.dtsi b/arch/m68k/dts/mcf523x.dtsi
index 41c7b9b2d1..fb5a4cdc21 100644
--- a/arch/m68k/dts/mcf523x.dtsi
+++ b/arch/m68k/dts/mcf523x.dtsi
@@ -23,6 +23,13 @@
 			ranges = <0x00000000 0x40000000 0x40000000>;
 			reg = <0x40000000 0x40000000>;
 
+			wdog0: watchdog@140000 {
+				compatible = "fsl,mcf5208-wdt";
+				reg = <0x140000 0x10>;
+				big-endian;
+				status = "disabled";
+			};
+
 			uart0: uart@200 {
 				compatible = "fsl,mcf-uart";
 				reg = <0x200 0x40>;
diff --git a/arch/m68k/dts/mcf5271.dtsi b/arch/m68k/dts/mcf5271.dtsi
index fc82bd3c24..0884c13ab1 100644
--- a/arch/m68k/dts/mcf5271.dtsi
+++ b/arch/m68k/dts/mcf5271.dtsi
@@ -23,6 +23,13 @@
 			ranges = <0x00000000 0x40000000 0x40000000>;
 			reg = <0x40000000 0x40000000>;
 
+			wdog0: watchdog@140000 {
+				compatible = "fsl,mcf5208-wdt";
+				reg = <0x140000 0x10>;
+				big-endian;
+				status = "disabled";
+			};
+
 			uart0: uart@200 {
 				compatible = "fsl,mcf-uart";
 				reg = <0x200 0x40>;
diff --git a/arch/m68k/dts/mcf5275.dtsi b/arch/m68k/dts/mcf5275.dtsi
index 402517cdec..78210569da 100644
--- a/arch/m68k/dts/mcf5275.dtsi
+++ b/arch/m68k/dts/mcf5275.dtsi
@@ -24,6 +24,13 @@
 			ranges = <0x00000000 0x40000000 0x40000000>;
 			reg = <0x40000000 0x40000000>;
 
+			wdog0: watchdog@140000 {
+				compatible = "fsl,mcf5208-wdt";
+				reg = <0x140000 0x10>;
+				big-endian;
+				status = "disabled";
+			};
+
 			uart0: uart@200 {
 				compatible = "fsl,mcf-uart";
 				reg = <0x200 0x40>;
diff --git a/arch/m68k/dts/mcf5282.dtsi b/arch/m68k/dts/mcf5282.dtsi
index 883c0d0324..40704c5202 100644
--- a/arch/m68k/dts/mcf5282.dtsi
+++ b/arch/m68k/dts/mcf5282.dtsi
@@ -23,6 +23,13 @@
 			ranges = <0x00000000 0x40000000 0x40000000>;
 			reg = <0x40000000 0x40000000>;
 
+			wdog0: watchdog@140000 {
+				compatible = "fsl,mcf5282-wdt";
+				reg = <0x140000 0x10>;
+				big-endian;
+				status = "disabled";
+			};
+
 			uart0: uart@200 {
 				compatible = "fsl,mcf-uart";
 				reg = <0x200 0x40>;
diff --git a/arch/m68k/dts/mcf5329.dtsi b/arch/m68k/dts/mcf5329.dtsi
index 7501cc4b01..50ff73bca7 100644
--- a/arch/m68k/dts/mcf5329.dtsi
+++ b/arch/m68k/dts/mcf5329.dtsi
@@ -16,6 +16,13 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		wdog0: watchdog@fc098000 {
+			compatible = "fsl,mcf5208-wdt";
+			reg = <0xfc08c000 0x10>;
+			big-endian;
+			status = "disabled";
+		};
+
 		uart0: uart@fc060000 {
 			compatible = "fsl,mcf-uart";
 			reg = <0xfc060000 0x40>;
diff --git a/arch/m68k/dts/mcf537x.dtsi b/arch/m68k/dts/mcf537x.dtsi
index 338b8b4583..23c3754df0 100644
--- a/arch/m68k/dts/mcf537x.dtsi
+++ b/arch/m68k/dts/mcf537x.dtsi
@@ -16,6 +16,13 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		wdog0: watchdog@fc098000 {
+			compatible = "fsl,mcf5208-wdt";
+			reg = <0xfc08c000 0x10>;
+			big-endian;
+			status = "disabled";
+		};
+
 		uart0: uart@fc060000 {
 			compatible = "fsl,mcf-uart";
 			reg = <0xfc060000 0x40>;
-- 
2.41.0

[PATCH 4/5] configs: m68k: add watchdog driver

[Angelo Dureghello]

Add config options for mcf_wdt driver.

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 configs/M5208EVBE_defconfig           | 2 ++
 configs/astro_mcf5373l_defconfig      | 4 ++--
 configs/eb_cpu5282_defconfig          | 1 +
 configs/eb_cpu5282_internal_defconfig | 1 +
 4 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/configs/M5208EVBE_defconfig b/configs/M5208EVBE_defconfig
index 0c5afe869b..aa054f753f 100644
--- a/configs/M5208EVBE_defconfig
+++ b/configs/M5208EVBE_defconfig
@@ -50,3 +50,5 @@ CONFIG_MCFFEC=y
 CONFIG_MII=y
 CONFIG_MCFUART=y
 CONFIG_WATCHDOG_TIMEOUT_MSECS=5000
+CONFIG_WDT=y
+CONFIG_WDT_MCF=y
diff --git a/configs/astro_mcf5373l_defconfig b/configs/astro_mcf5373l_defconfig
index aade1f98be..f4dad3bcc8 100644
--- a/configs/astro_mcf5373l_defconfig
+++ b/configs/astro_mcf5373l_defconfig
@@ -46,5 +46,5 @@ CONFIG_DM_RTC=y
 CONFIG_MCFRTC=y
 CONFIG_SYS_MCFRTC_BASE=0xFC0A8000
 CONFIG_MCFUART=y
-CONFIG_WATCHDOG=y
-CONFIG_WATCHDOG_TIMEOUT_MSECS=3355
+CONFIG_WDT=y
+CONFIG_WDT_MCF=y
diff --git a/configs/eb_cpu5282_defconfig b/configs/eb_cpu5282_defconfig
index 24edecb510..2873322598 100644
--- a/configs/eb_cpu5282_defconfig
+++ b/configs/eb_cpu5282_defconfig
@@ -52,3 +52,4 @@ CONFIG_MII=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1338=y
 CONFIG_MCFUART=y
+CONFIG_WDT=y
diff --git a/configs/eb_cpu5282_internal_defconfig b/configs/eb_cpu5282_internal_defconfig
index 44e22eb01d..bd780034ba 100644
--- a/configs/eb_cpu5282_internal_defconfig
+++ b/configs/eb_cpu5282_internal_defconfig
@@ -50,3 +50,4 @@ CONFIG_MII=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1338=y
 CONFIG_MCFUART=y
+CONFIG_WDT=y
-- 
2.41.0

[PATCH 5/5] MAINTAINERS: add myself as mcf_wdt.c maintainer

[Angelo Dureghello]

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 228d8af433..59d011ffcf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -865,6 +865,7 @@ S:	Maintained
 T:	git https://source.denx.de/u-boot/custodians/u-boot-coldfire.git
 F:	arch/m68k/
 F:	doc/arch/m68k.rst
+F:	drivers/watchdog/mcf_wdt.c
 
 CYCLIC
 M:	Stefan Roese <sr@denx.de>
-- 
2.41.0

Re: [PATCH 1/5] drivers: watchdog: add mcf watchdog support

[Stefan Roese]

On 6/25/23 21:35, Angelo Dureghello wrote:
> This watchdog driver applies to the following
> mcf families:
> 
> - mcf52x2 (5271 5275 5282)
> - mcf532x (5329 5373)
> - mcf523x (5235)
> 
> Cpu's not listed for each family does not have WDT module.
> 
> Note, after some attempts testing by qemu on 5208 i
> finally abandoned, watchdog seems not implemented properly.
> 
> The driver has been tested in a real M5282EVM.
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
>   drivers/watchdog/Kconfig   |   7 ++
>   drivers/watchdog/Makefile  |   1 +
>   drivers/watchdog/mcf_wdt.c | 162 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 170 insertions(+)
>   create mode 100644 drivers/watchdog/mcf_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 646663528a..07fc4940e9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -178,6 +178,13 @@ config WDT_MAX6370
>   	help
>   	  Select this to enable max6370 watchdog timer.
>   
> +config WDT_MCF
> +	bool "ColdFire family watchdog timer support"
> +	depends on WDT
> +	help
> +	  Select this to enable ColdFire watchdog timer,
> +	  which supports mcf52x2 mcf532x mcf523x families.
> +
>   config WDT_MESON_GXBB
>   	bool "Amlogic watchdog timer support"
>   	depends on WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index fd5d9c7376..eef786f5e7 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
>   obj-$(CONFIG_WDT_FTWDT010) += ftwdt010_wdt.o
>   obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
>   obj-$(CONFIG_WDT_MAX6370) += max6370_wdt.o
> +obj-$(CONFIG_WDT_MCF) += mcf_wdt.o
>   obj-$(CONFIG_WDT_MESON_GXBB) += meson_gxbb_wdt.o
>   obj-$(CONFIG_WDT_MPC8xxx) += mpc8xxx_wdt.o
>   obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
> diff --git a/drivers/watchdog/mcf_wdt.c b/drivers/watchdog/mcf_wdt.c
> new file mode 100644
> index 0000000000..03842135c5
> --- /dev/null
> +++ b/drivers/watchdog/mcf_wdt.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mcf_wdt.c - driver for ColdFire on-chip watchdog
> + *
> + * Author: Angelo Dureghello <angelo@kernel-space.org>
> + *
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <hang.h>
> +#include <asm/io.h>
> +#include <wdt.h>
> +#include <linux/bitops.h>
> +
> +#define TIMEOUT_MAX	3360
> +#define TIMEOUT_MIN	500

Those 2 defines above are not used in this source. Please drop.

> +
> +#define DIVIDER_5XXX	4096
> +#define DIVIDER_5282	8192
> +
> +#define WCR_EN          BIT(0)
> +#define WCR_HALTED      BIT(1)
> +#define WCR_DOZE        BIT(2)
> +#define WCR_WAIT        BIT(3)
> +
> +struct watchdog_regs {
> +	u16 wcr;        /* Control */
> +	u16 wmr;        /* Service */
> +	u16 wcntr;      /* Counter */
> +	u16 wsr;        /* Reset Status */
> +};
> +
> +#if defined(CONFIG_WDT_MCF)

Is it possible to drop this "#if" somehow?

> +static void mcf_watchdog_reset(struct watchdog_regs *wdog)
> +{
> +#ifndef CONFIG_WATCHDOG_RESET_DISABLE
> +	writew(0x5555, &wdog->wsr);
> +	writew(0xaaaa, &wdog->wsr);
> +#endif /* CONFIG_WATCHDOG_RESET_DISABLE*/

checkpatch.pl warning:

WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' 
where possible
#37: FILE: drivers/watchdog/mcf_wdt.c:37:
+#ifndef CONFIG_WATCHDOG_RESET_DISABLE

> +}
> +
> +static void mcf_watchdog_init(struct watchdog_regs *wdog, u32 fixed_divider,
> +			      u64 timeout_msecs)
> +{
> +	u32 wdog_module;
> +
> +	/*
> +	 * The timer watchdog can be set between
> +	 * 0.5 and 128 Seconds.
> +	 */
> +
> +	/* set timeout and enable watchdog */
> +	wdog_module = ((CFG_SYS_CLK / 1000) * timeout_msecs);
> +
> +	writew((u16)(wdog_module / fixed_divider), &wdog->wmr);
> +	writew(WCR_EN, &wdog->wcr);
> +
> +	mcf_watchdog_reset(wdog);
> +}
> +
> +#if !CONFIG_IS_ENABLED(WDT)
> +void hw_watchdog_reset(void)
> +{
> +	struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
> +
> +	mcf_watchdog_reset(wdog);
> +}

Can you perhaps completely remove this hw_watchdog stuff? Is it really
needed on these Coldfire platforms?

> +
> +void hw_watchdog_init(void)
> +{
> +	struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
> +
> +	if (IS_ENABLED(CONFIG_MCF5282))
> +		writew(&wdog->wmr, wdog_module / DIVIDER_5282);
> +	else
> +		writew(&wdog->wmr, wdog_module / DIVIDER_5XXX);
> +
> +	mcf_watchdog_init(wdog, CONFIG_WATCHDOG_TIMEOUT_MSECS);
> +}
> +
> +#else /* CONFIG_WDT */
> +
> +struct mcf_wdt_priv {
> +	void __iomem *base;
> +	u32 fixed_divider;
> +};
> +
> +static int mcf_wdt_expire_now(struct udevice *dev, ulong flags)
> +{
> +	hang();
> +
> +	return 0;
> +}
> +
> +static int mcf_wdt_reset(struct udevice *dev)
> +{
> +	struct mcf_wdt_priv *priv = dev_get_priv(dev);
> +
> +	mcf_watchdog_reset(priv->base);
> +
> +	return 0;
> +}
> +
> +static int mcf_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
> +{
> +	struct mcf_wdt_priv *priv = dev_get_priv(dev);
> +
> +	/* Timeout from fdt is in second, driver works in msecs */

I'm a bit confused here. AFAIK, the time passed into the start()
function is in ms:

/*
  * Start the timer
  *
  * @dev: WDT Device
  * @timeout_ms: Number of ticks (milliseconds) before timer expires
  * @flags: Driver specific flags. This might be used to specify
  * which action needs to be executed when the timer expires
  * @return: 0 if OK, -ve on error
  */
int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags);

So there should be no need to convert to ms here in this
function. Please double-check here again.

Thanks,
Stefan

> +	mcf_watchdog_init(priv->base, priv->fixed_divider,
> +			  timeout * 1000);
> +
> +	return 0;
> +}
> +
> +static int mcf_wdt_stop(struct udevice *dev)
> +{
> +	struct mcf_wdt_priv *priv = dev_get_priv(dev);
> +	struct watchdog_regs *wdog = (struct watchdog_regs *)priv->base;
> +
> +	setbits_be16(&wdog->wcr, WCR_HALTED);
> +
> +	return 0;
> +}
> +
> +static int mcf_wdt_probe(struct udevice *dev)
> +{
> +	struct mcf_wdt_priv *priv = dev_get_priv(dev);
> +
> +	priv->base = dev_read_addr_ptr(dev);
> +	if (!priv->base)
> +		return -ENOENT;
> +
> +	priv->fixed_divider = (u32)dev_get_driver_data(dev);
> +
> +	return 0;
> +}
> +
> +static const struct wdt_ops mcf_wdt_ops = {
> +	.start		= mcf_wdt_start,
> +	.stop		= mcf_wdt_stop,
> +	.reset		= mcf_wdt_reset,
> +	.expire_now	= mcf_wdt_expire_now,
> +};
> +
> +static const struct udevice_id mcf_wdt_ids[] = {
> +	{ .compatible = "fsl,mcf5208-wdt", .data = DIVIDER_5XXX },
> +	{ .compatible = "fsl,mcf5282-wdt", .data = DIVIDER_5282 },
> +	{}
> +};
> +
> +U_BOOT_DRIVER(mcf_wdt) = {
> +	.name		= "mcf_wdt",
> +	.id		= UCLASS_WDT,
> +	.of_match	= mcf_wdt_ids,
> +	.probe		= mcf_wdt_probe,
> +	.ops		= &mcf_wdt_ops,
> +	.priv_auto	= sizeof(struct mcf_wdt_priv),
> +	.flags		= DM_FLAG_PRE_RELOC,
> +};
> +#endif
> +#endif

Re: [PATCH 2/5] m68k: move watchdog functions in mcf_wdt driver

[Stefan Roese]

On 6/25/23 21:35, Angelo Dureghello wrote:
> Move watchdog functions inside a separate watchdog driver.
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
>   arch/m68k/cpu/mcf523x/cpu.c | 42 ---------------------------------
>   arch/m68k/cpu/mcf52x2/cpu.c | 47 +------------------------------------
>   arch/m68k/cpu/mcf532x/cpu.c | 44 ----------------------------------
>   3 files changed, 1 insertion(+), 132 deletions(-)

Now this is a nice diff-stat. Thanks for cleaning this up and moving
all this stuff into the watchdog driver. And again, please double-check,
if the hw_foo stuff (now in the wdt driver) can be completely removed.

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> diff --git a/arch/m68k/cpu/mcf523x/cpu.c b/arch/m68k/cpu/mcf523x/cpu.c
> index ba2c228911..bef67767b4 100644
> --- a/arch/m68k/cpu/mcf523x/cpu.c
> +++ b/arch/m68k/cpu/mcf523x/cpu.c
> @@ -12,7 +12,6 @@
>   #include <init.h>
>   #include <net.h>
>   #include <vsprintf.h>
> -#include <watchdog.h>
>   #include <command.h>
>   #include <netdev.h>
>   #include <asm/global_data.h>
> @@ -62,47 +61,6 @@ int print_cpuinfo(void)
>   };
>   #endif /* CONFIG_DISPLAY_CPUINFO */
>   
> -#if defined(CONFIG_WATCHDOG)
> -/* Called by macro WATCHDOG_RESET */
> -void watchdog_reset(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -
> -	/* Count register */
> -	out_be16(&wdp->sr, 0x5555);
> -	asm("nop");
> -	out_be16(&wdp->sr, 0xaaaa);
> -}
> -
> -int watchdog_disable(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -
> -	/* UserManual, once the wdog is disabled, wdog cannot be re-enabled */
> -	/* halted watchdog timer */
> -	setbits_be16(&wdp->cr, WTM_WCR_HALTED);
> -
> -	puts("WATCHDOG:disabled\n");
> -	return (0);
> -}
> -
> -int watchdog_init(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -	u32 wdog_module = 0;
> -
> -	/* set timeout and enable watchdog */
> -	wdog_module = ((CFG_SYS_CLK / CONFIG_SYS_HZ) * CONFIG_WATCHDOG_TIMEOUT_MSECS);
> -	wdog_module |= (wdog_module / 8192);
> -	out_be16(&wdp->mr, wdog_module);
> -
> -	out_be16(&wdp->cr, WTM_WCR_EN);
> -	puts("WATCHDOG:enabled\n");
> -
> -	return (0);
> -}
> -#endif				/* CONFIG_WATCHDOG */
> -
>   #if defined(CONFIG_MCFFEC)
>   /* Default initializations for MCFFEC controllers.  To override,
>    * create a board-specific function called:
> diff --git a/arch/m68k/cpu/mcf52x2/cpu.c b/arch/m68k/cpu/mcf52x2/cpu.c
> index d7cbf11e25..5042a38b3e 100644
> --- a/arch/m68k/cpu/mcf52x2/cpu.c
> +++ b/arch/m68k/cpu/mcf52x2/cpu.c
> @@ -17,7 +17,6 @@
>   #include <init.h>
>   #include <net.h>
>   #include <vsprintf.h>
> -#include <watchdog.h>
>   #include <command.h>
>   #include <asm/global_data.h>
>   #include <asm/immap.h>
> @@ -53,51 +52,7 @@ int print_cpuinfo(void)
>   	return 0;
>   };
>   #endif /* CONFIG_DISPLAY_CPUINFO */
> -
> -#if defined(CONFIG_WATCHDOG)
> -/* Called by macro WATCHDOG_RESET */
> -void watchdog_reset(void)
> -{
> -	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
> -
> -	out_be16(&wdt->sr, 0x5555);
> -	out_be16(&wdt->sr, 0xaaaa);
> -}
> -
> -int watchdog_disable(void)
> -{
> -	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
> -
> -	/* reset watchdog counter */
> -	out_be16(&wdt->sr, 0x5555);
> -	out_be16(&wdt->sr, 0xaaaa);
> -	/* disable watchdog timer */
> -	out_be16(&wdt->cr, 0);
> -
> -	puts("WATCHDOG:disabled\n");
> -	return (0);
> -}
> -
> -int watchdog_init(void)
> -{
> -	wdog_t *wdt = (wdog_t *)(MMAP_WDOG);
> -
> -	/* disable watchdog */
> -	out_be16(&wdt->cr, 0);
> -
> -	/* set timeout and enable watchdog */
> -	out_be16(&wdt->mr,
> -		(CONFIG_WATCHDOG_TIMEOUT_MSECS * CONFIG_SYS_HZ) / (32768 * 1000) - 1);
> -
> -	/* reset watchdog counter */
> -	out_be16(&wdt->sr, 0x5555);
> -	out_be16(&wdt->sr, 0xaaaa);
> -
> -	puts("WATCHDOG:enabled\n");
> -	return (0);
> -}
> -#endif				/* #ifdef CONFIG_WATCHDOG */
> -#endif				/* #ifdef CONFIG_M5208 */
> +#endif /* #ifdef CONFIG_M5208 */
>   
>   #ifdef  CONFIG_M5271
>   #if defined(CONFIG_DISPLAY_CPUINFO)
> diff --git a/arch/m68k/cpu/mcf532x/cpu.c b/arch/m68k/cpu/mcf532x/cpu.c
> index 548cbca36a..18d20a8926 100644
> --- a/arch/m68k/cpu/mcf532x/cpu.c
> +++ b/arch/m68k/cpu/mcf532x/cpu.c
> @@ -12,7 +12,6 @@
>   #include <init.h>
>   #include <net.h>
>   #include <vsprintf.h>
> -#include <watchdog.h>
>   #include <command.h>
>   #include <netdev.h>
>   #include <asm/global_data.h>
> @@ -102,49 +101,6 @@ int print_cpuinfo(void)
>   };
>   #endif /* CONFIG_DISPLAY_CPUINFO */
>   
> -#if defined(CONFIG_WATCHDOG)
> -/* Called by macro WATCHDOG_RESET */
> -void watchdog_reset(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -
> -	/* Count register */
> -	out_be16(&wdp->sr, 0x5555);
> -	out_be16(&wdp->sr, 0xaaaa);
> -}
> -
> -int watchdog_disable(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -
> -	/* UserManual, once the wdog is disabled, wdog cannot be re-enabled */
> -	/* halted watchdog timer */
> -	setbits_be16(&wdp->cr, WTM_WCR_HALTED);
> -
> -	puts("WATCHDOG:disabled\n");
> -	return (0);
> -}
> -
> -int watchdog_init(void)
> -{
> -	wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
> -	u32 wdog_module = 0;
> -
> -	/* set timeout and enable watchdog */
> -	wdog_module = ((CFG_SYS_CLK / 1000) * CONFIG_WATCHDOG_TIMEOUT_MSECS);
> -#ifdef CONFIG_M5329
> -	out_be16(&wdp->mr, wdog_module / 8192);
> -#else
> -	out_be16(&wdp->mr, wdog_module / 4096);
> -#endif
> -
> -	out_be16(&wdp->cr, WTM_WCR_EN);
> -	puts("WATCHDOG:enabled\n");
> -
> -	return (0);
> -}
> -#endif				/* CONFIG_WATCHDOG */
> -
>   #if defined(CONFIG_MCFFEC)
>   /* Default initializations for MCFFEC controllers.  To override,
>    * create a board-specific function called:

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 3/5] m68k: dts: add watchdog node

[Stefan Roese]

On 6/25/23 21:35, Angelo Dureghello wrote:
> Add watchdog node for the implemented mcf_wdt driver.
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
>   arch/m68k/dts/M5208EVBE.dts | 5 +++++
>   arch/m68k/dts/mcf5208.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf523x.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf5271.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf5275.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf5282.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf5329.dtsi  | 7 +++++++
>   arch/m68k/dts/mcf537x.dtsi  | 7 +++++++
>   8 files changed, 54 insertions(+)
> 
> diff --git a/arch/m68k/dts/M5208EVBE.dts b/arch/m68k/dts/M5208EVBE.dts
> index 1c32718af4..ec203e8b69 100644
> --- a/arch/m68k/dts/M5208EVBE.dts
> +++ b/arch/m68k/dts/M5208EVBE.dts
> @@ -15,6 +15,11 @@
>   	};
>   };
>   
> +&wdog0 {
> +	timeout-sec = <32>;
> +	status = "okay";
> +};
> +
>   &uart0 {
>   	bootph-all;
>   	status = "okay";
> diff --git a/arch/m68k/dts/mcf5208.dtsi b/arch/m68k/dts/mcf5208.dtsi
> index 9392facfa8..b06dc4bb26 100644
> --- a/arch/m68k/dts/mcf5208.dtsi
> +++ b/arch/m68k/dts/mcf5208.dtsi
> @@ -16,6 +16,13 @@
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   
> +		wdog0: watchdog@fc08c000 {
> +			compatible = "fsl,mcf5208-wdt";
> +			reg = <0xfc08c000 0x10>;
> +			big-endian;
> +			status = "disabled";
> +		};

I was not able to find this compatible property in the Linux Kernel
source tree. Is this the official version of the Coldfire WDT DT node?
Just checking..

Thanks,
Stefan

> +
>   		uart0: uart@fc060000 {
>   			compatible = "fsl,mcf-uart";
>   			reg = <0xfc060000 0x40>;
> diff --git a/arch/m68k/dts/mcf523x.dtsi b/arch/m68k/dts/mcf523x.dtsi
> index 41c7b9b2d1..fb5a4cdc21 100644
> --- a/arch/m68k/dts/mcf523x.dtsi
> +++ b/arch/m68k/dts/mcf523x.dtsi
> @@ -23,6 +23,13 @@
>   			ranges = <0x00000000 0x40000000 0x40000000>;
>   			reg = <0x40000000 0x40000000>;
>   
> +			wdog0: watchdog@140000 {
> +				compatible = "fsl,mcf5208-wdt";
> +				reg = <0x140000 0x10>;
> +				big-endian;
> +				status = "disabled";
> +			};
> +
>   			uart0: uart@200 {
>   				compatible = "fsl,mcf-uart";
>   				reg = <0x200 0x40>;
> diff --git a/arch/m68k/dts/mcf5271.dtsi b/arch/m68k/dts/mcf5271.dtsi
> index fc82bd3c24..0884c13ab1 100644
> --- a/arch/m68k/dts/mcf5271.dtsi
> +++ b/arch/m68k/dts/mcf5271.dtsi
> @@ -23,6 +23,13 @@
>   			ranges = <0x00000000 0x40000000 0x40000000>;
>   			reg = <0x40000000 0x40000000>;
>   
> +			wdog0: watchdog@140000 {
> +				compatible = "fsl,mcf5208-wdt";
> +				reg = <0x140000 0x10>;
> +				big-endian;
> +				status = "disabled";
> +			};
> +
>   			uart0: uart@200 {
>   				compatible = "fsl,mcf-uart";
>   				reg = <0x200 0x40>;
> diff --git a/arch/m68k/dts/mcf5275.dtsi b/arch/m68k/dts/mcf5275.dtsi
> index 402517cdec..78210569da 100644
> --- a/arch/m68k/dts/mcf5275.dtsi
> +++ b/arch/m68k/dts/mcf5275.dtsi
> @@ -24,6 +24,13 @@
>   			ranges = <0x00000000 0x40000000 0x40000000>;
>   			reg = <0x40000000 0x40000000>;
>   
> +			wdog0: watchdog@140000 {
> +				compatible = "fsl,mcf5208-wdt";
> +				reg = <0x140000 0x10>;
> +				big-endian;
> +				status = "disabled";
> +			};
> +
>   			uart0: uart@200 {
>   				compatible = "fsl,mcf-uart";
>   				reg = <0x200 0x40>;
> diff --git a/arch/m68k/dts/mcf5282.dtsi b/arch/m68k/dts/mcf5282.dtsi
> index 883c0d0324..40704c5202 100644
> --- a/arch/m68k/dts/mcf5282.dtsi
> +++ b/arch/m68k/dts/mcf5282.dtsi
> @@ -23,6 +23,13 @@
>   			ranges = <0x00000000 0x40000000 0x40000000>;
>   			reg = <0x40000000 0x40000000>;
>   
> +			wdog0: watchdog@140000 {
> +				compatible = "fsl,mcf5282-wdt";
> +				reg = <0x140000 0x10>;
> +				big-endian;
> +				status = "disabled";
> +			};
> +
>   			uart0: uart@200 {
>   				compatible = "fsl,mcf-uart";
>   				reg = <0x200 0x40>;
> diff --git a/arch/m68k/dts/mcf5329.dtsi b/arch/m68k/dts/mcf5329.dtsi
> index 7501cc4b01..50ff73bca7 100644
> --- a/arch/m68k/dts/mcf5329.dtsi
> +++ b/arch/m68k/dts/mcf5329.dtsi
> @@ -16,6 +16,13 @@
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   
> +		wdog0: watchdog@fc098000 {
> +			compatible = "fsl,mcf5208-wdt";
> +			reg = <0xfc08c000 0x10>;
> +			big-endian;
> +			status = "disabled";
> +		};
> +
>   		uart0: uart@fc060000 {
>   			compatible = "fsl,mcf-uart";
>   			reg = <0xfc060000 0x40>;
> diff --git a/arch/m68k/dts/mcf537x.dtsi b/arch/m68k/dts/mcf537x.dtsi
> index 338b8b4583..23c3754df0 100644
> --- a/arch/m68k/dts/mcf537x.dtsi
> +++ b/arch/m68k/dts/mcf537x.dtsi
> @@ -16,6 +16,13 @@
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   
> +		wdog0: watchdog@fc098000 {
> +			compatible = "fsl,mcf5208-wdt";
> +			reg = <0xfc08c000 0x10>;
> +			big-endian;
> +			status = "disabled";
> +		};
> +
>   		uart0: uart@fc060000 {
>   			compatible = "fsl,mcf-uart";
>   			reg = <0xfc060000 0x40>;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 3/5] m68k: dts: add watchdog node

[Angelo Dureghello]

Hi Stefan,

On 18/07/23 2:26 PM, Stefan Roese wrote:
> On 6/25/23 21:35, Angelo Dureghello wrote:
>> Add watchdog node for the implemented mcf_wdt driver.
>>
>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>> ---
>>   arch/m68k/dts/M5208EVBE.dts | 5 +++++
>>   arch/m68k/dts/mcf5208.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf523x.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf5271.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf5275.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf5282.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf5329.dtsi  | 7 +++++++
>>   arch/m68k/dts/mcf537x.dtsi  | 7 +++++++
>>   8 files changed, 54 insertions(+)
>>
>> diff --git a/arch/m68k/dts/M5208EVBE.dts b/arch/m68k/dts/M5208EVBE.dts
>> index 1c32718af4..ec203e8b69 100644
>> --- a/arch/m68k/dts/M5208EVBE.dts
>> +++ b/arch/m68k/dts/M5208EVBE.dts
>> @@ -15,6 +15,11 @@
>>       };
>>   };
>> +&wdog0 {
>> +    timeout-sec = <32>;
>> +    status = "okay";
>> +};
>> +
>>   &uart0 {
>>       bootph-all;
>>       status = "okay";
>> diff --git a/arch/m68k/dts/mcf5208.dtsi b/arch/m68k/dts/mcf5208.dtsi
>> index 9392facfa8..b06dc4bb26 100644
>> --- a/arch/m68k/dts/mcf5208.dtsi
>> +++ b/arch/m68k/dts/mcf5208.dtsi
>> @@ -16,6 +16,13 @@
>>           #address-cells = <1>;
>>           #size-cells = <1>;
>> +        wdog0: watchdog@fc08c000 {
>> +            compatible = "fsl,mcf5208-wdt";
>> +            reg = <0xfc08c000 0x10>;
>> +            big-endian;
>> +            status = "disabled";
>> +        };
> 
> I was not able to find this compatible property in the Linux Kernel
> source tree. Is this the official version of the Coldfire WDT DT node?
> Just checking..
> 

there is no fdt support for m68k in Linux, after discussing the need in 
u-boot, i implemented fdt here the best i could.

> Thanks,
> Stefan
> 
>> +
>>           uart0: uart@fc060000 {
>>               compatible = "fsl,mcf-uart";
>>               reg = <0xfc060000 0x40>;
>> diff --git a/arch/m68k/dts/mcf523x.dtsi b/arch/m68k/dts/mcf523x.dtsi
>> index 41c7b9b2d1..fb5a4cdc21 100644
>> --- a/arch/m68k/dts/mcf523x.dtsi
>> +++ b/arch/m68k/dts/mcf523x.dtsi
>> @@ -23,6 +23,13 @@
>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>               reg = <0x40000000 0x40000000>;
>> +            wdog0: watchdog@140000 {
>> +                compatible = "fsl,mcf5208-wdt";
>> +                reg = <0x140000 0x10>;
>> +                big-endian;
>> +                status = "disabled";
>> +            };
>> +
>>               uart0: uart@200 {
>>                   compatible = "fsl,mcf-uart";
>>                   reg = <0x200 0x40>;
>> diff --git a/arch/m68k/dts/mcf5271.dtsi b/arch/m68k/dts/mcf5271.dtsi
>> index fc82bd3c24..0884c13ab1 100644
>> --- a/arch/m68k/dts/mcf5271.dtsi
>> +++ b/arch/m68k/dts/mcf5271.dtsi
>> @@ -23,6 +23,13 @@
>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>               reg = <0x40000000 0x40000000>;
>> +            wdog0: watchdog@140000 {
>> +                compatible = "fsl,mcf5208-wdt";
>> +                reg = <0x140000 0x10>;
>> +                big-endian;
>> +                status = "disabled";
>> +            };
>> +
>>               uart0: uart@200 {
>>                   compatible = "fsl,mcf-uart";
>>                   reg = <0x200 0x40>;
>> diff --git a/arch/m68k/dts/mcf5275.dtsi b/arch/m68k/dts/mcf5275.dtsi
>> index 402517cdec..78210569da 100644
>> --- a/arch/m68k/dts/mcf5275.dtsi
>> +++ b/arch/m68k/dts/mcf5275.dtsi
>> @@ -24,6 +24,13 @@
>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>               reg = <0x40000000 0x40000000>;
>> +            wdog0: watchdog@140000 {
>> +                compatible = "fsl,mcf5208-wdt";
>> +                reg = <0x140000 0x10>;
>> +                big-endian;
>> +                status = "disabled";
>> +            };
>> +
>>               uart0: uart@200 {
>>                   compatible = "fsl,mcf-uart";
>>                   reg = <0x200 0x40>;
>> diff --git a/arch/m68k/dts/mcf5282.dtsi b/arch/m68k/dts/mcf5282.dtsi
>> index 883c0d0324..40704c5202 100644
>> --- a/arch/m68k/dts/mcf5282.dtsi
>> +++ b/arch/m68k/dts/mcf5282.dtsi
>> @@ -23,6 +23,13 @@
>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>               reg = <0x40000000 0x40000000>;
>> +            wdog0: watchdog@140000 {
>> +                compatible = "fsl,mcf5282-wdt";
>> +                reg = <0x140000 0x10>;
>> +                big-endian;
>> +                status = "disabled";
>> +            };
>> +
>>               uart0: uart@200 {
>>                   compatible = "fsl,mcf-uart";
>>                   reg = <0x200 0x40>;
>> diff --git a/arch/m68k/dts/mcf5329.dtsi b/arch/m68k/dts/mcf5329.dtsi
>> index 7501cc4b01..50ff73bca7 100644
>> --- a/arch/m68k/dts/mcf5329.dtsi
>> +++ b/arch/m68k/dts/mcf5329.dtsi
>> @@ -16,6 +16,13 @@
>>           #address-cells = <1>;
>>           #size-cells = <1>;
>> +        wdog0: watchdog@fc098000 {
>> +            compatible = "fsl,mcf5208-wdt";
>> +            reg = <0xfc08c000 0x10>;
>> +            big-endian;
>> +            status = "disabled";
>> +        };
>> +
>>           uart0: uart@fc060000 {
>>               compatible = "fsl,mcf-uart";
>>               reg = <0xfc060000 0x40>;
>> diff --git a/arch/m68k/dts/mcf537x.dtsi b/arch/m68k/dts/mcf537x.dtsi
>> index 338b8b4583..23c3754df0 100644
>> --- a/arch/m68k/dts/mcf537x.dtsi
>> +++ b/arch/m68k/dts/mcf537x.dtsi
>> @@ -16,6 +16,13 @@
>>           #address-cells = <1>;
>>           #size-cells = <1>;
>> +        wdog0: watchdog@fc098000 {
>> +            compatible = "fsl,mcf5208-wdt";
>> +            reg = <0xfc08c000 0x10>;
>> +            big-endian;
>> +            status = "disabled";
>> +        };
>> +
>>           uart0: uart@fc060000 {
>>               compatible = "fsl,mcf-uart";
>>               reg = <0xfc060000 0x40>;
> 
> Viele Grüße,
> Stefan Roese
> 

Regards,
angelo

Re: [PATCH 1/5] drivers: watchdog: add mcf watchdog support

[Angelo Dureghello]

Hi Stefan,

thanks for the feedbacks,

On 18/07/23 2:23 PM, Stefan Roese wrote:
> On 6/25/23 21:35, Angelo Dureghello wrote:
>> This watchdog driver applies to the following
>> mcf families:
>>
>> - mcf52x2 (5271 5275 5282)
>> - mcf532x (5329 5373)
>> - mcf523x (5235)
>>
>> Cpu's not listed for each family does not have WDT module.
>>
>> Note, after some attempts testing by qemu on 5208 i
>> finally abandoned, watchdog seems not implemented properly.
>>
>> The driver has been tested in a real M5282EVM.
>>
>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>> ---
>>   drivers/watchdog/Kconfig   |   7 ++
>>   drivers/watchdog/Makefile  |   1 +
>>   drivers/watchdog/mcf_wdt.c | 162 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 170 insertions(+)
>>   create mode 100644 drivers/watchdog/mcf_wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 646663528a..07fc4940e9 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -178,6 +178,13 @@ config WDT_MAX6370
>>       help
>>         Select this to enable max6370 watchdog timer.
>> +config WDT_MCF
>> +    bool "ColdFire family watchdog timer support"
>> +    depends on WDT
>> +    help
>> +      Select this to enable ColdFire watchdog timer,
>> +      which supports mcf52x2 mcf532x mcf523x families.
>> +
>>   config WDT_MESON_GXBB
>>       bool "Amlogic watchdog timer support"
>>       depends on WDT
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index fd5d9c7376..eef786f5e7 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
>>   obj-$(CONFIG_WDT_FTWDT010) += ftwdt010_wdt.o
>>   obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
>>   obj-$(CONFIG_WDT_MAX6370) += max6370_wdt.o
>> +obj-$(CONFIG_WDT_MCF) += mcf_wdt.o
>>   obj-$(CONFIG_WDT_MESON_GXBB) += meson_gxbb_wdt.o
>>   obj-$(CONFIG_WDT_MPC8xxx) += mpc8xxx_wdt.o
>>   obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
>> diff --git a/drivers/watchdog/mcf_wdt.c b/drivers/watchdog/mcf_wdt.c
>> new file mode 100644
>> index 0000000000..03842135c5
>> --- /dev/null
>> +++ b/drivers/watchdog/mcf_wdt.c
>> @@ -0,0 +1,162 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * mcf_wdt.c - driver for ColdFire on-chip watchdog
>> + *
>> + * Author: Angelo Dureghello <angelo@kernel-space.org>
>> + *
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <hang.h>
>> +#include <asm/io.h>
>> +#include <wdt.h>
>> +#include <linux/bitops.h>
>> +
>> +#define TIMEOUT_MAX    3360
>> +#define TIMEOUT_MIN    500
> 
> Those 2 defines above are not used in this source. Please drop.
> 
ok
>> +
>> +#define DIVIDER_5XXX    4096
>> +#define DIVIDER_5282    8192
>> +
>> +#define WCR_EN          BIT(0)
>> +#define WCR_HALTED      BIT(1)
>> +#define WCR_DOZE        BIT(2)
>> +#define WCR_WAIT        BIT(3)
>> +
>> +struct watchdog_regs {
>> +    u16 wcr;        /* Control */
>> +    u16 wmr;        /* Service */
>> +    u16 wcntr;      /* Counter */
>> +    u16 wsr;        /* Reset Status */
>> +};
>> +
>> +#if defined(CONFIG_WDT_MCF)
> 
> Is it possible to drop this "#if" somehow?
> 
will try
>> +static void mcf_watchdog_reset(struct watchdog_regs *wdog)
>> +{
>> +#ifndef CONFIG_WATCHDOG_RESET_DISABLE
>> +    writew(0x5555, &wdog->wsr);
>> +    writew(0xaaaa, &wdog->wsr);
>> +#endif /* CONFIG_WATCHDOG_RESET_DISABLE*/
> 
> checkpatch.pl warning:
> 
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' 
> where possible
> #37: FILE: drivers/watchdog/mcf_wdt.c:37:
> +#ifndef CONFIG_WATCHDOG_RESET_DISABLE
> 
ok will try to use if (IS_ENABLED

  +}
>> +
>> +static void mcf_watchdog_init(struct watchdog_regs *wdog, u32 
>> fixed_divider,
>> +                  u64 timeout_msecs)
>> +{
>> +    u32 wdog_module;
>> +
>> +    /*
>> +     * The timer watchdog can be set between
>> +     * 0.5 and 128 Seconds.
>> +     */
>> +
>> +    /* set timeout and enable watchdog */
>> +    wdog_module = ((CFG_SYS_CLK / 1000) * timeout_msecs);
>> +
>> +    writew((u16)(wdog_module / fixed_divider), &wdog->wmr);
>> +    writew(WCR_EN, &wdog->wcr);
>> +
>> +    mcf_watchdog_reset(wdog);
>> +}
>> +
>> +#if !CONFIG_IS_ENABLED(WDT)
>> +void hw_watchdog_reset(void)
>> +{
>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
>> +
>> +    mcf_watchdog_reset(wdog);
>> +}
> 
> Can you perhaps completely remove this hw_watchdog stuff? Is it really
> needed on these Coldfire platforms?
> 
Will recheck, maybe yes,
>> +
>> +void hw_watchdog_init(void)
>> +{
>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
>> +
>> +    if (IS_ENABLED(CONFIG_MCF5282))
>> +        writew(&wdog->wmr, wdog_module / DIVIDER_5282);
>> +    else
>> +        writew(&wdog->wmr, wdog_module / DIVIDER_5XXX);
>> +
>> +    mcf_watchdog_init(wdog, CONFIG_WATCHDOG_TIMEOUT_MSECS);
>> +}
>> +
>> +#else /* CONFIG_WDT */
>> +
>> +struct mcf_wdt_priv {
>> +    void __iomem *base;
>> +    u32 fixed_divider;
>> +};
>> +
>> +static int mcf_wdt_expire_now(struct udevice *dev, ulong flags)
>> +{
>> +    hang();
>> +
>> +    return 0;
>> +}
>> +
>> +static int mcf_wdt_reset(struct udevice *dev)
>> +{
>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +    mcf_watchdog_reset(priv->base);
>> +
>> +    return 0;
>> +}
>> +
>> +static int mcf_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>> +{
>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +    /* Timeout from fdt is in second, driver works in msecs */
> 
> I'm a bit confused here. AFAIK, the time passed into the start()
> function is in ms:
> 
> /*
>   * Start the timer
>   *
>   * @dev: WDT Device
>   * @timeout_ms: Number of ticks (milliseconds) before timer expires
>   * @flags: Driver specific flags. This might be used to specify
>   * which action needs to be executed when the timer expires
>   * @return: 0 if OK, -ve on error
>   */
> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags);
> 
> So there should be no need to convert to ms here in this
> function. Please double-check here again.
> 
Thanks, will check.

> Thanks,
> Stefan
> 

Regards,
angelo

>> +    mcf_watchdog_init(priv->base, priv->fixed_divider,
>> +              timeout * 1000);
>> +
>> +    return 0;
>> +}
>> +
>> +static int mcf_wdt_stop(struct udevice *dev)
>> +{
>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)priv->base;
>> +
>> +    setbits_be16(&wdog->wcr, WCR_HALTED);
>> +
>> +    return 0;
>> +}
>> +
>> +static int mcf_wdt_probe(struct udevice *dev)
>> +{
>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +    priv->base = dev_read_addr_ptr(dev);
>> +    if (!priv->base)
>> +        return -ENOENT;
>> +
>> +    priv->fixed_divider = (u32)dev_get_driver_data(dev);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct wdt_ops mcf_wdt_ops = {
>> +    .start        = mcf_wdt_start,
>> +    .stop        = mcf_wdt_stop,
>> +    .reset        = mcf_wdt_reset,
>> +    .expire_now    = mcf_wdt_expire_now,
>> +};
>> +
>> +static const struct udevice_id mcf_wdt_ids[] = {
>> +    { .compatible = "fsl,mcf5208-wdt", .data = DIVIDER_5XXX },
>> +    { .compatible = "fsl,mcf5282-wdt", .data = DIVIDER_5282 },
>> +    {}
>> +};
>> +
>> +U_BOOT_DRIVER(mcf_wdt) = {
>> +    .name        = "mcf_wdt",
>> +    .id        = UCLASS_WDT,
>> +    .of_match    = mcf_wdt_ids,
>> +    .probe        = mcf_wdt_probe,
>> +    .ops        = &mcf_wdt_ops,
>> +    .priv_auto    = sizeof(struct mcf_wdt_priv),
>> +    .flags        = DM_FLAG_PRE_RELOC,
>> +};
>> +#endif
>> +#endif
> 
> 

Re: [PATCH 3/5] m68k: dts: add watchdog node

[Stefan Roese]

Hi Angelo,

On 7/18/23 15:58, Angelo Dureghello wrote:
> Hi Stefan,
> 
> On 18/07/23 2:26 PM, Stefan Roese wrote:
>> On 6/25/23 21:35, Angelo Dureghello wrote:
>>> Add watchdog node for the implemented mcf_wdt driver.
>>>
>>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>>> ---
>>>   arch/m68k/dts/M5208EVBE.dts | 5 +++++
>>>   arch/m68k/dts/mcf5208.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf523x.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf5271.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf5275.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf5282.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf5329.dtsi  | 7 +++++++
>>>   arch/m68k/dts/mcf537x.dtsi  | 7 +++++++
>>>   8 files changed, 54 insertions(+)
>>>
>>> diff --git a/arch/m68k/dts/M5208EVBE.dts b/arch/m68k/dts/M5208EVBE.dts
>>> index 1c32718af4..ec203e8b69 100644
>>> --- a/arch/m68k/dts/M5208EVBE.dts
>>> +++ b/arch/m68k/dts/M5208EVBE.dts
>>> @@ -15,6 +15,11 @@
>>>       };
>>>   };
>>> +&wdog0 {
>>> +    timeout-sec = <32>;
>>> +    status = "okay";
>>> +};
>>> +
>>>   &uart0 {
>>>       bootph-all;
>>>       status = "okay";
>>> diff --git a/arch/m68k/dts/mcf5208.dtsi b/arch/m68k/dts/mcf5208.dtsi
>>> index 9392facfa8..b06dc4bb26 100644
>>> --- a/arch/m68k/dts/mcf5208.dtsi
>>> +++ b/arch/m68k/dts/mcf5208.dtsi
>>> @@ -16,6 +16,13 @@
>>>           #address-cells = <1>;
>>>           #size-cells = <1>;
>>> +        wdog0: watchdog@fc08c000 {
>>> +            compatible = "fsl,mcf5208-wdt";
>>> +            reg = <0xfc08c000 0x10>;
>>> +            big-endian;
>>> +            status = "disabled";
>>> +        };
>>
>> I was not able to find this compatible property in the Linux Kernel
>> source tree. Is this the official version of the Coldfire WDT DT node?
>> Just checking..
>>
> 
> there is no fdt support for m68k in Linux, after discussing the need in 
> u-boot, i implemented fdt here the best i could.

Ok, I see. BTW: Is "big-endian" necessary here?

Thanks,
Stefan

>> Thanks,
>> Stefan
>>
>>> +
>>>           uart0: uart@fc060000 {
>>>               compatible = "fsl,mcf-uart";
>>>               reg = <0xfc060000 0x40>;
>>> diff --git a/arch/m68k/dts/mcf523x.dtsi b/arch/m68k/dts/mcf523x.dtsi
>>> index 41c7b9b2d1..fb5a4cdc21 100644
>>> --- a/arch/m68k/dts/mcf523x.dtsi
>>> +++ b/arch/m68k/dts/mcf523x.dtsi
>>> @@ -23,6 +23,13 @@
>>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>>               reg = <0x40000000 0x40000000>;
>>> +            wdog0: watchdog@140000 {
>>> +                compatible = "fsl,mcf5208-wdt";
>>> +                reg = <0x140000 0x10>;
>>> +                big-endian;
>>> +                status = "disabled";
>>> +            };
>>> +
>>>               uart0: uart@200 {
>>>                   compatible = "fsl,mcf-uart";
>>>                   reg = <0x200 0x40>;
>>> diff --git a/arch/m68k/dts/mcf5271.dtsi b/arch/m68k/dts/mcf5271.dtsi
>>> index fc82bd3c24..0884c13ab1 100644
>>> --- a/arch/m68k/dts/mcf5271.dtsi
>>> +++ b/arch/m68k/dts/mcf5271.dtsi
>>> @@ -23,6 +23,13 @@
>>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>>               reg = <0x40000000 0x40000000>;
>>> +            wdog0: watchdog@140000 {
>>> +                compatible = "fsl,mcf5208-wdt";
>>> +                reg = <0x140000 0x10>;
>>> +                big-endian;
>>> +                status = "disabled";
>>> +            };
>>> +
>>>               uart0: uart@200 {
>>>                   compatible = "fsl,mcf-uart";
>>>                   reg = <0x200 0x40>;
>>> diff --git a/arch/m68k/dts/mcf5275.dtsi b/arch/m68k/dts/mcf5275.dtsi
>>> index 402517cdec..78210569da 100644
>>> --- a/arch/m68k/dts/mcf5275.dtsi
>>> +++ b/arch/m68k/dts/mcf5275.dtsi
>>> @@ -24,6 +24,13 @@
>>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>>               reg = <0x40000000 0x40000000>;
>>> +            wdog0: watchdog@140000 {
>>> +                compatible = "fsl,mcf5208-wdt";
>>> +                reg = <0x140000 0x10>;
>>> +                big-endian;
>>> +                status = "disabled";
>>> +            };
>>> +
>>>               uart0: uart@200 {
>>>                   compatible = "fsl,mcf-uart";
>>>                   reg = <0x200 0x40>;
>>> diff --git a/arch/m68k/dts/mcf5282.dtsi b/arch/m68k/dts/mcf5282.dtsi
>>> index 883c0d0324..40704c5202 100644
>>> --- a/arch/m68k/dts/mcf5282.dtsi
>>> +++ b/arch/m68k/dts/mcf5282.dtsi
>>> @@ -23,6 +23,13 @@
>>>               ranges = <0x00000000 0x40000000 0x40000000>;
>>>               reg = <0x40000000 0x40000000>;
>>> +            wdog0: watchdog@140000 {
>>> +                compatible = "fsl,mcf5282-wdt";
>>> +                reg = <0x140000 0x10>;
>>> +                big-endian;
>>> +                status = "disabled";
>>> +            };
>>> +
>>>               uart0: uart@200 {
>>>                   compatible = "fsl,mcf-uart";
>>>                   reg = <0x200 0x40>;
>>> diff --git a/arch/m68k/dts/mcf5329.dtsi b/arch/m68k/dts/mcf5329.dtsi
>>> index 7501cc4b01..50ff73bca7 100644
>>> --- a/arch/m68k/dts/mcf5329.dtsi
>>> +++ b/arch/m68k/dts/mcf5329.dtsi
>>> @@ -16,6 +16,13 @@
>>>           #address-cells = <1>;
>>>           #size-cells = <1>;
>>> +        wdog0: watchdog@fc098000 {
>>> +            compatible = "fsl,mcf5208-wdt";
>>> +            reg = <0xfc08c000 0x10>;
>>> +            big-endian;
>>> +            status = "disabled";
>>> +        };
>>> +
>>>           uart0: uart@fc060000 {
>>>               compatible = "fsl,mcf-uart";
>>>               reg = <0xfc060000 0x40>;
>>> diff --git a/arch/m68k/dts/mcf537x.dtsi b/arch/m68k/dts/mcf537x.dtsi
>>> index 338b8b4583..23c3754df0 100644
>>> --- a/arch/m68k/dts/mcf537x.dtsi
>>> +++ b/arch/m68k/dts/mcf537x.dtsi
>>> @@ -16,6 +16,13 @@
>>>           #address-cells = <1>;
>>>           #size-cells = <1>;
>>> +        wdog0: watchdog@fc098000 {
>>> +            compatible = "fsl,mcf5208-wdt";
>>> +            reg = <0xfc08c000 0x10>;
>>> +            big-endian;
>>> +            status = "disabled";
>>> +        };
>>> +
>>>           uart0: uart@fc060000 {
>>>               compatible = "fsl,mcf-uart";
>>>               reg = <0xfc060000 0x40>;
>>
>> Viele Grüße,
>> Stefan Roese
>>
> 
> Regards,
> angelo

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 3/5] m68k: dts: add watchdog node

[Simon Glass]

On Tue, 18 Jul 2023 at 08:00, Angelo Dureghello <angelo@kernel-space.org> wrote:
>
> Hi Stefan,
>
> On 18/07/23 2:26 PM, Stefan Roese wrote:
> > On 6/25/23 21:35, Angelo Dureghello wrote:
> >> Add watchdog node for the implemented mcf_wdt driver.
> >>
> >> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> >> ---
> >>   arch/m68k/dts/M5208EVBE.dts | 5 +++++
> >>   arch/m68k/dts/mcf5208.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf523x.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf5271.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf5275.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf5282.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf5329.dtsi  | 7 +++++++
> >>   arch/m68k/dts/mcf537x.dtsi  | 7 +++++++
> >>   8 files changed, 54 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 1/5] drivers: watchdog: add mcf watchdog support

[Stefan Roese]

Hi Angelo,

On 7/18/23 16:06, Angelo Dureghello wrote:
> Hi Stefan,
> 
> thanks for the feedbacks,
> 
> On 18/07/23 2:23 PM, Stefan Roese wrote:
>> On 6/25/23 21:35, Angelo Dureghello wrote:
>>> This watchdog driver applies to the following
>>> mcf families:
>>>
>>> - mcf52x2 (5271 5275 5282)
>>> - mcf532x (5329 5373)
>>> - mcf523x (5235)
>>>
>>> Cpu's not listed for each family does not have WDT module.
>>>
>>> Note, after some attempts testing by qemu on 5208 i
>>> finally abandoned, watchdog seems not implemented properly.
>>>
>>> The driver has been tested in a real M5282EVM.
>>>
>>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>>> ---
>>>   drivers/watchdog/Kconfig   |   7 ++
>>>   drivers/watchdog/Makefile  |   1 +
>>>   drivers/watchdog/mcf_wdt.c | 162 +++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 170 insertions(+)
>>>   create mode 100644 drivers/watchdog/mcf_wdt.c
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 646663528a..07fc4940e9 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -178,6 +178,13 @@ config WDT_MAX6370
>>>       help
>>>         Select this to enable max6370 watchdog timer.
>>> +config WDT_MCF
>>> +    bool "ColdFire family watchdog timer support"
>>> +    depends on WDT
>>> +    help
>>> +      Select this to enable ColdFire watchdog timer,
>>> +      which supports mcf52x2 mcf532x mcf523x families.
>>> +
>>>   config WDT_MESON_GXBB
>>>       bool "Amlogic watchdog timer support"
>>>       depends on WDT
>>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>>> index fd5d9c7376..eef786f5e7 100644
>>> --- a/drivers/watchdog/Makefile
>>> +++ b/drivers/watchdog/Makefile
>>> @@ -31,6 +31,7 @@ obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
>>>   obj-$(CONFIG_WDT_FTWDT010) += ftwdt010_wdt.o
>>>   obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
>>>   obj-$(CONFIG_WDT_MAX6370) += max6370_wdt.o
>>> +obj-$(CONFIG_WDT_MCF) += mcf_wdt.o
>>>   obj-$(CONFIG_WDT_MESON_GXBB) += meson_gxbb_wdt.o
>>>   obj-$(CONFIG_WDT_MPC8xxx) += mpc8xxx_wdt.o
>>>   obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
>>> diff --git a/drivers/watchdog/mcf_wdt.c b/drivers/watchdog/mcf_wdt.c
>>> new file mode 100644
>>> index 0000000000..03842135c5
>>> --- /dev/null
>>> +++ b/drivers/watchdog/mcf_wdt.c
>>> @@ -0,0 +1,162 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * mcf_wdt.c - driver for ColdFire on-chip watchdog
>>> + *
>>> + * Author: Angelo Dureghello <angelo@kernel-space.org>
>>> + *
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <hang.h>
>>> +#include <asm/io.h>
>>> +#include <wdt.h>
>>> +#include <linux/bitops.h>
>>> +
>>> +#define TIMEOUT_MAX    3360
>>> +#define TIMEOUT_MIN    500
>>
>> Those 2 defines above are not used in this source. Please drop.
>>
> ok
>>> +
>>> +#define DIVIDER_5XXX    4096
>>> +#define DIVIDER_5282    8192
>>> +
>>> +#define WCR_EN          BIT(0)
>>> +#define WCR_HALTED      BIT(1)
>>> +#define WCR_DOZE        BIT(2)
>>> +#define WCR_WAIT        BIT(3)
>>> +
>>> +struct watchdog_regs {
>>> +    u16 wcr;        /* Control */
>>> +    u16 wmr;        /* Service */
>>> +    u16 wcntr;      /* Counter */
>>> +    u16 wsr;        /* Reset Status */
>>> +};
>>> +
>>> +#if defined(CONFIG_WDT_MCF)
>>
>> Is it possible to drop this "#if" somehow?
>>
> will try
>>> +static void mcf_watchdog_reset(struct watchdog_regs *wdog)
>>> +{
>>> +#ifndef CONFIG_WATCHDOG_RESET_DISABLE
>>> +    writew(0x5555, &wdog->wsr);
>>> +    writew(0xaaaa, &wdog->wsr);
>>> +#endif /* CONFIG_WATCHDOG_RESET_DISABLE*/
>>
>> checkpatch.pl warning:
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' 
>> where possible
>> #37: FILE: drivers/watchdog/mcf_wdt.c:37:
>> +#ifndef CONFIG_WATCHDOG_RESET_DISABLE
>>
> ok will try to use if (IS_ENABLED
> 
>   +}
>>> +
>>> +static void mcf_watchdog_init(struct watchdog_regs *wdog, u32 
>>> fixed_divider,
>>> +                  u64 timeout_msecs)
>>> +{
>>> +    u32 wdog_module;
>>> +
>>> +    /*
>>> +     * The timer watchdog can be set between
>>> +     * 0.5 and 128 Seconds.
>>> +     */
>>> +
>>> +    /* set timeout and enable watchdog */
>>> +    wdog_module = ((CFG_SYS_CLK / 1000) * timeout_msecs);
>>> +
>>> +    writew((u16)(wdog_module / fixed_divider), &wdog->wmr);
>>> +    writew(WCR_EN, &wdog->wcr);
>>> +
>>> +    mcf_watchdog_reset(wdog);
>>> +}
>>> +
>>> +#if !CONFIG_IS_ENABLED(WDT)
>>> +void hw_watchdog_reset(void)
>>> +{
>>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
>>> +
>>> +    mcf_watchdog_reset(wdog);
>>> +}
>>
>> Can you perhaps completely remove this hw_watchdog stuff? Is it really
>> needed on these Coldfire platforms?
>>
> Will recheck, maybe yes,

Perfect. That would be great.

Thanks,
Stefan

>>> +
>>> +void hw_watchdog_init(void)
>>> +{
>>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)MMAP_WDOG;
>>> +
>>> +    if (IS_ENABLED(CONFIG_MCF5282))
>>> +        writew(&wdog->wmr, wdog_module / DIVIDER_5282);
>>> +    else
>>> +        writew(&wdog->wmr, wdog_module / DIVIDER_5XXX);
>>> +
>>> +    mcf_watchdog_init(wdog, CONFIG_WATCHDOG_TIMEOUT_MSECS);
>>> +}
>>> +
>>> +#else /* CONFIG_WDT */
>>> +
>>> +struct mcf_wdt_priv {
>>> +    void __iomem *base;
>>> +    u32 fixed_divider;
>>> +};
>>> +
>>> +static int mcf_wdt_expire_now(struct udevice *dev, ulong flags)
>>> +{
>>> +    hang();
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int mcf_wdt_reset(struct udevice *dev)
>>> +{
>>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>>> +
>>> +    mcf_watchdog_reset(priv->base);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int mcf_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>>> +{
>>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>>> +
>>> +    /* Timeout from fdt is in second, driver works in msecs */
>>
>> I'm a bit confused here. AFAIK, the time passed into the start()
>> function is in ms:
>>
>> /*
>>   * Start the timer
>>   *
>>   * @dev: WDT Device
>>   * @timeout_ms: Number of ticks (milliseconds) before timer expires
>>   * @flags: Driver specific flags. This might be used to specify
>>   * which action needs to be executed when the timer expires
>>   * @return: 0 if OK, -ve on error
>>   */
>> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags);
>>
>> So there should be no need to convert to ms here in this
>> function. Please double-check here again.
>>
> Thanks, will check.
> 
>> Thanks,
>> Stefan
>>
> 
> Regards,
> angelo
> 
>>> +    mcf_watchdog_init(priv->base, priv->fixed_divider,
>>> +              timeout * 1000);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int mcf_wdt_stop(struct udevice *dev)
>>> +{
>>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>>> +    struct watchdog_regs *wdog = (struct watchdog_regs *)priv->base;
>>> +
>>> +    setbits_be16(&wdog->wcr, WCR_HALTED);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int mcf_wdt_probe(struct udevice *dev)
>>> +{
>>> +    struct mcf_wdt_priv *priv = dev_get_priv(dev);
>>> +
>>> +    priv->base = dev_read_addr_ptr(dev);
>>> +    if (!priv->base)
>>> +        return -ENOENT;
>>> +
>>> +    priv->fixed_divider = (u32)dev_get_driver_data(dev);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct wdt_ops mcf_wdt_ops = {
>>> +    .start        = mcf_wdt_start,
>>> +    .stop        = mcf_wdt_stop,
>>> +    .reset        = mcf_wdt_reset,
>>> +    .expire_now    = mcf_wdt_expire_now,
>>> +};
>>> +
>>> +static const struct udevice_id mcf_wdt_ids[] = {
>>> +    { .compatible = "fsl,mcf5208-wdt", .data = DIVIDER_5XXX },
>>> +    { .compatible = "fsl,mcf5282-wdt", .data = DIVIDER_5282 },
>>> +    {}
>>> +};
>>> +
>>> +U_BOOT_DRIVER(mcf_wdt) = {
>>> +    .name        = "mcf_wdt",
>>> +    .id        = UCLASS_WDT,
>>> +    .of_match    = mcf_wdt_ids,
>>> +    .probe        = mcf_wdt_probe,
>>> +    .ops        = &mcf_wdt_ops,
>>> +    .priv_auto    = sizeof(struct mcf_wdt_priv),
>>> +    .flags        = DM_FLAG_PRE_RELOC,
>>> +};
>>> +#endif
>>> +#endif
>>
>>

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de