[PATCH] kvx: watchdog: Add early watchdog init

[PATCH] kvx: watchdog: Add early watchdog init

[Julian Vetter]

From: Jules Maselbas <jmaselbas@kalray.eu>

Add support for a watchdog that starts as early as possible in barebox.
So, if there is an issue the board is reset properly.

Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
---
 drivers/watchdog/Kconfig   | 13 +++++++++++++
 drivers/watchdog/kvx_wdt.c | 16 ++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 762e37c9c2..b1924b5b05 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -89,6 +89,19 @@ config WATCHDOG_KVX
 	help
 	  Add support for the KVX core watchdog.
 
+if WATCHDOG_KVX
+config WATCHDOG_KVX_EARLY_INIT
+	bool "KVX Core watchdog early init"
+	depends on WATCHDOG_KVX
+	help
+	  Activate the watchdog with early initcall
+
+config WATCHDOG_KVX_EARLY_TIMEOUT
+	int "KVX Core watchdog early timeout in cycles"
+	depends on WATCHDOG_KVX_EARLY_INIT
+	default 100000000
+endif
+
 config WATCHDOG_BCM2835
 	bool "Watchdog for BCM283x SoCs"
 	depends on ARCH_BCM283X || COMPILE_TEST
diff --git a/drivers/watchdog/kvx_wdt.c b/drivers/watchdog/kvx_wdt.c
index be6b08b71c..3e66aa38ba 100644
--- a/drivers/watchdog/kvx_wdt.c
+++ b/drivers/watchdog/kvx_wdt.c
@@ -91,3 +91,19 @@ static struct driver kvx_wdt_driver = {
 	.of_compatible	= DRV_OF_COMPAT(kvx_wdt_of_match),
 };
 device_platform_driver(kvx_wdt_driver);
+
+#ifdef CONFIG_WATCHDOG_KVX_EARLY_INIT
+static int kvx_wdt_early_init(void)
+{
+	/* Set Start watchdog counting */
+	kvx_sfr_set(WDV, CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT);
+	kvx_sfr_set(WDR, 0);
+
+	/* Start watchdog counting */
+	kvx_sfr_set_field(TCR, WUI, 1);
+	kvx_sfr_set_field(TCR, WCE, 1);
+
+	return 0;
+}
+core_initcall(kvx_wdt_early_init);
+#endif
-- 
2.34.1

Re: [PATCH] kvx: watchdog: Add early watchdog init

[Ahmad Fatoum]

Hello Julian,

On 23.10.24 10:55, Julian Vetter wrote:
> From: Jules Maselbas <jmaselbas@kalray.eu>
> 
> Add support for a watchdog that starts as early as possible in barebox.
> So, if there is an issue the board is reset properly.
> > Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
> ---
>  drivers/watchdog/Kconfig   | 13 +++++++++++++
>  drivers/watchdog/kvx_wdt.c | 16 ++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 762e37c9c2..b1924b5b05 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -89,6 +89,19 @@ config WATCHDOG_KVX
>  	help
>  	  Add support for the KVX core watchdog.
>  
> +if WATCHDOG_KVX

`if' and `depends on' are equivalent, so just drop this line alongside
the endif.

> +config WATCHDOG_KVX_EARLY_INIT
> +	bool "KVX Core watchdog early init"
> +	depends on WATCHDOG_KVX
> +	help
> +	  Activate the watchdog with early initcall
> +
> +config WATCHDOG_KVX_EARLY_TIMEOUT
> +	int "KVX Core watchdog early timeout in cycles"
> +	depends on WATCHDOG_KVX_EARLY_INIT
> +	default 100000000
> +endif
> +
>  config WATCHDOG_BCM2835
>  	bool "Watchdog for BCM283x SoCs"
>  	depends on ARCH_BCM283X || COMPILE_TEST
> diff --git a/drivers/watchdog/kvx_wdt.c b/drivers/watchdog/kvx_wdt.c
> index be6b08b71c..3e66aa38ba 100644
> --- a/drivers/watchdog/kvx_wdt.c
> +++ b/drivers/watchdog/kvx_wdt.c
> @@ -91,3 +91,19 @@ static struct driver kvx_wdt_driver = {
>  	.of_compatible	= DRV_OF_COMPAT(kvx_wdt_of_match),
>  };
>  device_platform_driver(kvx_wdt_driver);
> +
> +#ifdef CONFIG_WATCHDOG_KVX_EARLY_INIT

Move this down before the core_initcall

> +static int kvx_wdt_early_init(void)

Add __maybe_unused here.

That way compile test in CI tests this function even if option is disabled.

> +{
> +	/* Set Start watchdog counting */
> +	kvx_sfr_set(WDV, CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT);
> +	kvx_sfr_set(WDR, 0);
> +
> +	/* Start watchdog counting */
> +	kvx_sfr_set_field(TCR, WUI, 1);
> +	kvx_sfr_set_field(TCR, WCE, 1);
> +
> +	return 0;
> +}
> +core_initcall(kvx_wdt_early_init);
> +#endif


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] kvx: watchdog: Add early watchdog init

[Julian Vetter]

Hello Ahmad,

On 10/23/24 11:25, Ahmad Fatoum wrote:
> Hello Julian,
> 
> On 23.10.24 10:55, Julian Vetter wrote:
>> From: Jules Maselbas <jmaselbas@kalray.eu>
>>
>> Add support for a watchdog that starts as early as possible in barebox.
>> So, if there is an issue the board is reset properly.
>>> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
>> ---
>>   drivers/watchdog/Kconfig   | 13 +++++++++++++
>>   drivers/watchdog/kvx_wdt.c | 16 ++++++++++++++++
>>   2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 762e37c9c2..b1924b5b05 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -89,6 +89,19 @@ config WATCHDOG_KVX
>>   	help
>>   	  Add support for the KVX core watchdog.
>>   
>> +if WATCHDOG_KVX
> 
> `if' and `depends on' are equivalent, so just drop this line alongside
> the endif.

Yes, thanks. Fixed.

> 
>> +config WATCHDOG_KVX_EARLY_INIT
>> +	bool "KVX Core watchdog early init"
>> +	depends on WATCHDOG_KVX
>> +	help
>> +	  Activate the watchdog with early initcall
>> +
>> +config WATCHDOG_KVX_EARLY_TIMEOUT
>> +	int "KVX Core watchdog early timeout in cycles"
>> +	depends on WATCHDOG_KVX_EARLY_INIT
>> +	default 100000000
>> +endif
>> +
>>   config WATCHDOG_BCM2835
>>   	bool "Watchdog for BCM283x SoCs"
>>   	depends on ARCH_BCM283X || COMPILE_TEST
>> diff --git a/drivers/watchdog/kvx_wdt.c b/drivers/watchdog/kvx_wdt.c
>> index be6b08b71c..3e66aa38ba 100644
>> --- a/drivers/watchdog/kvx_wdt.c
>> +++ b/drivers/watchdog/kvx_wdt.c
>> @@ -91,3 +91,19 @@ static struct driver kvx_wdt_driver = {
>>   	.of_compatible	= DRV_OF_COMPAT(kvx_wdt_of_match),
>>   };
>>   device_platform_driver(kvx_wdt_driver);
>> +
>> +#ifdef CONFIG_WATCHDOG_KVX_EARLY_INIT
> 
> Move this down before the core_initcall
> 
>> +static int kvx_wdt_early_init(void)
> 
> Add __maybe_unused here.
> 
> That way compile test in CI tests this function even if option is disabled.
> 

I have tried what you proposed but the CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT 
is only defined if we enable the CONFIG_WATCHDOG_KVX_EARLY_INIT. So, it 
fails if we have the watchdog enabled but not the EARLY_INIT. I could 
either keep the #ifdef before the function. So, it will not be covered 
by compile tests or I add something like the following to the function:

int wtd_timeout = 0;

#ifdef CONFIG_WATCHDOG_KVX_EARLY_INIT
wtd_timeout = CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT
#endif

What do you think?

Thank you.

>> +{
>> +	/* Set Start watchdog counting */
>> +	kvx_sfr_set(WDV, CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT);
>> +	kvx_sfr_set(WDR, 0);
>> +
>> +	/* Start watchdog counting */
>> +	kvx_sfr_set_field(TCR, WUI, 1);
>> +	kvx_sfr_set_field(TCR, WCE, 1);
>> +
>> +	return 0;
>> +}
>> +core_initcall(kvx_wdt_early_init);
>> +#endif
> 
> 

Re: [PATCH] kvx: watchdog: Add early watchdog init

[Ahmad Fatoum]

On 24.10.24 14:35, Julian Vetter wrote:
fdef CONFIG_WATCHDOG_KVX_EARLY_INIT>>
>> Move this down before the core_initcall
>>
>>> +static int kvx_wdt_early_init(void)
>>
>> Add __maybe_unused here.
>>
>> That way compile test in CI tests this function even if option is disabled.
>>
> 
> I have tried what you proposed but the CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT is only defined if we enable the CONFIG_WATCHDOG_KVX_EARLY_INIT. So, it fails if we have the watchdog enabled but not the EARLY_INIT. I could either keep the #ifdef before the function. So, it will not be covered by compile tests or I add something like the following to the function:
> 
> int wtd_timeout = 0;
> 
> #ifdef CONFIG_WATCHDOG_KVX_EARLY_INIT
> wtd_timeout = CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT
> #endif
> 
> What do you think?

I missed the undefined macro. Never mind then, I think
we can leave it as is.

Cheers,
Ahmad

> 
> Thank you.
> 
>>> +{
>>> +    /* Set Start watchdog counting */
>>> +    kvx_sfr_set(WDV, CONFIG_WATCHDOG_KVX_EARLY_TIMEOUT);
>>> +    kvx_sfr_set(WDR, 0);
>>> +
>>> +    /* Start watchdog counting */
>>> +    kvx_sfr_set_field(TCR, WUI, 1);
>>> +    kvx_sfr_set_field(TCR, WCE, 1);
>>> +
>>> +    return 0;
>>> +}
>>> +core_initcall(kvx_wdt_early_init);
>>> +#endif
>>
>>
> 
> 
> 
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |