Re: [v3] gpio_wdt: Add "always_running" feature to GPIO watchdog

[Guenter Roeck <linux@roeck-us.net>]

On Fri, Nov 21, 2014 at 10:40:28AM +0100, Mike Looijmans wrote:
> On some chips, like the TPS386000, the trigger cannot be disabled
> and the CPU must keep toggling the line at all times. Add a switch
> "always_running" to keep toggling the GPIO line regardless of the
> state of the soft part of the watchdog. The "armed" member keeps
> track of whether a timeout must also cause a reset.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---

Actially, my reviewed-by: was to early. See below.

> v3: Indentation adjusted to match
> Fix error path in probe when notification registration fails
> Prevent double assignment of "armed" variable
> 
>  .../devicetree/bindings/watchdog/gpio-wdt.txt      |    5 +++
>  drivers/watchdog/gpio_wdt.c                        |   39 ++++++++++++++++----
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> index 37afec1..1987949 100644
> --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> @@ -13,6 +13,11 @@ Required Properties:
>      by the GPIO flags.
>  - hw_margin_ms: Maximum time to reset watchdog circuit (milliseconds).
>  
> +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.
> +
>  Example:
>  	watchdog: watchdog {
>  		/* ADM706 */
> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
> index 220a9e0..9bfbd73 100644
> --- a/drivers/watchdog/gpio_wdt.c
> +++ b/drivers/watchdog/gpio_wdt.c
> @@ -31,6 +31,8 @@ struct gpio_wdt_priv {
>  	int			gpio;
>  	bool			active_low;
>  	bool			state;
> +	bool			always_running;
> +	bool			armed;
>  	unsigned int		hw_algo;
>  	unsigned int		hw_margin;
>  	unsigned long		last_jiffies;
> @@ -48,10 +50,8 @@ static void gpio_wdt_disable(struct gpio_wdt_priv *priv)
>  		gpio_direction_input(priv->gpio);
>  }
>  
> -static int gpio_wdt_start(struct watchdog_device *wdd)
> +static void gpio_wdt_start_impl(struct gpio_wdt_priv *priv)

This function is now void but still has a return value.

Guenter