Re: [PATCH] watchdog: Read device status through sysfs attributes for GPIO-controlled Watchdog.

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

On 12/8/19 10:45 AM, harshal chaudhari wrote:
> Hi Wim and Guenter,
> 
> This patch adds following attributes to GPIO-controlled watchdog device's sysfs interface to read its different status.
> 
> 
You lost me. What does this patch do that CONFIG_WATCHDOG_SYSFS doesn't do as well ?

Guenter

> * state - reads whether device is active or not
> 
> * identity - reads Watchdog device's identity string.
> 
> * timeout - reads current timeout.
> 
> * bootstatus - reads status of the watchdog device at boot.
> 
> * nowayout - reads whether nowayout feature was set or not.
> 
> 
> Testing with GPIO Watchdog:
> 
> 
> $ cat bootstatus
> 
> 0
> 
> $ cat identity
> 
> GPIO Watchdog
> 
> $ cat nowayout
> 
> 0
> 
> $ cat state
> 
> inactive
> 
> $ cat timeout
> 
> 60
> 
> 
> Signed-off-by: harshal chaudhari <harshalchau04@gmail.com <mailto:harshalchau04@gmail.com>>
> 
> 
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> index 198794963786..762149375280 100644
> --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> @@ -26,3 +26,31 @@ Example:
>                  hw_algo = "toggle";
>                  hw_margin_ms = <1600>;
>          };
> +
> +
> +* Read device status through sysfs attributes
> +
> +  state - reads whether device is active or not
> +
> +       It is a read only file. It gives active/inactive status of
> +       watchdog device.
> +
> +  identity - reads Watchdog device's identity string.
> +
> +       It is a read only file. It contains identity string of
> +       watchdog device.
> +
> +  timeout - reads current timeout.
> +       It is a read only file. It is read to know about current
> +       value of timeout programmed.
> +
> +  bootstatus - reads status of the watchdog device at boot
> +       It is a read only file. It contains status of the watchdog
> +       device at boot. It is equivalent to WDIOC_GETBOOTSTATUS of
> +       ioctl interface.
> +
> +  nowayout - reads whether nowayout feature was set or not
> +       It is a read only file. While reading, it gives '1' if that
> +       device supports nowayout feature else, it gives '0'.
> +
> +For more details refer the documents as in Documentation/ABI/testing/sysfs-class-watchdog
> 
> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
> index 0923201ce874..d49eb77ec8f1 100644
> --- a/drivers/watchdog/gpio_wdt.c
> +++ b/drivers/watchdog/gpio_wdt.c
> @@ -90,6 +90,68 @@ static int gpio_wdt_stop(struct watchdog_device *wdd)
>          return 0;
>   }
> 
> +static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,
> +                               char *buf)
> +{
> +       struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%u\n", wdd->timeout);
> +}
> +static DEVICE_ATTR_RO(timeout);
> +
> +static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
> +                               char *buf)
> +{
> +       struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%s\n", wdd->info->identity);
> +}
> +static DEVICE_ATTR_RO(identity);
> +
> +static ssize_t state_show(struct device *dev, struct device_attribute *attr,
> +                               char *buf)
> +{
> +       struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +       if (watchdog_active(wdd))
> +               return sprintf(buf, "active\n");
> +
> +       return sprintf(buf, "inactive\n");
> +}
> +static DEVICE_ATTR_RO(state);
> +
> +static ssize_t bootstatus_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%u\n", wdd->bootstatus);
> +}
> +static DEVICE_ATTR_RO(bootstatus);
> +
> +static ssize_t nowayout_show(struct device *dev, struct device_attribute *attr,
> +                               char *buf)
> +{
> +       struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%d\n", !!test_bit(WDOG_NO_WAY_OUT, &wdd->status));
> +}
> +static DEVICE_ATTR_RO(nowayout);
> +
> +static struct attribute *wdt_attrs[] = {
> +       &dev_attr_state.attr,
> +       &dev_attr_identity.attr,
> +       &dev_attr_timeout.attr,
> +       &dev_attr_bootstatus.attr,
> +       &dev_attr_nowayout.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group wdt_group = {
> +       .attrs = wdt_attrs,
> +};
> +__ATTRIBUTE_GROUPS(wdt);
> +
>   static const struct watchdog_info gpio_wdt_ident = {
>          .options        = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
>                            WDIOF_SETTIMEOUT,
> @@ -155,6 +217,7 @@ static int gpio_wdt_probe(struct platform_device *pdev)
>          priv->wdd.max_hw_heartbeat_ms = hw_margin;
>          priv->wdd.parent        = dev;
>          priv->wdd.timeout       = SOFT_TIMEOUT_DEF;
> +       priv->wdd.groups        = wdt_groups;
> 
>          watchdog_init_timeout(&priv->wdd, 0, dev);
>          watchdog_set_nowayout(&priv->wdd, nowayout);
> 
> 
>