Re: [PATCH 3/4] watchdog: Allow to use CONFIG_WDT without starting watchdog

On Tuesday 09 March 2021 17:12:40 Stefan Roese wrote:

On 09.03.21 14:27, Pali Rohár wrote:

...

On Tuesday 09 March 2021 12:33:24 Stefan Roese wrote:

...

On 05.03.21 22:36, Pali Rohár wrote:

...

In some cases it is useful to compile support for U-Boot command 'wdt' without starting HW watchdog in early U-Boot phase. For example when user

Nitpicking: when "the" user

...

want to start watchdog only on demand by some boot script.

This change adds a new compile option WATCHDOG_AUTOSTART to control whether U-Boot should automatically start watchdog during init phase or not.

start "the" watchdog

...

This option is enabled by default as it was was default behavior prior

as it was the default

...

introducing this new change. When compiling U-Boot users can decide to turn this option off.

Signed-off-by: Pali Rohár pali@kernel.org

drivers/watchdog/Kconfig | 13 +++++++++++++ drivers/watchdog/wdt-uclass.c | 7 +++++++ 2 files changed, 20 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 602ccbe41c00..aa76a8f2d239 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -9,6 +9,19 @@ config WATCHDOG this option if you want to service enabled watchdog by U-Boot. Disable this option if you want U-Boot to start watchdog but never service it. +config WATCHDOG_AUTOSTART

• bool "Automatically start watchdog timer"
• depends on WDT
• default y
• help

•  Automatically start watchdog timer and start servicing it during
  

•  init phase. Enabled by default. Disable this option if you want
  

•  to compile U-Boot with CONFIG_WDT support but do not want to
  

•  activate watchdog, like when CONFIG_WDT option is disabled. You
  

•  would be able to start watchdog manually by 'wdt' command. Useful
  

•  when you want to have support for 'wdt' command but do not want
  

•  to have watchdog enabled by default.
  

• config WATCHDOG_TIMEOUT_MSECS int "Watchdog timeout in msec" default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 7500b3ed90e3..00a408bf5cc5 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -27,7 +27,9 @@ static ulong reset_period = 1000; int initr_watchdog(void) { u32 timeout = WATCHDOG_TIMEOUT_SECS; +#ifdef CONFIG_WATCHDOG_AUTOSTART int ret; +#endif

Please don't add more #ifdef's if possible, see below...

...

/*
 * Init watchdog: This will call the probe function of the

@@ -51,6 +53,10 @@ int initr_watchdog(void) 4 * reset_period) / 4; } +#ifndef CONFIG_WATCHDOG_AUTOSTART

• printf("WDT: Not starting\n");
• return 0;

+#else ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); if (ret != 0) { printf("WDT: Failed to start\n"); @@ -61,6 +67,7 @@ int initr_watchdog(void) IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); return 0; +#endif

Please use this instead here:

if (CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) { ...

Thanks, Stefan

...

} int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)

Viele Grüße, Stefan

I have fixed these issues in V2.

Thanks.

Could you please next time when sending an updated patch series / version include the tags, like my RB tag? As patchwork collects these automatically and now I need to re-send them again - which I will do this time. ;)

Thanks, Stefan

Ok, no problem!