[PATCH 1/4 v5] watchdog: Implement generic watchdog_reset() version

[Stefan Roese]

Hi Christophe

On 19.02.20 20:21, Christophe Leroy wrote:
> 
> 
> Le 25/04/2019 à 09:17, Stefan Roese a écrit :
>> This patch tries to implement a generic watchdog_reset() function that
>> can be used by all boards that want to service the watchdog device in
>> U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG.
>>
>> Without this approach, new boards or platforms needed to implement a
>> board specific version of this functionality, mostly copy'ing the same
>> code over and over again into their board or platforms code base.
>>
>> With this new generic function, the scattered other functions are now
>> removed to be replaced by the generic one. The new version also enables
>> the configuration of the watchdog timeout via the DT "timeout-sec"
>> property (if enabled via CONFIG_OF_CONTROL).
>>
>> This patch also adds a new flag to the GD flags, to flag that the
>> watchdog is ready to use and adds the pointer to the watchdog device
>> to the GD. This enables us to remove the global "watchdog_dev"
>> variable, which was prone to cause problems because of its potentially
>> very early use in watchdog_reset(), even before the BSS is cleared.
>>
>> Signed-off-by: Stefan Roese <sr at denx.de>
>> Cc: Heiko Schocher <hs at denx.de>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: Michal Simek <michal.simek at xilinx.com>
>> Cc: "Marek Behún" <marek.behun at nic.cz>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>> Cc: Maxim Sloyko <maxims at google.com>
>> Cc: Erik van Luijk <evanluijk at interact.nl>
>> Cc: Ryder Lee <ryder.lee at mediatek.com>
>> Cc: Weijie Gao <weijie.gao at mediatek.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: "Álvaro Fernández Rojas" <noltari at gmail.com>
>> Cc: Philippe Reynes <philippe.reynes at softathome.com>
>> Cc: Christophe Leroy <christophe.leroy at c-s.fr>
>> Cc: Chris Packham <chris.packham at alliedtelesis.co.nz>
>> Reviewed-by: Michal Simek <michal.simek at xilinx.com>
>> Tested-by: Michal Simek <michal.simek at xilinx.com> (on zcu100)
>> ---
> 
> [...]
> 
>> diff --git a/drivers/watchdog/wdt-uclass.c 
>> b/drivers/watchdog/wdt-uclass.c
>> index 23b7e3360d..bbfac4f0f9 100644
>> --- a/drivers/watchdog/wdt-uclass.c
>> +++ b/drivers/watchdog/wdt-uclass.c
>> @@ -10,6 +10,8 @@
>>   #include <dm/device-internal.h>
>>   #include <dm/lists.h>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>>   int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>>   {
>>       const struct wdt_ops *ops = device_get_ops(dev);
>> @@ -63,6 +65,30 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
>>       return ret;
>>   }
>> +#if defined(CONFIG_WATCHDOG)
>> +/*
>> + * Called by macro WATCHDOG_RESET. This function be called *very* early,
>> + * so we need to make sure, that the watchdog driver is ready before 
>> using
>> + * it in this function.
>> + */
>> +void watchdog_reset(void)
>> +{
>> +    static ulong next_reset;
>> +    ulong now;
>> +
>> +    /* Exit if GD is not ready or watchdog is not initialized yet */
>> +    if (!gd || !(gd->flags & GD_FLG_WDT_READY))
>> +        return;
>> +
>> +    /* Do not reset the watchdog too often */
>> +    now = get_timer(0);
>> +    if (now > next_reset) {
>> +        next_reset = now + 1000;    /* reset every 1000ms */
>> +        wdt_reset(gd->watchdog_dev);
>> +    }
> 
> This is a problem for the MPC8xx.
> 
> When running with a MPC8xx at 132MHz clock, the watchdog will fire about 
> 1s after the last refresh. So the above makes the board unusable.

So you need a shorted delay between the wdt_reset() calls? Is this
correct? We could introduce a new Kconfig option which defaults to
1000 (ms) and you can "select" a shorter value for MPC8xx.

Would this work for you?

> Is that really necessary to restrict the refresh like this in the core 
> part of Watchdog driver ?

It seems to work for most cases (all until I hear of this MPC8xx issue).

Thanks,
Stefan