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

[Angelo Dureghello]

Hi Stefan,

On 24/07/23 8:21 AM, Stefan Roese wrote:
> 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.
> 
ops, thanks for catching it, i had a reflog reset at a certain point,
and this test left in.

Fixing.

> 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
> 


Regards,
angelo