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