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

[Stefan Roese]

Hi Angelo,

On 7/23/23 23: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 at kernel-space.org>
> ---
> Changes for v2:
> - remove unnecessary hardcoded timeouts
> - remove unnecessary hw_watchdog_xxx stuff

Perfect, thanks!

> - rewrite wdog module reg calculation
> - using IS_ENABLED() where possible
> ---
>   drivers/watchdog/Kconfig   |   7 ++
>   drivers/watchdog/Makefile  |   1 +
>   drivers/watchdog/mcf_wdt.c | 137 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 145 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..cad6097a7d
> --- /dev/null
> +++ b/drivers/watchdog/mcf_wdt.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mcf_wdt.c - driver for ColdFire on-chip watchdog
> + *
> + * Author: Angelo Dureghello <angelo at kernel-space.org>
> + *
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <hang.h>
> +#include <asm/io.h>
> +#include <wdt.h>
> +#include <linux/bitops.h>
> +
> +#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 */
> +};
> +
> +static void mcf_watchdog_reset(struct watchdog_regs *wdog)
> +{
> +	if (!IS_ENABLED(CONFIG_WATCHDOG_RESET_DISABLE)) {
> +		writew(0x5555, &wdog->wsr);
> +		writew(0xaaaa, &wdog->wsr);
> +	}
> +}
> +
> +static void mcf_watchdog_init(struct watchdog_regs *wdog, u32 fixed_divider,
> +			      u64 timeout_msecs)
> +{
> +	u32 wdog_module, cycles_per_sec;
> +
> +	cycles_per_sec = CFG_SYS_CLK / fixed_divider;
> +
> +	wdog_module = cycles_per_sec * ((u32)timeout_msecs / 1000);
> +	wdog_module += (cycles_per_sec / 1000) * ((u32)timeout_msecs % 1000);
> +
> +	/* Limit check, max 16 bits */
> +	if (wdog_module > 0xffff)
> +		wdog_module = 0xffff;
> +
> +	wdog_module = 0x8000;

Hmmm, all the calculations are overwritten here. This does not seem to
be correct. Please check again.

Thanks,
Stefan

> +
> +	/* Set timeout and enable watchdog */
> +	writew((u16)wdog_module, &wdog->wmr);
> +	writew(WCR_EN, &wdog->wcr);
> +
> +	mcf_watchdog_reset(wdog);
> +}
> +
> +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 (timeout) comes in milliseconds */
> +	mcf_watchdog_init(priv->base, priv->fixed_divider, timeout);
> +
> +	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,
> +};

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 at denx.de