Hi, On Thu, 2017-03-30 at 06:11 -0700, Guenter Roeck wrote: > On 03/14/2017 07:11 AM, Sylvain Lemieux wrote: > > From: Sylvain Lemieux <slemieux@xxxxxxxxxxx> > > > > There is a need to allow a grace period after the watchdog software > > client has closed. It could be used for syncing the filesystem or > > allow graceful termination while still providing a hardware reset > > in case the system has hung. > > > > The "always-running" configuration from device-tree does not provide > > this since it will automatically keep the hardware watchdog alive as > > soon as the software client closes (i.e. keep toggling the GPIO line > > regardless of the state of the soft part of the watchdog). > > > > The "keep-armed-on-close" member in the GPIO watchdog implementation > > indicates if an expired timeout should cause a reset. > > > > This patch add a new "keep-armed-on-close" device-tree configuration > > that will keep the watchdog "armed" until the next timeout period after > > a close. During this period, the hardware watchdog is kept alive. > > A software watchdog client that wants to provide a grace period before > > a hard reset can set the timeout before properly closing. > > > > The description doesn't match what the code actually does, at least from > an infrastructure perspective. The infrastructure would just keep it running. > I will need to send a new version with an updated description; I did not update the description after this patch was rebased on-top of the "watchdog: gpio: keepalives" patch. > What you are really asking for is something the infrastructure should possibly > do by itself automatically: To keep pinging a HW watchdog after close until > the configured (software) timeout period expires. This would be in line with > expectations. > > Also, I seem to recall that the gpio_wdt patch this relies on has a problem > if the watchdog is opened and closed repeatedly. It is still on my task list > to track this down. The original "watchdog: gpio: keepalives" patch did not have any issue. I think this patch should be apply mainline. The problem was related to the management of the WDOG_HW_RUNNING flag (updated in version 2 of this patch) and an issue in the test program on our lpc32xx platform (also using pnx4008 watchdog) during the initial testing with kernel 4.9. Sylvain > > Guenter > > > Signed-off-by: Sylvain Lemieux <slemieux@xxxxxxxxxxx> > > --- > > * This patch depend on the: > > "watchdog: gpio: Convert to use infrastructure triggered keepalives"; > > ref. https://lkml.org/lkml/2016/2/28/239 (version 8) > > > > Changes from v1 to v2: > > * Rebased on-top of the "watchdog: gpio: keepalives" patch. > > - Updated the management of the "WDOG_HW_RUNNING" flag. > > * Tested with v4.11-rc1. > > > > Documentation/devicetree/bindings/watchdog/gpio-wdt.txt | 3 +++ > > drivers/watchdog/gpio_wdt.c | 10 ++++++++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > > index 83d28146e39b..48db076771b2 100644 > > --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > > +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > > @@ -17,6 +17,9 @@ Optional Properties: > > - always-running: If the watchdog timer cannot be disabled, add this flag to > > have the driver keep toggling the signal without a client. It will only cease > > to toggle the signal when the device is open and the timeout elapsed. > > +- keep-armed-on-close: if the watchdog timer need to keep toggling the signal > > + when close, until the timeout elapsed, add this flag to have the driver > > + keep toggling the signal, until the timeout elapsed. > > - timeout-sec: Contains the watchdog timeout in seconds. > > - start-at-init: Start kicking watchdog as soon as driver is loaded. > > > > diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c > > index 7b46d224cb56..2f4799bee9bd 100644 > > --- a/drivers/watchdog/gpio_wdt.c > > +++ b/drivers/watchdog/gpio_wdt.c > > @@ -29,6 +29,7 @@ struct gpio_wdt_priv { > > bool active_low; > > bool state; > > bool always_running; > > + bool keep_armed_on_close; > > unsigned int hw_algo; > > struct watchdog_device wdd; > > }; > > @@ -78,6 +79,13 @@ static int gpio_wdt_stop(struct watchdog_device *wdd) > > { > > struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd); > > > > + if(priv->keep_armed_on_close) { > > + /* Keep the driver running on close. */ > > + set_bit(WDOG_HW_RUNNING, &wdd->status); > > + > > + return 0; > > + } > > + > > if (!priv->always_running) { > > gpio_wdt_disable(priv); > > clear_bit(WDOG_HW_RUNNING, &wdd->status); > > @@ -148,6 +156,8 @@ static int gpio_wdt_probe(struct platform_device *pdev) > > > > priv->always_running = of_property_read_bool(pdev->dev.of_node, > > "always-running"); > > + priv->keep_armed_on_close = of_property_read_bool(pdev->dev.of_node, > > + "keep-armed-on-close"); > > > > watchdog_set_drvdata(&priv->wdd, priv); > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
