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

[Christophe Leroy]

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.

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

> +}
> +#endif
> +
>   static int wdt_post_bind(struct udevice *dev)
>   {
>   #if defined(CONFIG_NEEDS_MANUAL_RELOC)

Thanks,
Christophe