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

[Stefan Roese]

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 at 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