[U-Boot] [RFC PATCH v1] wdt: Add a glue driver for watchdog

[U-Boot] [RFC PATCH v1] wdt: Add a glue driver for watchdog

[Andy Shevchenko]

This driver enables first found watchdog as a system one and uses it as a
system hardware watchdog device by providing hw_watchdog_reset(),
hw_watchdog_disable() and void hw_watchdog_init() hooks.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

This is not a working solution, just an opinion to which direction we might go
with ugly broken WDT class.

Feel free to ignore, drop, modify, etc.

 drivers/watchdog/Kconfig  |  8 +++++++
 drivers/watchdog/Makefile |  1 +
 drivers/watchdog/wdt-hw.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)
 create mode 100644 drivers/watchdog/wdt-hw.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0ed5aa987f..4837e73565 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -35,6 +35,14 @@ config WDT
 	  What exactly happens when the timer expires is up to a particular
 	  device/driver.
 
+config WDT_HW
+	bool "Enable hardware Watchdog Timer"
+	depends on WDT
+	select HW_WATCHDOG
+	help
+		Enable hardware Watchdog Timer. This is a glue driver to enable
+		first found watchdog device as a system one.
+
 config WDT_SANDBOX
 	bool "Enable Watchdog Timer support for Sandbox"
 	depends on SANDBOX && WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0ad16f661e..c4a297e88b 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
 obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
 obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o
 obj-$(CONFIG_WDT) += wdt-uclass.o
+obj-$(CONFIG_WDT_HW) += wdt-hw.o
 obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o
 obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o
 obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o
diff --git a/drivers/watchdog/wdt-hw.c b/drivers/watchdog/wdt-hw.c
new file mode 100644
index 0000000000..3e98db86c8
--- /dev/null
+++ b/drivers/watchdog/wdt-hw.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <dm.h>
+#include <wdt.h>
+
+/* Hardware timeout in milliseconds */
+#ifdef CONFIG_WATCHDOG_TIMEOUT_MSECS
+#define WATCHDOG_HEARTBEAT CONFIG_WATCHDOG_TIMEOUT_MSECS
+#else
+#define WATCHDOG_HEARTBEAT 60000
+#endif
+
+static struct udevice *wdt;
+
+void hw_watchdog_reset(void)
+{
+	int ret;
+
+	if (!wdt)
+		return;
+
+	ret = wdt_reset(wdt);
+	if (ret)
+		printf("Can't reset watchdog device: %d\n", ret);
+}
+
+void hw_watchdog_disable(void)
+{
+	int ret;
+
+	if (!wdt)
+		return;
+
+	ret = wdt_stop(wdt);
+	if (ret)
+		printf("Can't stop watchdog device: %d\n", ret);
+}
+
+void hw_watchdog_init(void)
+{
+	int ret;
+
+	ret = uclass_first_device(UCLASS_WDT, &wdt);
+	if (ret)
+		printf("Can't find watchdog device: %d\n", ret);
+	else {
+		ret = wdt_start(wdt, WATCHDOG_HEARTBEAT * 1000, 0);
+		if (ret)
+			printf("Can't start watchdog device: %d\n", ret);
+	}
+}
-- 
2.11.0

[U-Boot] [U-Boot,RFC,v1] wdt: Add a glue driver for watchdog

[Heinrich Schuchardt]

On 07/05/2017 07:45 PM, Andy Shevchenko wrote:
> This driver enables first found watchdog as a system one and uses it as a
> system hardware watchdog device by providing hw_watchdog_reset(),
> hw_watchdog_disable() and void hw_watchdog_init() hooks.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> 
> This is not a working solution, just an opinion to which direction we might go
> with ugly broken WDT class.
> 
> Feel free to ignore, drop, modify, etc.
> 
>  drivers/watchdog/Kconfig  |  8 +++++++
>  drivers/watchdog/Makefile |  1 +
>  drivers/watchdog/wdt-hw.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 drivers/watchdog/wdt-hw.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0ed5aa987f..4837e73565 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -35,6 +35,14 @@ config WDT
>  	  What exactly happens when the timer expires is up to a particular
>  	  device/driver.
>  
> +config WDT_HW
> +	bool "Enable hardware Watchdog Timer"
> +	depends on WDT
> +	select HW_WATCHDOG
> +	help
> +		Enable hardware Watchdog Timer. This is a glue driver to enable
> +		first found watchdog device as a system one.
> +
>  config WDT_SANDBOX
>  	bool "Enable Watchdog Timer support for Sandbox"
>  	depends on SANDBOX && WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 0ad16f661e..c4a297e88b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>  obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
>  obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o
>  obj-$(CONFIG_WDT) += wdt-uclass.o
> +obj-$(CONFIG_WDT_HW) += wdt-hw.o
>  obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o
>  obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o
>  obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o
> diff --git a/drivers/watchdog/wdt-hw.c b/drivers/watchdog/wdt-hw.c
> new file mode 100644
> index 0000000000..3e98db86c8
> --- /dev/null
> +++ b/drivers/watchdog/wdt-hw.c
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <dm.h>
> +#include <wdt.h>
> +
> +/* Hardware timeout in milliseconds */
> +#ifdef CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#define WATCHDOG_HEARTBEAT CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#else
> +#define WATCHDOG_HEARTBEAT 60000
> +#endif
> +
> +static struct udevice *wdt;
> +
> +void hw_watchdog_reset(void)
> +{
> +	int ret;
> +
> +	if (!wdt)
> +		return;
> +
> +	ret = wdt_reset(wdt);
> +	if (ret)
> +		printf("Can't reset watchdog device: %d\n", ret);

I would suggest debug() instead of printf().

Imagine this being called from an EFI application in a loop. We wouldn't
want to fill the screen with this message.

Best regards

Heinrich Schuchardt

[U-Boot] [U-Boot,RFC,v1] wdt: Add a glue driver for watchdog

[Andy Shevchenko]

On Wed, 2017-07-05 at 21:56 +0200, Heinrich Schuchardt wrote:
> On 07/05/2017 07:45 PM, Andy Shevchenko wrote:

> > This is not a working solution, just an opinion to which direction
> > we might go
> > with ugly broken WDT class.
> > 
> > Feel free to ignore, drop, modify, etc.

^^^^^^ DISCLAIMER

> > +	ret = wdt_reset(wdt);
> > +	if (ret)
> > +		printf("Can't reset watchdog device: %d\n", ret);
> 
> I would suggest debug() instead of printf().
> 
> Imagine this being called from an EFI application in a loop. We
> wouldn't
> want to fill the screen with this message.

Thanks for the indeed correct suggestion, though please read a
disclaimer above. I gave up on that.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[U-Boot] [U-Boot,RFC,v1] wdt: Add a glue driver for watchdog

[Heinrich Schuchardt]

On 07/05/2017 07:45 PM, Andy Shevchenko wrote:
> This driver enables first found watchdog as a system one and uses it as a
> system hardware watchdog device by providing hw_watchdog_reset(),
> hw_watchdog_disable() and void hw_watchdog_init() hooks.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> 
> This is not a working solution, just an opinion to which direction we might go
> with ugly broken WDT class.

We need a watchdog driver to complete the UEFI implementation.

Currently you are only thinking about a hardware watchdog. We do not
have hardware watchdog drivers for every board.

I would prefer if we had a software fallback solution that would update
the timer ticks when waiting for keyboard or in other loops like network
traffic.

Best regards

Heinrich

> 
> Feel free to ignore, drop, modify, etc.
> 
>  drivers/watchdog/Kconfig  |  8 +++++++
>  drivers/watchdog/Makefile |  1 +
>  drivers/watchdog/wdt-hw.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 drivers/watchdog/wdt-hw.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0ed5aa987f..4837e73565 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -35,6 +35,14 @@ config WDT
>  	  What exactly happens when the timer expires is up to a particular
>  	  device/driver.
>  
> +config WDT_HW
> +	bool "Enable hardware Watchdog Timer"
> +	depends on WDT
> +	select HW_WATCHDOG
> +	help
> +		Enable hardware Watchdog Timer. This is a glue driver to enable
> +		first found watchdog device as a system one.
> +
>  config WDT_SANDBOX
>  	bool "Enable Watchdog Timer support for Sandbox"
>  	depends on SANDBOX && WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 0ad16f661e..c4a297e88b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>  obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
>  obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o
>  obj-$(CONFIG_WDT) += wdt-uclass.o
> +obj-$(CONFIG_WDT_HW) += wdt-hw.o
>  obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o
>  obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o
>  obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o
> diff --git a/drivers/watchdog/wdt-hw.c b/drivers/watchdog/wdt-hw.c
> new file mode 100644
> index 0000000000..3e98db86c8
> --- /dev/null
> +++ b/drivers/watchdog/wdt-hw.c
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (c) 2017 Intel Corporation
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <dm.h>
> +#include <wdt.h>
> +
> +/* Hardware timeout in milliseconds */
> +#ifdef CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#define WATCHDOG_HEARTBEAT CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#else
> +#define WATCHDOG_HEARTBEAT 60000
> +#endif
> +
> +static struct udevice *wdt;
> +
> +void hw_watchdog_reset(void)
> +{
> +	int ret;
> +
> +	if (!wdt)
> +		return;
> +
> +	ret = wdt_reset(wdt);
> +	if (ret)
> +		printf("Can't reset watchdog device: %d\n", ret);
> +}
> +
> +void hw_watchdog_disable(void)
> +{
> +	int ret;
> +
> +	if (!wdt)
> +		return;
> +
> +	ret = wdt_stop(wdt);
> +	if (ret)
> +		printf("Can't stop watchdog device: %d\n", ret);
> +}
> +
> +void hw_watchdog_init(void)
> +{
> +	int ret;
> +
> +	ret = uclass_first_device(UCLASS_WDT, &wdt);
> +	if (ret)
> +		printf("Can't find watchdog device: %d\n", ret);
> +	else {
> +		ret = wdt_start(wdt, WATCHDOG_HEARTBEAT * 1000, 0);
> +		if (ret)
> +			printf("Can't start watchdog device: %d\n", ret);
> +	}
> +}
>