[PATCH 4/4] watchdog: Automatically register device with sysreset

[Simon Glass]

Hi Samuel,

On Sun, 22 Aug 2021 at 14:41, Samuel Holland <samuel at sholland.org> wrote:
>
> Add an option to automatically register the first watchdog device with
> the wdt_reboot driver for use with sysreset. This allows sysreset to be
> a drop-in replacement for platform-specific watchdog reset code, without
> needing any device tree changes.
>
> Signed-off-by: Samuel Holland <samuel at sholland.org>
> ---
>
>  drivers/sysreset/Kconfig             |  7 +++++++
>  drivers/sysreset/sysreset_watchdog.c | 24 ++++++++++++++++++++++++
>  drivers/watchdog/wdt-uclass.c        |  5 +++++
>  include/sysreset.h                   | 14 ++++++++++++++
>  4 files changed, 50 insertions(+)

This is a bit odd, but I suppose it is OK.

Reviewed-by: Simon Glass <sjg at chromium.org>

>
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index fdc858ccbac..49354b1b27a 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -119,6 +119,13 @@ config SYSRESET_WATCHDOG
>         help
>           Reboot support for generic watchdog reset.
>
> +config SYSRESET_WATCHDOG_AUTO
> +       bool "Automatically register first watchdog with sysreset"
> +       depends on SYSRESET_WATCHDOG
> +       help
> +         If enabled, the first watchdog (as selected by the watchdog uclass)
> +         will automatically be registered with the watchdog reboot driver.
> +
>  config SYSRESET_RESETCTL
>         bool "Enable support for reset controller reboot driver"
>         select DM_RESET
> diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c
> index b723f5647cd..35efcac59dd 100644
> --- a/drivers/sysreset/sysreset_watchdog.c
> +++ b/drivers/sysreset/sysreset_watchdog.c
> @@ -5,7 +5,9 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <dm/device-internal.h>
>  #include <errno.h>
> +#include <malloc.h>
>  #include <sysreset.h>
>  #include <wdt.h>
>
> @@ -57,3 +59,25 @@ U_BOOT_DRIVER(wdt_reboot) = {
>         .plat_auto      = sizeof(struct wdt_reboot_plat),
>         .ops = &wdt_reboot_ops,
>  };
> +
> +#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
> +int sysreset_register_wdt(struct udevice *dev)

Could you pass in plat here instead of allocating it? You could add it
to wdt-uclass.c as some private data for the uclass...

> +{
> +       struct wdt_reboot_plat *plat = malloc(sizeof(*plat));
> +       int ret;
> +
> +       if (!plat)
> +               return -ENOMEM;
> +
> +       plat->wdt = dev;
> +
> +       ret = device_bind(dev, DM_DRIVER_GET(wdt_reboot),
> +                         dev->name, plat, ofnode_null(), NULL);
> +       if (ret) {
> +               free(plat);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +#endif
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 17334dbda6c..3170ef9d945 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -10,6 +10,7 @@
>  #include <errno.h>
>  #include <hang.h>
>  #include <log.h>
> +#include <sysreset.h>
>  #include <time.h>
>  #include <wdt.h>
>  #include <asm/global_data.h>
> @@ -53,6 +54,10 @@ int initr_watchdog(void)
>                                                     4 * reset_period) / 4;
>         }
>
> +       ret = sysreset_register_wdt(gd->watchdog_dev);
> +       if (ret)
> +               printf("WDT:   Failed to register sysreset\n");
> +
>         if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
>                 printf("WDT:   Not starting\n");
>                 return 0;
> diff --git a/include/sysreset.h b/include/sysreset.h
> index 701e4f5c86e..ad45fe0db28 100644
> --- a/include/sysreset.h
> +++ b/include/sysreset.h
> @@ -118,4 +118,18 @@ void sysreset_walk_halt(enum sysreset_t type);
>   */
>  void reset_cpu(void);
>
> +/**
> + * sysreset_register_wdt() - register a watchdog for use with sysreset
> + *
> + * This registers the given watchdog timer to be used to reset the system.
> + *
> + * @dev:       WDT device
> + * @return:    0 if OK, -errno if error
> + */
> +#if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO)
> +int sysreset_register_wdt(struct udevice *dev);
> +#else
> +static inline int sysreset_register_wdt(struct udevice *dev) { return 0; }
> +#endif
> +
>  #endif
> --
> 2.31.1
>

Regards,
Simon