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

[Heinrich Schuchardt]

On 10/29/21 05:16, Samuel Holland wrote:
> Add an option to automatically register watchdog devices 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>
> ---
> 
> Changes in v2:
>   - Rebase on top of 492ee6b8d0e7 (now handle all watchdogs).
> 
>   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(+)
> 
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index de75c9cccc..f6d60038b8 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -131,6 +131,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 b723f5647c..35efcac59d 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)
> +{
> +	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 7570710c4d..985f114084 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>
> @@ -44,6 +45,10 @@ static void init_watchdog_dev(struct udevice *dev)
>   
>   	priv = dev_get_uclass_priv(dev);
>   
> +	ret = sysreset_register_wdt(dev);
> +	if (ret)
> +		printf("WDT:   Failed to register sysreset\n");
> +
>   	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
>   		printf("WDT:   Not starting %s\n", dev->name);
>   		return;
> diff --git a/include/sysreset.h b/include/sysreset.h
> index 9d4ed87cea..d0e63e6d2b 100644
> --- a/include/sysreset.h
> +++ b/include/sysreset.h
> @@ -133,4 +133,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
> 

Running Gitlab CI for the complete series
https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/346194#L106

    sandbox:  +   sandbox_spl
+In file included from arch/sandbox/include/asm/state.h:10,
+                 from include/test/test.h:130,
+                 from include/test/lib.h:9,
+                 from test/cmd/test_echo.c:12:
+include/sysreset.h: In function ‘arch_reset_for_test’:
+include/sysreset.h:147:19: error: invalid storage class for function 
‘sysreset_register_wdt’
+  147 | static inline int sysreset_register_wdt(struct udevice *dev) { 
return 0; }
+      |                   ^~~~~~~~~~~~~~~~~~~~~
+                 from include/test/common.h:10,
+                 from test/common/cmd_ut_common.c:11:

Please, remove the static inline version of sysreset_register_wdt() and 
use an if clause in drivers/watchdog/wdt-uclass.c instead:

+       if IS_ENABLED(CONFIG_SYSRESET_WATCHDOG_AUTO) {
+               ret = sysreset_register_wdt(dev);
+               if (ret)
+                       printf("WDT:   Failed to register sysreset\n");
+       }

Best regards

Heinrich