[PATCH] watchdog: gpio_wdt: add support for stoppable devices

Back when I added this driver in commit 2ac8490412c9, I wrote
The corresponding linux driver apparently has support for some watchdog circuits which can be disabled by tri-stating the gpio, but I have never actually encountered such a chip in the wild;
That has changed now; I have a board with just such a watchdog on my desk currently. Add support for that.
- For a hw_algo="toggle" device, the gpio is requested as output if the always-running flag is set, otherwise as input.
- The ->start() method is updated to change the direction to output when required (i.e. it is not always-running).
- The ->stop() method is implemented, but of course reports failure if always-running.
As I still haven't met any hw_algo="level" devices, I'm not entirely sure how they fit in, but I'm borrowing logic from the corresponding linux driver:
- In ->probe(), such devices always request the gpio as GPIOD_IS_OUT.
- In ->stop(), the linux driver has an "eternal ping" comment and sets the gpio to (logic) high.
Signed-off-by: Rasmus Villemoes ravi@prevas.dk --- drivers/watchdog/gpio_wdt.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index 2920c2c751f..e889861e917 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -45,14 +45,32 @@ static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags) if (priv->always_running) return 0;
- return -ENOSYS; + dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_OUT); + gpio_wdt_reset(dev); + + return 0; +} + +static int gpio_wdt_stop(struct udevice *dev) +{ + struct gpio_wdt_priv *priv = dev_get_priv(dev); + + if (priv->always_running) + return -EOPNOTSUPP; + + if (priv->hw_algo == HW_ALGO_TOGGLE) + dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_IN); + else + dm_gpio_set_value(&priv->gpio, 1); + + return 0; }
static int dm_probe(struct udevice *dev) { struct gpio_wdt_priv *priv = dev_get_priv(dev); - int ret; const char *algo = dev_read_string(dev, "hw_algo"); + int ret, flags;
if (!algo) return -EINVAL; @@ -64,7 +82,9 @@ static int dm_probe(struct udevice *dev) return -EINVAL;
priv->always_running = dev_read_bool(dev, "always-running"); - ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, GPIOD_IS_OUT); + flags = priv->always_running || priv->hw_algo == HW_ALGO_LEVEL ? + GPIOD_IS_OUT : GPIOD_IS_IN; + ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, flags); if (ret < 0) { dev_err(dev, "Request for wdt gpio failed: %d\n", ret); return ret; @@ -78,6 +98,7 @@ static int dm_probe(struct udevice *dev)
static const struct wdt_ops gpio_wdt_ops = { .start = gpio_wdt_start, + .stop = gpio_wdt_stop, .reset = gpio_wdt_reset, };

On 10/2/24 21:23, Rasmus Villemoes wrote:
Back when I added this driver in commit 2ac8490412c9, I wrote
The corresponding linux driver apparently has support for some watchdog circuits which can be disabled by tri-stating the gpio, but I have never actually encountered such a chip in the wild;
That has changed now; I have a board with just such a watchdog on my desk currently. Add support for that.
For a hw_algo="toggle" device, the gpio is requested as output if the always-running flag is set, otherwise as input.
The ->start() method is updated to change the direction to output when required (i.e. it is not always-running).
The ->stop() method is implemented, but of course reports failure if always-running.
As I still haven't met any hw_algo="level" devices, I'm not entirely sure how they fit in, but I'm borrowing logic from the corresponding linux driver:
In ->probe(), such devices always request the gpio as GPIOD_IS_OUT.
In ->stop(), the linux driver has an "eternal ping" comment and sets the gpio to (logic) high.
Signed-off-by: Rasmus Villemoes ravi@prevas.dk
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/watchdog/gpio_wdt.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index 2920c2c751f..e889861e917 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -45,14 +45,32 @@ static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags) if (priv->always_running) return 0;
- return -ENOSYS;
- dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_OUT);
- gpio_wdt_reset(dev);
- return 0;
+}
+static int gpio_wdt_stop(struct udevice *dev) +{
struct gpio_wdt_priv *priv = dev_get_priv(dev);
if (priv->always_running)
return -EOPNOTSUPP;
if (priv->hw_algo == HW_ALGO_TOGGLE)
dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_IN);
else
dm_gpio_set_value(&priv->gpio, 1);
return 0; }
static int dm_probe(struct udevice *dev) { struct gpio_wdt_priv *priv = dev_get_priv(dev);
- int ret; const char *algo = dev_read_string(dev, "hw_algo");
int ret, flags;
if (!algo) return -EINVAL;
@@ -64,7 +82,9 @@ static int dm_probe(struct udevice *dev) return -EINVAL;
priv->always_running = dev_read_bool(dev, "always-running");
- ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, GPIOD_IS_OUT);
- flags = priv->always_running || priv->hw_algo == HW_ALGO_LEVEL ?
GPIOD_IS_OUT : GPIOD_IS_IN;
- ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, flags); if (ret < 0) { dev_err(dev, "Request for wdt gpio failed: %d\n", ret); return ret;
@@ -78,6 +98,7 @@ static int dm_probe(struct udevice *dev)
static const struct wdt_ops gpio_wdt_ops = { .start = gpio_wdt_start,
- .stop = gpio_wdt_stop, .reset = gpio_wdt_reset, };
Viele Grüße, Stefan Roese

Hi Rasmus,
On 10/2/24 21:23, Rasmus Villemoes wrote:
Back when I added this driver in commit 2ac8490412c9, I wrote
The corresponding linux driver apparently has support for some watchdog circuits which can be disabled by tri-stating the gpio, but I have never actually encountered such a chip in the wild;
That has changed now; I have a board with just such a watchdog on my desk currently. Add support for that.
For a hw_algo="toggle" device, the gpio is requested as output if the always-running flag is set, otherwise as input.
The ->start() method is updated to change the direction to output when required (i.e. it is not always-running).
The ->stop() method is implemented, but of course reports failure if always-running.
As I still haven't met any hw_algo="level" devices, I'm not entirely sure how they fit in, but I'm borrowing logic from the corresponding linux driver:
In ->probe(), such devices always request the gpio as GPIOD_IS_OUT.
In ->stop(), the linux driver has an "eternal ping" comment and sets the gpio to (logic) high.
Signed-off-by: Rasmus Villemoes ravi@prevas.dk
While running a CI build, I'm seeing some sandbox failures, most likely related to this change here:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=375&view=logs...
Could you please take a look at this?
Thanks, Stefan
drivers/watchdog/gpio_wdt.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index 2920c2c751f..e889861e917 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -45,14 +45,32 @@ static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags) if (priv->always_running) return 0;
- return -ENOSYS;
- dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_OUT);
- gpio_wdt_reset(dev);
- return 0;
+}
+static int gpio_wdt_stop(struct udevice *dev) +{
struct gpio_wdt_priv *priv = dev_get_priv(dev);
if (priv->always_running)
return -EOPNOTSUPP;
if (priv->hw_algo == HW_ALGO_TOGGLE)
dm_gpio_set_dir_flags(&priv->gpio, GPIOD_IS_IN);
else
dm_gpio_set_value(&priv->gpio, 1);
return 0; }
static int dm_probe(struct udevice *dev) { struct gpio_wdt_priv *priv = dev_get_priv(dev);
- int ret; const char *algo = dev_read_string(dev, "hw_algo");
int ret, flags;
if (!algo) return -EINVAL;
@@ -64,7 +82,9 @@ static int dm_probe(struct udevice *dev) return -EINVAL;
priv->always_running = dev_read_bool(dev, "always-running");
- ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, GPIOD_IS_OUT);
- flags = priv->always_running || priv->hw_algo == HW_ALGO_LEVEL ?
GPIOD_IS_OUT : GPIOD_IS_IN;
- ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, flags); if (ret < 0) { dev_err(dev, "Request for wdt gpio failed: %d\n", ret); return ret;
@@ -78,6 +98,7 @@ static int dm_probe(struct udevice *dev)
static const struct wdt_ops gpio_wdt_ops = { .start = gpio_wdt_start,
- .stop = gpio_wdt_stop, .reset = gpio_wdt_reset, };
Viele Grüße, Stefan Roese

On Tue, Oct 22 2024, Stefan Roese sr@denx.de wrote:
Hi Rasmus,
On 10/2/24 21:23, Rasmus Villemoes wrote:
While running a CI build, I'm seeing some sandbox failures, most likely related to this change here:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=375&view=logs...
Could you please take a look at this?
Ah yes, sorry about that. The two -ENOSYS in test/dm/wdt.c should probably become -EOPNOTSUPP. Can you test with that change folded in, or do you want me to resend?
Rasmus

On 10/22/24 13:43, Rasmus Villemoes wrote:
On Tue, Oct 22 2024, Stefan Roese sr@denx.de wrote:
Hi Rasmus,
On 10/2/24 21:23, Rasmus Villemoes wrote:
While running a CI build, I'm seeing some sandbox failures, most likely related to this change here:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=375&view=logs...
Could you please take a look at this?
Ah yes, sorry about that. The two -ENOSYS in test/dm/wdt.c should probably become -EOPNOTSUPP. Can you test with that change folded in, or do you want me to resend?
Let me give it a try...
Thanks, Stefan

On 10/22/24 13:46, Stefan Roese wrote:
On 10/22/24 13:43, Rasmus Villemoes wrote:
On Tue, Oct 22 2024, Stefan Roese sr@denx.de wrote:
Hi Rasmus,
On 10/2/24 21:23, Rasmus Villemoes wrote:
While running a CI build, I'm seeing some sandbox failures, most likely related to this change here:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=375&view=logs...
Could you please take a look at this?
Ah yes, sorry about that. The two -ENOSYS in test/dm/wdt.c should probably become -EOPNOTSUPP. Can you test with that change folded in, or do you want me to resend?
Let me give it a try...
Changes folded in and CI is happy.
Applied to u-boot-watchdog/master
Thanks, Stefan
participants (2)
-
Rasmus Villemoes
-
Stefan Roese