[PATCH 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[PATCH 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[Pali Rohár]

Watchdog is ready after successful call of ops->start() callback in
wdt_start() function. And is stopped after successful call of ops->stop()
callback in wdt_stop function.

So move setting of GD_FLG_WDT_READY flag from initr_watchdog() function to
wdt_start() and ensure that GD_FLG_WDT_READY flag is unset in wdt_stop()
function.

This change ensures that GD_FLG_WDT_READY flag is set only when watchdog is
running. And ensures that flag is also also when watchdog was started not
only by initr_watchdog() call (e.g. by U-Boot 'wdt' command).

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/watchdog/wdt-uclass.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 28f7918c4673..3f707f61f74f 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -51,7 +51,6 @@ int initr_watchdog(void)
 	}
 
 	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
-	gd->flags |= GD_FLG_WDT_READY;
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
 	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
 
@@ -61,21 +60,31 @@ int initr_watchdog(void)
 int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
+	int ret;
 
 	if (!ops->start)
 		return -ENOSYS;
 
-	return ops->start(dev, timeout_ms, flags);
+	ret = ops->start(dev, timeout_ms, flags);
+	if (ret == 0)
+		gd->flags |= GD_FLG_WDT_READY;
+
+	return ret;
 }
 
 int wdt_stop(struct udevice *dev)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
+	int ret;
 
 	if (!ops->stop)
 		return -ENOSYS;
 
-	return ops->stop(dev);
+	ret = ops->stop(dev);
+	if (ret == 0)
+		gd->flags &= ~GD_FLG_WDT_READY;
+
+	return ret;
 }
 
 int wdt_reset(struct udevice *dev)
-- 
2.20.1

[PATCH 2/4] watchdog: Show error message when initr_watchdog() cannot start watchdog

[Pali Rohár]

Function wdt_start() may fail. So in initr_watchdog() function check return
value of wdt_start() call and print error message when watchdog starting
failed.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/watchdog/wdt-uclass.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 3f707f61f74f..7500b3ed90e3 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -27,6 +27,7 @@ static ulong reset_period = 1000;
 int initr_watchdog(void)
 {
 	u32 timeout = WATCHDOG_TIMEOUT_SECS;
+	int ret;
 
 	/*
 	 * Init watchdog: This will call the probe function of the
@@ -50,7 +51,12 @@ int initr_watchdog(void)
 						    4 * reset_period) / 4;
 	}
 
-	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+	if (ret != 0) {
+		printf("WDT:   Failed to start\n");
+		return 0;
+	}
+
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
 	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
 
-- 
2.20.1

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

[Pali Rohár]

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

This option is enabled by default as it was was default behavior prior
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
 
 	/*
 	 * 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
 }
 
 int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
-- 
2.20.1

[PATCH 4/4] arm: mvebu: Espressobin: Enable watchdog support but do not start it

[Pali Rohár]

Espressobin had disabled watchdog support (CONFIG_WDT) because older stable
Linux kernel versions (which are used by current stable OpenWRT and Debian
versions) do not have support for Armada 3700 watchdog driver. Therefore
they are not able to periodically kick watchdog so Espressobin enter into
boot loop.

This change enable CONFIG_WDT, CONFIG_WDT_ARMADA_37XX and CONFIG_CMD_WDT
options which add support for U-Boot 'wdt' command. And unset new
CONFIG_WATCHDOG_AUTOSTART option which cause that watchdog is not
automatically started by U-Boot during init phase, like when CONFIG_WDT
option is not set at all.

So with this change, U-Boot on Espressobin would have working 'wdt' command
which can be used from boot scripts (e.g. for enabling watchdog prior new
Linux booting kernel). But default behavior of watchdog status stays
unchanged, U-Boot does not start watchdog on Espressobin during its init
phase.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 configs/mvebu_espressobin-88f3720_defconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index c791f98dfea5..20aa21b06b3d 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -32,6 +32,7 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
+CONFIG_CMD_WDT=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
@@ -88,5 +89,8 @@ CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_MCS7830=y
 CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
+# CONFIG_WATCHDOG_AUTOSTART is not set
+CONFIG_WDT=y
+CONFIG_WDT_ARMADA_37XX=y
 CONFIG_SHA1=y
 CONFIG_SHA256=y
-- 
2.20.1

[PATCH 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[Stefan Roese]

On 05.03.21 22:36, Pali Roh?r wrote:
> Watchdog is ready after successful call of ops->start() callback in
> wdt_start() function. And is stopped after successful call of ops->stop()
> callback in wdt_stop function.
> 
> So move setting of GD_FLG_WDT_READY flag from initr_watchdog() function to
> wdt_start() and ensure that GD_FLG_WDT_READY flag is unset in wdt_stop()
> function.
> 
> This change ensures that GD_FLG_WDT_READY flag is set only when watchdog is
> running. And ensures that flag is also also when watchdog was started not
> only by initr_watchdog() call (e.g. by U-Boot 'wdt' command).
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/watchdog/wdt-uclass.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 28f7918c4673..3f707f61f74f 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -51,7 +51,6 @@ int initr_watchdog(void)
>   	}
>   
>   	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> -	gd->flags |= GD_FLG_WDT_READY;
>   	printf("WDT:   Started with%s servicing (%ds timeout)\n",
>   	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
>   
> @@ -61,21 +60,31 @@ int initr_watchdog(void)
>   int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->start)
>   		return -ENOSYS;
>   
> -	return ops->start(dev, timeout_ms, flags);
> +	ret = ops->start(dev, timeout_ms, flags);
> +	if (ret == 0)
> +		gd->flags |= GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_stop(struct udevice *dev)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->stop)
>   		return -ENOSYS;
>   
> -	return ops->stop(dev);
> +	ret = ops->stop(dev);
> +	if (ret == 0)
> +		gd->flags &= ~GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_reset(struct udevice *dev)
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/4] watchdog: Show error message when initr_watchdog() cannot start watchdog

[Stefan Roese]

On 05.03.21 22:36, Pali Roh?r wrote:
> Function wdt_start() may fail. So in initr_watchdog() function check return
> value of wdt_start() call and print error message when watchdog starting
> failed.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/watchdog/wdt-uclass.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 3f707f61f74f..7500b3ed90e3 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -27,6 +27,7 @@ static ulong reset_period = 1000;
>   int initr_watchdog(void)
>   {
>   	u32 timeout = WATCHDOG_TIMEOUT_SECS;
> +	int ret;
>   
>   	/*
>   	 * Init watchdog: This will call the probe function of the
> @@ -50,7 +51,12 @@ int initr_watchdog(void)
>   						    4 * reset_period) / 4;
>   	}
>   
> -	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> +	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> +	if (ret != 0) {
> +		printf("WDT:   Failed to start\n");
> +		return 0;
> +	}
> +
>   	printf("WDT:   Started with%s servicing (%ds timeout)\n",
>   	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
>   
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

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

[Stefan Roese]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 4/4] arm: mvebu: Espressobin: Enable watchdog support but do not start it

[Stefan Roese]

On 05.03.21 22:36, Pali Roh?r wrote:
> Espressobin had disabled watchdog support (CONFIG_WDT) because older stable
> Linux kernel versions (which are used by current stable OpenWRT and Debian
> versions) do not have support for Armada 3700 watchdog driver. Therefore
> they are not able to periodically kick watchdog so Espressobin enter into
> boot loop.
> 
> This change enable CONFIG_WDT, CONFIG_WDT_ARMADA_37XX and CONFIG_CMD_WDT
> options which add support for U-Boot 'wdt' command. And unset new
> CONFIG_WATCHDOG_AUTOSTART option which cause that watchdog is not
> automatically started by U-Boot during init phase, like when CONFIG_WDT
> option is not set at all.
> 
> So with this change, U-Boot on Espressobin would have working 'wdt' command
> which can be used from boot scripts (e.g. for enabling watchdog prior new
> Linux booting kernel). But default behavior of watchdog status stays
> unchanged, U-Boot does not start watchdog on Espressobin during its init
> phase.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   configs/mvebu_espressobin-88f3720_defconfig | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index c791f98dfea5..20aa21b06b3d 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -32,6 +32,7 @@ CONFIG_CMD_MMC=y
>   CONFIG_CMD_PCI=y
>   CONFIG_CMD_SPI=y
>   CONFIG_CMD_USB=y
> +CONFIG_CMD_WDT=y
>   CONFIG_CMD_TFTPPUT=y
>   CONFIG_CMD_CACHE=y
>   CONFIG_CMD_TIME=y
> @@ -88,5 +89,8 @@ CONFIG_USB_ETHER_ASIX=y
>   CONFIG_USB_ETHER_MCS7830=y
>   CONFIG_USB_ETHER_RTL8152=y
>   CONFIG_USB_ETHER_SMSC95XX=y
> +# CONFIG_WATCHDOG_AUTOSTART is not set
> +CONFIG_WDT=y
> +CONFIG_WDT_ARMADA_37XX=y
>   CONFIG_SHA1=y
>   CONFIG_SHA256=y
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH v2 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[Pali Rohár]

Watchdog is ready after successful call of ops->start() callback in
wdt_start() function. And is stopped after successful call of ops->stop()
callback in wdt_stop function.

So move setting of GD_FLG_WDT_READY flag from initr_watchdog() function to
wdt_start() and ensure that GD_FLG_WDT_READY flag is unset in wdt_stop()
function.

This change ensures that GD_FLG_WDT_READY flag is set only when watchdog is
running. And ensures that flag is also also when watchdog was started not
only by initr_watchdog() call (e.g. by U-Boot 'wdt' command).

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/watchdog/wdt-uclass.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 28f7918c4673..3f707f61f74f 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -51,7 +51,6 @@ int initr_watchdog(void)
 	}
 
 	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
-	gd->flags |= GD_FLG_WDT_READY;
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
 	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
 
@@ -61,21 +60,31 @@ int initr_watchdog(void)
 int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
+	int ret;
 
 	if (!ops->start)
 		return -ENOSYS;
 
-	return ops->start(dev, timeout_ms, flags);
+	ret = ops->start(dev, timeout_ms, flags);
+	if (ret == 0)
+		gd->flags |= GD_FLG_WDT_READY;
+
+	return ret;
 }
 
 int wdt_stop(struct udevice *dev)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
+	int ret;
 
 	if (!ops->stop)
 		return -ENOSYS;
 
-	return ops->stop(dev);
+	ret = ops->stop(dev);
+	if (ret == 0)
+		gd->flags &= ~GD_FLG_WDT_READY;
+
+	return ret;
 }
 
 int wdt_reset(struct udevice *dev)
-- 
2.20.1

[PATCH v2 2/4] watchdog: Show error message when initr_watchdog() cannot start watchdog

[Pali Rohár]

Function wdt_start() may fail. So in initr_watchdog() function check return
value of wdt_start() call and print error message when watchdog starting
failed.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/watchdog/wdt-uclass.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 3f707f61f74f..7500b3ed90e3 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -27,6 +27,7 @@ static ulong reset_period = 1000;
 int initr_watchdog(void)
 {
 	u32 timeout = WATCHDOG_TIMEOUT_SECS;
+	int ret;
 
 	/*
 	 * Init watchdog: This will call the probe function of the
@@ -50,7 +51,12 @@ int initr_watchdog(void)
 						    4 * reset_period) / 4;
 	}
 
-	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+	if (ret != 0) {
+		printf("WDT:   Failed to start\n");
+		return 0;
+	}
+
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
 	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
 
-- 
2.20.1

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

[Pali Rohár]

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 the
user want to start the 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 the watchdog during init phase or not.

This option is enabled by default as it was the default behavior prior
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 |  5 +++++
 2 files changed, 18 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..0603ffbd36d9 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -51,6 +51,11 @@ int initr_watchdog(void)
 						    4 * reset_period) / 4;
 	}
 
+	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
+		printf("WDT:   Not starting\n");
+		return 0;
+	}
+
 	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
 	if (ret != 0) {
 		printf("WDT:   Failed to start\n");
-- 
2.20.1

[PATCH v2 4/4] arm: mvebu: Espressobin: Enable watchdog support but do not start it

[Pali Rohár]

Espressobin had disabled watchdog support (CONFIG_WDT) because older stable
Linux kernel versions (which are used by current stable OpenWRT and Debian
versions) do not have support for Armada 3700 watchdog driver. Therefore
they are not able to periodically kick watchdog so Espressobin enter into
boot loop.

This change enable CONFIG_WDT, CONFIG_WDT_ARMADA_37XX and CONFIG_CMD_WDT
options which add support for U-Boot 'wdt' command. And unset new
CONFIG_WATCHDOG_AUTOSTART option which cause that watchdog is not
automatically started by U-Boot during init phase, like when CONFIG_WDT
option is not set at all.

So with this change, U-Boot on Espressobin would have working 'wdt' command
which can be used from boot scripts (e.g. for enabling watchdog prior new
Linux booting kernel). But default behavior of watchdog status stays
unchanged, U-Boot does not start watchdog on Espressobin during its init
phase.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 configs/mvebu_espressobin-88f3720_defconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index c791f98dfea5..20aa21b06b3d 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -32,6 +32,7 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
+CONFIG_CMD_WDT=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
@@ -88,5 +89,8 @@ CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_MCS7830=y
 CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
+# CONFIG_WATCHDOG_AUTOSTART is not set
+CONFIG_WDT=y
+CONFIG_WDT_ARMADA_37XX=y
 CONFIG_SHA1=y
 CONFIG_SHA256=y
-- 
2.20.1

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

[Pali Rohár]

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.

[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@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

[PATCH v2 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[Stefan Roese]

On 09.03.21 14:26, Pali Roh?r wrote:
> Watchdog is ready after successful call of ops->start() callback in
> wdt_start() function. And is stopped after successful call of ops->stop()
> callback in wdt_stop function.
> 
> So move setting of GD_FLG_WDT_READY flag from initr_watchdog() function to
> wdt_start() and ensure that GD_FLG_WDT_READY flag is unset in wdt_stop()
> function.
> 
> This change ensures that GD_FLG_WDT_READY flag is set only when watchdog is
> running. And ensures that flag is also also when watchdog was started not
> only by initr_watchdog() call (e.g. by U-Boot 'wdt' command).
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/watchdog/wdt-uclass.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 28f7918c4673..3f707f61f74f 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -51,7 +51,6 @@ int initr_watchdog(void)
>   	}
>   
>   	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> -	gd->flags |= GD_FLG_WDT_READY;
>   	printf("WDT:   Started with%s servicing (%ds timeout)\n",
>   	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
>   
> @@ -61,21 +60,31 @@ int initr_watchdog(void)
>   int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->start)
>   		return -ENOSYS;
>   
> -	return ops->start(dev, timeout_ms, flags);
> +	ret = ops->start(dev, timeout_ms, flags);
> +	if (ret == 0)
> +		gd->flags |= GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_stop(struct udevice *dev)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->stop)
>   		return -ENOSYS;
>   
> -	return ops->stop(dev);
> +	ret = ops->stop(dev);
> +	if (ret == 0)
> +		gd->flags &= ~GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_reset(struct udevice *dev)
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH v2 2/4] watchdog: Show error message when initr_watchdog() cannot start watchdog

[Stefan Roese]

On 09.03.21 14:26, Pali Roh?r wrote:
> Function wdt_start() may fail. So in initr_watchdog() function check return
> value of wdt_start() call and print error message when watchdog starting
> failed.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/watchdog/wdt-uclass.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 3f707f61f74f..7500b3ed90e3 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -27,6 +27,7 @@ static ulong reset_period = 1000;
>   int initr_watchdog(void)
>   {
>   	u32 timeout = WATCHDOG_TIMEOUT_SECS;
> +	int ret;
>   
>   	/*
>   	 * Init watchdog: This will call the probe function of the
> @@ -50,7 +51,12 @@ int initr_watchdog(void)
>   						    4 * reset_period) / 4;
>   	}
>   
> -	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> +	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> +	if (ret != 0) {
> +		printf("WDT:   Failed to start\n");
> +		return 0;
> +	}
> +
>   	printf("WDT:   Started with%s servicing (%ds timeout)\n",
>   	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
>   
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

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

[Stefan Roese]

On 09.03.21 14:26, 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 the
> user want to start the 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 the watchdog during init phase or not.
> 
> This option is enabled by default as it was the default behavior prior
> 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>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/watchdog/Kconfig      | 13 +++++++++++++
>   drivers/watchdog/wdt-uclass.c |  5 +++++
>   2 files changed, 18 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..0603ffbd36d9 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -51,6 +51,11 @@ int initr_watchdog(void)
>   						    4 * reset_period) / 4;
>   	}
>   
> +	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
> +		printf("WDT:   Not starting\n");
> +		return 0;
> +	}
> +
>   	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
>   	if (ret != 0) {
>   		printf("WDT:   Failed to start\n");
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH v2 4/4] arm: mvebu: Espressobin: Enable watchdog support but do not start it

[Stefan Roese]

On 09.03.21 14:26, Pali Roh?r wrote:
> Espressobin had disabled watchdog support (CONFIG_WDT) because older stable
> Linux kernel versions (which are used by current stable OpenWRT and Debian
> versions) do not have support for Armada 3700 watchdog driver. Therefore
> they are not able to periodically kick watchdog so Espressobin enter into
> boot loop.
> 
> This change enable CONFIG_WDT, CONFIG_WDT_ARMADA_37XX and CONFIG_CMD_WDT
> options which add support for U-Boot 'wdt' command. And unset new
> CONFIG_WATCHDOG_AUTOSTART option which cause that watchdog is not
> automatically started by U-Boot during init phase, like when CONFIG_WDT
> option is not set at all.
> 
> So with this change, U-Boot on Espressobin would have working 'wdt' command
> which can be used from boot scripts (e.g. for enabling watchdog prior new
> Linux booting kernel). But default behavior of watchdog status stays
> unchanged, U-Boot does not start watchdog on Espressobin during its init
> phase.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   configs/mvebu_espressobin-88f3720_defconfig | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index c791f98dfea5..20aa21b06b3d 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -32,6 +32,7 @@ CONFIG_CMD_MMC=y
>   CONFIG_CMD_PCI=y
>   CONFIG_CMD_SPI=y
>   CONFIG_CMD_USB=y
> +CONFIG_CMD_WDT=y
>   CONFIG_CMD_TFTPPUT=y
>   CONFIG_CMD_CACHE=y
>   CONFIG_CMD_TIME=y
> @@ -88,5 +89,8 @@ CONFIG_USB_ETHER_ASIX=y
>   CONFIG_USB_ETHER_MCS7830=y
>   CONFIG_USB_ETHER_RTL8152=y
>   CONFIG_USB_ETHER_SMSC95XX=y
> +# CONFIG_WATCHDOG_AUTOSTART is not set
> +CONFIG_WDT=y
> +CONFIG_WDT_ARMADA_37XX=y
>   CONFIG_SHA1=y
>   CONFIG_SHA256=y
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

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

[Pali Rohár]

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!

[PATCH v2 1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop()

[Stefan Roese]

On 09.03.21 14:26, Pali Roh?r wrote:
> Watchdog is ready after successful call of ops->start() callback in
> wdt_start() function. And is stopped after successful call of ops->stop()
> callback in wdt_stop function.
> 
> So move setting of GD_FLG_WDT_READY flag from initr_watchdog() function to
> wdt_start() and ensure that GD_FLG_WDT_READY flag is unset in wdt_stop()
> function.
> 
> This change ensures that GD_FLG_WDT_READY flag is set only when watchdog is
> running. And ensures that flag is also also when watchdog was started not
> only by initr_watchdog() call (e.g. by U-Boot 'wdt' command).
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master (whole series)

Thanks,
Stefan

> ---
>   drivers/watchdog/wdt-uclass.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 28f7918c4673..3f707f61f74f 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -51,7 +51,6 @@ int initr_watchdog(void)
>   	}
>   
>   	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> -	gd->flags |= GD_FLG_WDT_READY;
>   	printf("WDT:   Started with%s servicing (%ds timeout)\n",
>   	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
>   
> @@ -61,21 +60,31 @@ int initr_watchdog(void)
>   int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->start)
>   		return -ENOSYS;
>   
> -	return ops->start(dev, timeout_ms, flags);
> +	ret = ops->start(dev, timeout_ms, flags);
> +	if (ret == 0)
> +		gd->flags |= GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_stop(struct udevice *dev)
>   {
>   	const struct wdt_ops *ops = device_get_ops(dev);
> +	int ret;
>   
>   	if (!ops->stop)
>   		return -ENOSYS;
>   
> -	return ops->stop(dev);
> +	ret = ops->stop(dev);
> +	if (ret == 0)
> +		gd->flags &= ~GD_FLG_WDT_READY;
> +
> +	return ret;
>   }
>   
>   int wdt_reset(struct udevice *dev)
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de