mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible
@ 2019-11-08 11:03 Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 2/4] watchdog: implement generic support for .running device parameter Ahmad Fatoum
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2019-11-08 11:03 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, only the da9063 and da9053 have made use of the optional barebox
watchdog-priority binding. Move it into the core, so other device
drivers automatically have their watchdog-priority property parsed as
well. This patch doesn't introduce any functional changes for upstream
boards.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v3:
 unchanged
---
 drivers/mfd/da9053.c       |  1 -
 drivers/mfd/da9063.c       |  1 -
 drivers/watchdog/wd_core.c | 34 ++++++++++++++++++----------------
 include/watchdog.h         |  7 -------
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/mfd/da9053.c b/drivers/mfd/da9053.c
index 1faba813bee8..f4bfb68d0301 100644
--- a/drivers/mfd/da9053.c
+++ b/drivers/mfd/da9053.c
@@ -271,7 +271,6 @@ static int da9053_probe(struct device_d *dev)
 	da9053->dev = dev;
 	da9053->client = to_i2c_client(dev);
 	da9053->wd.set_timeout = da9053_set_timeout;
-	da9053->wd.priority = of_get_watchdog_priority(dev->device_node);
 	da9053->wd.hwdev = dev;
 
 	ret = da9053_enable_multiwrite(da9053);
diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c
index e1343bac7618..0862a7e94c46 100644
--- a/drivers/mfd/da9063.c
+++ b/drivers/mfd/da9063.c
@@ -377,7 +377,6 @@ static int da9063_probe(struct device_d *dev)
 	dev_data = ret < 0 ? NULL : dev_data_tmp;
 
 	priv = xzalloc(sizeof(struct da9063));
-	priv->wd.priority = of_get_watchdog_priority(dev->device_node);
 	priv->wd.set_timeout = da9063_watchdog_set_timeout;
 	priv->wd.hwdev = dev;
 	priv->timeout = DA9063_INITIAL_TIMEOUT;
diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 8b13950238c9..39cac6f6c494 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -127,6 +127,23 @@ static int watchdog_register_dev(struct watchdog *wd, const char *name, int id)
 	return register_device(&wd->dev);
 }
 
+/**
+ * dev_get_watchdog_priority() - get a device's desired watchdog priority
+ * @dev:	The device, which device_node to read the property from
+ *
+ * return: The priority
+ */
+static unsigned int dev_get_watchdog_priority(struct device_d *dev)
+{
+	unsigned int priority = WATCHDOG_DEFAULT_PRIORITY;
+
+	if (dev)
+		of_property_read_u32(dev->device_node, "watchdog-priority",
+				     &priority);
+
+	return priority;
+}
+
 int watchdog_register(struct watchdog *wd)
 {
 	struct param_d *p;
@@ -146,7 +163,7 @@ int watchdog_register(struct watchdog *wd)
 		return ret;
 
 	if (!wd->priority)
-		wd->priority = WATCHDOG_DEFAULT_PRIORITY;
+		wd->priority = dev_get_watchdog_priority(wd->hwdev);
 
 	p = dev_add_param_uint32(&wd->dev, "priority",
 				 watchdog_set_priority, NULL,
@@ -232,18 +249,3 @@ struct watchdog *watchdog_get_by_name(const char *name)
 	return NULL;
 }
 EXPORT_SYMBOL(watchdog_get_by_name);
-
-/**
- * of_get_watchdog_priority() - get the desired watchdog priority from device tree
- * @node:	The device_node to read the property from
- *
- * return: The priority
- */
-unsigned int of_get_watchdog_priority(struct device_node *node)
-{
-	unsigned int priority = WATCHDOG_DEFAULT_PRIORITY;
-
-	of_property_read_u32(node, "watchdog-priority", &priority);
-
-	return priority;
-}
diff --git a/include/watchdog.h b/include/watchdog.h
index 184a218916f4..105b7ca81093 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -35,7 +35,6 @@ int watchdog_deregister(struct watchdog *);
 struct watchdog *watchdog_get_default(void);
 struct watchdog *watchdog_get_by_name(const char *name);
 int watchdog_set_timeout(struct watchdog*, unsigned);
-unsigned int of_get_watchdog_priority(struct device_node *node);
 #else
 static inline int watchdog_register(struct watchdog *w)
 {
@@ -61,12 +60,6 @@ static inline int watchdog_set_timeout(struct watchdog*w, unsigned t)
 {
 	return 0;
 }
-
-
-static inline unsigned int of_get_watchdog_priority(struct device_node *node)
-{
-	return 0;
-}
 #endif
 
 #define WATCHDOG_DEFAULT_PRIORITY 100
-- 
2.24.0.rc1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 2/4] watchdog: implement generic support for .running device parameter
  2019-11-08 11:03 [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Ahmad Fatoum
@ 2019-11-08 11:03 ` Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 3/4] watchdog: imxwd: support .running device parameter on i.MX2+ Ahmad Fatoum
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2019-11-08 11:03 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Linux watchdog have an optional WDOG_HW_RUNNING bit that is used in
conjunction with CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED to automatically
ping running watchdogs until userspace takes over.

So far, when we ported Linux drivers, we dropped this detection, but it
would be useful to have this information in barebox as well:

The American Megatrends BIOS I am using allows configuring the hardware
watchdog from the BIOS. barebox enables the WDT as well, so in normal
operation we would never notice if after a BIOS update, the watchdog is
no longer enabled. If we maintain a running parameter on watchdog
devices, board code can be written to check whether the watchdog device
is indeed running.

To achieve this, add the necessary bits to the watchdog API. How we go
about it differs from Linux a little:

- We use an enum instead of a single bit, so we can differentiate between
  watchdogs that are not running and watchdogs whose running status is
  unknown.
- Because we can check whether watchdog_hw_running is supported, it now
  can fail and return a negative value in that case
- We do the maintenance of the running parameter after barebox
  feeds/disables the watchdog in the core, so it doesn't need to
  be replicated across drivers. Drivers hould only initialize the
  running parameter once at probe time.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v2 -> v3:
  - replace bitmask for running status with enum
  - use dev_add_param_enum_ro helper
v1 -> v2:
  - WDOG_HW_RUNNING: add comment describing use
  - struct watchdog: drop status_supported member in favor of having a
    single WDOG_HW_RUNNING_SUPPORTED bit
  - watchdog_set_timeout: return 0 instead of ret, when we now that ret
    == 0
---
 drivers/watchdog/wd_core.c | 21 ++++++++++++++++++++-
 include/watchdog.h         | 17 +++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 39cac6f6c494..fcead1175558 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -37,6 +37,8 @@ static const char *watchdog_name(struct watchdog *wd)
  */
 int watchdog_set_timeout(struct watchdog *wd, unsigned timeout)
 {
+	int ret;
+
 	if (!wd)
 		return -ENODEV;
 
@@ -45,7 +47,13 @@ int watchdog_set_timeout(struct watchdog *wd, unsigned timeout)
 
 	pr_debug("setting timeout on %s to %ds\n", watchdog_name(wd), timeout);
 
-	return wd->set_timeout(wd, timeout);
+	ret = wd->set_timeout(wd, timeout);
+	if (ret)
+		return ret;
+
+	wd->running = timeout ? WDOG_HW_RUNNING : WDOG_HW_NOT_RUNNING;
+
+	return 0;
 }
 EXPORT_SYMBOL(watchdog_set_timeout);
 
@@ -144,6 +152,12 @@ static unsigned int dev_get_watchdog_priority(struct device_d *dev)
 	return priority;
 }
 
+const char *running_names[] = {
+	[WDOG_HW_RUNNING_UNSUPPORTED] = "unknown",
+	[WDOG_HW_RUNNING] = "1",
+	[WDOG_HW_NOT_RUNNING] = "0",
+};
+
 int watchdog_register(struct watchdog *wd)
 {
 	struct param_d *p;
@@ -162,6 +176,11 @@ int watchdog_register(struct watchdog *wd)
 	if (ret)
 		return ret;
 
+	p = dev_add_param_enum_ro(&wd->dev, "running", &wd->running,
+				  running_names, ARRAY_SIZE(running_names));
+	if (IS_ERR(p))
+		return PTR_ERR(p);
+
 	if (!wd->priority)
 		wd->priority = dev_get_watchdog_priority(wd->hwdev);
 
diff --git a/include/watchdog.h b/include/watchdog.h
index 105b7ca81093..5790205a487b 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -16,6 +16,10 @@
 #include <poller.h>
 #include <driver.h>
 
+enum wdog_hw_runnning {
+	 WDOG_HW_RUNNING_UNSUPPORTED, WDOG_HW_RUNNING, WDOG_HW_NOT_RUNNING
+};
+
 struct watchdog {
 	int (*set_timeout)(struct watchdog *, unsigned);
 	const char *name;
@@ -27,8 +31,21 @@ struct watchdog {
 	unsigned int poller_enable;
 	struct poller_async poller;
 	struct list_head list;
+	int running; /* enum wdog_hw_running */
 };
 
+/*
+ * Use the following function to check whether or not the hardware watchdog
+ * is running
+ */
+static inline int watchdog_hw_running(struct watchdog *w)
+{
+	if (w->running == WDOG_HW_RUNNING_UNSUPPORTED)
+		return -ENOSYS;
+
+	return w->running == WDOG_HW_RUNNING;
+}
+
 #ifdef CONFIG_WATCHDOG
 int watchdog_register(struct watchdog *);
 int watchdog_deregister(struct watchdog *);
-- 
2.24.0.rc1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 3/4] watchdog: imxwd: support .running device parameter on i.MX2+
  2019-11-08 11:03 [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 2/4] watchdog: implement generic support for .running device parameter Ahmad Fatoum
@ 2019-11-08 11:03 ` Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 4/4] watchdog: f71808e: support .running device parameter Ahmad Fatoum
  2019-11-11  8:20 ` [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2019-11-08 11:03 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The i.MX can be fused to start the watchdog on power-on reset.
To give users an easy way to determine whether the watchdog is running,
implement support for WDOG_HW_RUNNING.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v2 -> v3:
  Use new enum wdog_hw_running
v1 -> v2:
  Use new WDOG_HW_RUNNING_SUPPORTED
---
 drivers/watchdog/imxwd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/watchdog/imxwd.c b/drivers/watchdog/imxwd.c
index 77a3bd76cefa..b2cfd1cd3a31 100644
--- a/drivers/watchdog/imxwd.c
+++ b/drivers/watchdog/imxwd.c
@@ -28,6 +28,7 @@ struct imx_wd_ops {
 	int (*set_timeout)(struct imx_wd *, unsigned);
 	void (*soc_reset)(struct imx_wd *);
 	int (*init)(struct imx_wd *);
+	bool (*is_running)(struct imx_wd *);
 	unsigned int timeout_max;
 };
 
@@ -111,6 +112,11 @@ static void imx1_soc_reset(struct imx_wd *priv)
 	writew(IMX1_WDOG_WCR_WDE, priv->base + IMX1_WDOG_WCR);
 }
 
+static inline bool imx21_watchdog_is_running(struct imx_wd *priv)
+{
+	return imxwd_read(priv, IMX21_WDOG_WCR) & IMX21_WDOG_WCR_WDE;
+}
+
 static int imx21_watchdog_set_timeout(struct imx_wd *priv, unsigned timeout)
 {
 	u16 val;
@@ -243,6 +249,13 @@ static int imx_wd_probe(struct device_d *dev)
 						"fsl,ext-reset-output");
 
 	if (IS_ENABLED(CONFIG_WATCHDOG_IMX)) {
+		if (priv->ops->is_running) {
+			if (priv->ops->is_running(priv))
+				priv->wd.running = WDOG_HW_RUNNING;
+			else
+				priv->wd.running = WDOG_HW_NOT_RUNNING;
+		}
+
 		ret = watchdog_register(&priv->wd);
 		if (ret)
 			goto on_error;
@@ -277,6 +290,7 @@ static const struct imx_wd_ops imx21_wd_ops = {
 	.set_timeout = imx21_watchdog_set_timeout,
 	.soc_reset = imx21_soc_reset,
 	.init = imx21_wd_init,
+	.is_running = imx21_watchdog_is_running,
 	.timeout_max = 128,
 };
 
-- 
2.24.0.rc1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 4/4] watchdog: f71808e: support .running device parameter
  2019-11-08 11:03 [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 2/4] watchdog: implement generic support for .running device parameter Ahmad Fatoum
  2019-11-08 11:03 ` [PATCH v3 3/4] watchdog: imxwd: support .running device parameter on i.MX2+ Ahmad Fatoum
@ 2019-11-08 11:03 ` Ahmad Fatoum
  2019-11-11  8:20 ` [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2019-11-08 11:03 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The American Megatrends BIOS I am using can be configured to start the
Fintek watchdog prior to the UEFI payloads. To avoid BIOS updates that reset
this functionality going unnoticed, implement support for WDOG_HW_RUNNING.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v2 -> v3:
  Use new enum wdog_hw_running
v1 -> v2:
  Use new WDOG_HW_RUNNING_SUPPORTED
---
 drivers/watchdog/f71808e_wdt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 4f881a1d02bc..5307ab0b3ead 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -222,7 +222,7 @@ static int f71808e_wdt_init(struct f71808e_wdt *wd, struct device_d *dev)
 {
 	struct watchdog *wdd = &wd->wdd;
 	const char * const *names = pulse_width_names;
-	int wdt_conf;
+	unsigned long wdt_conf;
 	int ret;
 
 	superio_enter(wd->sioaddr);
@@ -262,6 +262,11 @@ static int f71808e_wdt_init(struct f71808e_wdt *wd, struct device_d *dev)
 
 	dev_info(dev, "reset reason: %s\n", reset_source_name());
 
+	if (test_bit(F71808FG_FLAG_WD_EN, &wdt_conf))
+		wdd->running = WDOG_HW_RUNNING;
+	else
+		wdd->running = WDOG_HW_NOT_RUNNING;
+
 	ret = watchdog_register(wdd);
 	if (ret)
 		return ret;
-- 
2.24.0.rc1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible
  2019-11-08 11:03 [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2019-11-08 11:03 ` [PATCH v3 4/4] watchdog: f71808e: support .running device parameter Ahmad Fatoum
@ 2019-11-11  8:20 ` Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2019-11-11  8:20 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Fri, Nov 08, 2019 at 12:03:29PM +0100, Ahmad Fatoum wrote:
> So far, only the da9063 and da9053 have made use of the optional barebox
> watchdog-priority binding. Move it into the core, so other device
> drivers automatically have their watchdog-priority property parsed as
> well. This patch doesn't introduce any functional changes for upstream
> boards.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

Applied, thanks

Sascha


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-11-11  8:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 11:03 [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Ahmad Fatoum
2019-11-08 11:03 ` [PATCH v3 2/4] watchdog: implement generic support for .running device parameter Ahmad Fatoum
2019-11-08 11:03 ` [PATCH v3 3/4] watchdog: imxwd: support .running device parameter on i.MX2+ Ahmad Fatoum
2019-11-08 11:03 ` [PATCH v3 4/4] watchdog: f71808e: support .running device parameter Ahmad Fatoum
2019-11-11  8:20 ` [PATCH v3 1/4] watchdog: always populate watchdog priority from device tree if possible Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox