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

Hi Samuel,

On Sun, 22 Aug 2021 at 14:41, Samuel Holland samuel@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@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@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