From 85876c8be75894ecaa8ac75e01bfba0cbe1cfc45 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 8 Nov 2019 12:03:29 +0100 Subject: watchdog: always populate watchdog priority from device tree if possible 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 Signed-off-by: Sascha Hauer --- 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 1faba813be..f4bfb68d03 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 e1343bac76..0862a7e94c 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 8b13950238..39cac6f6c4 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 184a218916..105b7ca810 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 -- cgit v1.2.3 From e8a1aef7c1b8395ae7971d24a13db9b9a935902f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 8 Nov 2019 12:03:30 +0100 Subject: watchdog: implement generic support for .running device parameter 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 Signed-off-by: Sascha Hauer --- 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 39cac6f6c4..fcead11755 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 105b7ca810..5790205a48 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -16,6 +16,10 @@ #include #include +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 *); -- cgit v1.2.3 From fabd576bd3122004e66ceb53743071b1a96b59da Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 8 Nov 2019 12:03:31 +0100 Subject: watchdog: imxwd: support .running device parameter on i.MX2+ 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 Signed-off-by: Sascha Hauer --- drivers/watchdog/imxwd.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/watchdog/imxwd.c b/drivers/watchdog/imxwd.c index 77a3bd76ce..b2cfd1cd3a 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, }; -- cgit v1.2.3 From 0ae3c6ab8eaf2291905541914d972320cfa0910c Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 8 Nov 2019 12:03:32 +0100 Subject: watchdog: f71808e: support .running device parameter 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 Signed-off-by: Sascha Hauer --- 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 4f881a1d02..5307ab0b3e 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; -- cgit v1.2.3 From 1d86578493aaddf04672731275e5e2df0f2dda94 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 22 Nov 2019 14:39:29 +0100 Subject: watchdog: stm32_iwdg: explicitly set .running to UNSUPPORTED I've spent some time trying to get the ONF (Watchdog enable status) bit in the IWDG_SR register to read as something other than zero. It has since been confirmed to be non-functional[1]. To avoid someone else spending time on this, document that running status is unsupported on this hardware explicitly. No functional change as UNSUPPORTED is already the default. [1]: https://www.spinics.net/lists/arm-kernel/msg770527.html Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/stm32_iwdg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index 4d252e558c..9fcb5e420d 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -256,6 +256,7 @@ static int stm32_iwdg_probe(struct device_d *dev) wdd->set_timeout = stm32_iwdg_set_timeout; wdd->timeout_max = (RLR_MAX + 1) * data->max_prescaler * 1000; wdd->timeout_max /= wd->rate * 1000; + wdd->running = WDOG_HW_RUNNING_UNSUPPORTED; /* ONF bit not present in IP */ ret = watchdog_register(wdd); if (ret) { -- cgit v1.2.3 From 713a601bde07bb498f99bc6288d0c10ddcbe2e0a Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 21 Nov 2019 09:40:03 +0100 Subject: param: add dev_add_param_tristate(_ro) helpers This is can be considered an extension to the dev_add_param_bool interfaces with a third value that's "unknown". This can be used for cases, where barebox can flip a bit somewhere, but it has no way of knowing what the initial state of the bit was, e.g. turn on/off the watchdog, but no watchdog status. Turn on/off a co-processor, but no co-processor online status. And so on. Not providing a way to customize the "unknown" string is a deliberate choice, so future device parameters follow the same naming scheme. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- include/param.h | 24 ++++++++++++++++++++++++ lib/parameter.c | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/include/param.h b/include/param.h index 4ac502e726..d75f50ea3e 100644 --- a/include/param.h +++ b/include/param.h @@ -63,6 +63,16 @@ struct param_d *dev_add_param_enum(struct device_d *dev, const char *name, int (*get)(struct param_d *p, void *priv), int *value, const char * const *names, int max, void *priv); +enum param_tristate { PARAM_TRISTATE_UNKNOWN, PARAM_TRISTATE_TRUE, PARAM_TRISTATE_FALSE }; + +struct param_d *dev_add_param_tristate(struct device_d *dev, const char *name, + int (*set)(struct param_d *p, void *priv), + int (*get)(struct param_d *p, void *priv), + int *value, void *priv); + +struct param_d *dev_add_param_tristate_ro(struct device_d *dev, const char *name, + int *value); + struct param_d *dev_add_param_bitmask(struct device_d *dev, const char *name, int (*set)(struct param_d *p, void *priv), int (*get)(struct param_d *p, void *priv), @@ -144,6 +154,20 @@ static inline struct param_d *dev_add_param_bitmask(struct device_d *dev, const return ERR_PTR(-ENOSYS); } +static inline struct param_d *dev_add_param_tristate(struct device_d *dev, const char *name, + int (*set)(struct param_d *p, void *priv), + int (*get)(struct param_d *p, void *priv), + int *value, void *priv) +{ + return ERR_PTR(-ENOSYS); +} + +static inline struct param_d *dev_add_param_tristate_ro(struct device_d *dev, const char *name, + int *value) +{ + return ERR_PTR(-ENOSYS); +} + static inline struct param_d *dev_add_param_ip(struct device_d *dev, const char *name, int (*set)(struct param_d *p, void *priv), int (*get)(struct param_d *p, void *priv), diff --git a/lib/parameter.c b/lib/parameter.c index fdbb2e71d1..22695634e5 100644 --- a/lib/parameter.c +++ b/lib/parameter.c @@ -588,6 +588,28 @@ struct param_d *dev_add_param_enum(struct device_d *dev, const char *name, return &pe->param; } +static const char *const tristate_names[] = { + [PARAM_TRISTATE_UNKNOWN] = "unknown", + [PARAM_TRISTATE_TRUE] = "1", + [PARAM_TRISTATE_FALSE] = "0", +}; + +struct param_d *dev_add_param_tristate(struct device_d *dev, const char *name, + int (*set)(struct param_d *p, void *priv), + int (*get)(struct param_d *p, void *priv), + int *value, void *priv) +{ + return dev_add_param_enum(dev, name, set, get, value, tristate_names, + ARRAY_SIZE(tristate_names), priv); +} + +struct param_d *dev_add_param_tristate_ro(struct device_d *dev, const char *name, + int *value) +{ + return dev_add_param_enum_ro(dev, name, value, tristate_names, + ARRAY_SIZE(tristate_names)); +} + struct param_bitmask { struct param_d param; unsigned long *value; -- cgit v1.2.3 From aade8d53cf6cccf7958d2c11e5791dfcd85a584f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 21 Nov 2019 09:40:04 +0100 Subject: watchdog: core: use new dev_add_param_tristate helper for .running param Previous commit added a dev_add_param_tristate_ro that can be readily used instead of the enum parameter here. Use it. This also fixes the issue that running_names had external linkage. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/wd_core.c | 9 +-------- include/watchdog.h | 5 ++++- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c index fcead11755..b6e2a37b1f 100644 --- a/drivers/watchdog/wd_core.c +++ b/drivers/watchdog/wd_core.c @@ -152,12 +152,6 @@ 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; @@ -176,8 +170,7 @@ 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)); + p = dev_add_param_tristate_ro(&wd->dev, "running", &wd->running); if (IS_ERR(p)) return PTR_ERR(p); diff --git a/include/watchdog.h b/include/watchdog.h index 5790205a48..9741570ce2 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -15,9 +15,12 @@ #include #include +#include enum wdog_hw_runnning { - WDOG_HW_RUNNING_UNSUPPORTED, WDOG_HW_RUNNING, WDOG_HW_NOT_RUNNING + WDOG_HW_RUNNING_UNSUPPORTED = PARAM_TRISTATE_UNKNOWN, + WDOG_HW_RUNNING = PARAM_TRISTATE_TRUE, + WDOG_HW_NOT_RUNNING = PARAM_TRISTATE_FALSE }; struct watchdog { -- cgit v1.2.3