watchdog: Sysfs status read support

We need few sysfs attributes to know different status of a watchdog device. To do that, we need to associate .dev_groups with watchdog_class. So convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device attributes defined in that file can be attached to it.

Signed-off-by Pratyush Anand <[email protected]>

Suggested-by Guenter Roeck <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

---
  drivers/watchdog/watchdog_core.c | 15 ++-------------
  drivers/watchdog/watchdog_core.h |  2 +-
  drivers/watchdog/watchdog_dev.c  | 26 ++++++++++++++++++++++----
  3 files changed, 25 insertions(+), 18 deletions(-)

drivers/watchdog/watchdog_core.c +2 -13

index 873f13972cf4..a55e846eec79 100644
@@ -285,19 +285,9 @@ static int __init watchdog_deferred_registration(void)
 
  static int __init watchdog_init(void)
  {
-	int err;
-
-	watchdog_class = class_create(THIS_MODULE, "watchdog");
-	if (IS_ERR(watchdog_class)) {
-		pr_err("couldn't create class\n");
+	watchdog_class = watchdog_dev_init();
+	if (IS_ERR(watchdog_class))
  		return PTR_ERR(watchdog_class);
-	}
-
-	err = watchdog_dev_init();
-	if (err < 0) {
-		class_destroy(watchdog_class);
-		return err;
-	}
 
  	watchdog_deferred_registration();
  	return 0;

@@ -306,7 +296,6 @@ static int __init watchdog_init(void)
  static void __exit watchdog_exit(void)
  {
  	watchdog_dev_exit();
-	class_destroy(watchdog_class);
  	ida_destroy(&watchdog_ida);
  }
 

drivers/watchdog/watchdog_core.h +1 -1

index 6c951418fca7..1c8d6b0e68c7 100644
@@ -33,5 +33,5 @@
   */
  extern int watchdog_dev_register(struct watchdog_device *);
  extern int watchdog_dev_unregister(struct watchdog_device *);
-extern int __init watchdog_dev_init(void);
+extern struct class * __init watchdog_dev_init(void);
  extern void __exit watchdog_dev_exit(void);

drivers/watchdog/watchdog_dev.c +22 -4

index 56a649e66eb2..055a4e1b4c13 100644
@@ -581,18 +581,35 @@ int watchdog_dev_unregister(struct watchdog_device *wdd)
  	return 0;
  }
 
+static struct class watchdog_class = {
+	.name =		"watchdog",
+	.owner =	THIS_MODULE,
+};
+
  /*
   *	watchdog_dev_init: init dev part of watchdog core
   *
   *	Allocate a range of chardev nodes to use for watchdog devices
   */
 
-int __init watchdog_dev_init(void)
+struct class * __init watchdog_dev_init(void)
  {
-	int err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
-	if (err < 0)
+	int err;
+
+	err = class_register(&watchdog_class);
+	if (err < 0) {
+		pr_err("couldn't register class\n");
+		return ERR_PTR(err);
+	}
+
+	err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
+	if (err < 0) {
  		pr_err("watchdog: unable to allocate char dev region\n");
-	return err;
+		class_unregister(&watchdog_class);
+		return ERR_PTR(err);
+	}
+
+	return &watchdog_class;
  }
 
  /*

@@ -604,4 +621,5 @@ int __init watchdog_dev_init(void)
  void __exit watchdog_dev_exit(void)
  {
  	unregister_chrdev_region(watchdog_devt, MAX_DOGS);
+	class_unregister(&watchdog_class);
  }

Quoted 6 lines · Pratyush Anand

We need few sysfs attributes to know different status of a watchdog device. To do that, we need to associate .dev_groups with watchdog_class. So convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device attributes defined in that file can be attached to it.

Quoted 3 lines

Signed-off-by Pratyush Anand <[email protected]>

Suggested-by Guenter Roeck <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

As things evolve, I'd by now prefer to move the call to device_create() into watchdog_dev.c, but that can wait for a follow-up patch if Wim is ok with this series.

Quoted 11 lines · Guenter Roeck

Quoted 9 lines · Pratyush Anand

We need few sysfs attributes to know different status of a watchdog device. To do that, we need to associate .dev_groups with watchdog_class. So convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device attributes defined in that file can be attached to it.

Signed-off-by Pratyush Anand <[email protected]>

Suggested-by Guenter Roeck <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

Quoted 3 lines

As things evolve, I'd by now prefer to move the call to device_create() into watchdog_dev.c, but that can wait for a follow-up patch if Wim is ok with this series.

Thanks for your quick review.

OK. I will wait for Wim's comment and then may be I will send another version with your comment for patch 1/1 incorporated.

Quoted 11 lines · Guenter Roeck

Quoted 9 lines · Pratyush Anand

We need few sysfs attributes to know different status of a watchdog device. To do that, we need to associate .dev_groups with watchdog_class. So convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device attributes defined in that file can be attached to it.

Signed-off-by Pratyush Anand <[email protected]>

Suggested-by Guenter Roeck <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

Quoted 3 lines

As things evolve, I'd by now prefer to move the call to device_create() into watchdog_dev.c, but that can wait for a follow-up patch if Wim is ok with this series.

Thanks for your quick review.

OK. I will wait for Wim's comment and then may be I will send another version with your comment for patch 1/1 incorporated.

Quoted 18 lines · Pratyush Anand

Quoted 7 lines · Guenter Roeck

Quoted 6 lines · Pratyush Anand

We need few sysfs attributes to know different status of a watchdog device. To do that, we need to associate .dev_groups with watchdog_class. So convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device attributes defined in that file can be attached to it.

Quoted 7 lines

Quoted 3 lines

Signed-off-by Pratyush Anand <[email protected]>

Suggested-by Guenter Roeck <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

As things evolve, I'd by now prefer to move the call to device_create() into watchdog_dev.c, but that can wait for a follow-up patch if Wim is ok with this series.

Thanks for your quick review.

Quoted 3 lines

OK. I will wait for Wim's comment and then may be I will send another version with your comment for patch 1/1 incorporated.

Thinking about it, my comment is really minor and should not hold up the series from going forward. So let's just skip another version unless Wim or someone else has any comments.

This patch adds following attributes to watchdog device's sysfs interface to read its different status.

* state - reads whether device is active or not
* identity - reads Watchdog device's identity string.
* timeout - reads current timeout.
* timeleft - reads timeleft before watchdog generates a reset
* bootstatus - reads status of the watchdog device at boot
* status - reads watchdog device's internal status bits
* nowayout - reads whether nowayout feature was set or not

Testing with iTCO_wdt:

  # cd /sys/class/watchdog/watchdog1/
  # ls
bootstatus  dev  device  identity  nowayout  power  state
subsystem  timeleft  timeout  uevent
  # cat identity
iTCO_wdt
  # cat timeout
30
  # cat state
inactive
  # echo > /dev/watchdog1
  # cat timeleft
26
  # cat state
active
  # cat bootstatus
0
  # cat nowayout
0

Signed-off-by Pratyush Anand <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

---
  Documentation/ABI/testing/sysfs-class-watchdog |  51 +++++++++++
  drivers/watchdog/Kconfig                       |   7 ++
  drivers/watchdog/watchdog_core.c               |   2 +-
  drivers/watchdog/watchdog_dev.c                | 114 +++++++++++++++++++++++++
  4 files changed, 173 insertions(+), 1 deletion(-)

create mode 100644 Documentation/ABI/testing/sysfs-class-watchdog

Documentation/ABI/testing/sysfs-class-watchdog new file +51 -0

new file mode 100644
index 000000000000..736046b33040
@@ -0,0 +1,51 @@
+What:		/sys/class/watchdog/watchdogn/bootstatus
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It contains status of the watchdog
+		device at boot. It is equivalent to WDIOC_GETBOOTSTATUS of
+		ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/identity
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It contains identity string of
+		watchdog device.
+
+What:		/sys/class/watchdog/watchdogn/nowayout
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. While reading, it gives '1' if that
+		device supports nowayout feature else, it gives '0'.
+
+What:		/sys/class/watchdog/watchdogn/state
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It gives active/inactive status of
+		watchdog device.
+
+What:		/sys/class/watchdog/watchdogn/status
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It contains watchdog device's
+		internal status bits. It is equivalent to WDIOC_GETSTATUS
+		of ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/timeleft
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It contains value of time left for
+		reset generation. It is equivalent to WDIOC_GETTIMELEFT of
+		ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/timeout
+Date:		August 2015
+Contact:	Wim Van Sebroeck <[email protected]>
+Description:
+		It is a read only file. It is read to know about current
+		value of timeout programmed.

drivers/watchdog/Kconfig +7 -0

index 1c427beffadd..71e47dde7d4a 100644
@@ -46,6 +46,13 @@ config WATCHDOG_NOWAYOUT
  	  get killed. If you say Y here, the watchdog cannot be stopped once
  	  it has been started.
 
+config WATCHDOG_SYSFS
+	bool "Read different watchdog information through sysfs"
+	default n
+	help
+	  Say Y here if you want to enable watchdog device status read through
+	  sysfs attributes.
+
  #
  # General Watchdog drivers
  #

drivers/watchdog/watchdog_core.c +1 -1

index a55e846eec79..62f4496b7975 100644
@@ -194,7 +194,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 
  	devno = wdd->cdev.dev;
  	wdd->dev = device_create(watchdog_class, wdd->parent, devno,
-					NULL, "watchdog%d", wdd->id);
+					wdd, "watchdog%d", wdd->id);
  	if (IS_ERR(wdd->dev)) {
  		watchdog_dev_unregister(wdd);
  		ida_simple_remove(&watchdog_ida, id);

drivers/watchdog/watchdog_dev.c +99 -0

index 055a4e1b4c13..d93118e97d11 100644
@@ -247,6 +247,117 @@ out_timeleft:
  	return err;
  }
 
+#ifdef CONFIG_WATCHDOG_SYSFS
+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 ssize_t status_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	ssize_t status;
+	unsigned int val;
+
+	status = watchdog_get_status(wdd, &val);
+	if (!status)
+		status = sprintf(buf, "%u\n", val);
+
+	return status;
+}
+static DEVICE_ATTR_RO(status);
+
+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 timeleft_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	ssize_t status;
+	unsigned int val;
+
+	status = watchdog_get_timeleft(wdd, &val);
+	if (!status)
+		status = sprintf(buf, "%u\n", val);
+
+	return status;
+}
+static DEVICE_ATTR_RO(timeleft);
+
+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 umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
+				int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_status.attr && !wdd->ops->status)
+		mode = 0;
+	else if (attr == &dev_attr_timeleft.attr && !wdd->ops->get_timeleft)
+		mode = 0;
+
+	return mode;
+}
+static struct attribute *wdt_attrs[] = {
+	&dev_attr_state.attr,
+	&dev_attr_identity.attr,
+	&dev_attr_timeout.attr,
+	&dev_attr_timeleft.attr,
+	&dev_attr_bootstatus.attr,

drivers/watchdog/watchdog_dev.c +5 -0

+#endif
+
  /*
   *	watchdog_ioctl_op: call the watchdog drivers ioctl op if defined
   *	@wdd: the watchdog device to do the ioctl on

@@ -584,6 +695,9 @@ int watchdog_dev_unregister(struct watchdog_device *wdd)
  static struct class watchdog_class = {
  	.name =		"watchdog",
  	.owner =	THIS_MODULE,
+#ifdef CONFIG_WATCHDOG_SYSFS
+	.dev_groups =	wdt_groups,
+#endif
  };
 
  /*

These patches provide support to read different watchdog device status through sysfs interface.

Changes since V4:
* All sysfs code has been protected under CONFIG_WATCHDOG_SYSFS

Changes since V3:
* Added Reviewed by tag
* Corrected a checkpatch warning

Changes since V2:
* Used static struct class watchdog_class in stead of pointer. It helped to keep using device_create().
* Above logic was moved to a separate patch. Changed subject line of other patch to look more relevant

Changes since V1(RFC):
* Removed keepalive and start ABI
* timeout is read only now
* state returns text
* only supported ABI visible
* ABI contact changed to MAINTAINER
* unnecessary mutex removed
* aligned continuation with '('
* unnecessary initialization of status (= 0) corrected
* unnecessary else removed
* used __ATTRIBUTE_GROUPS
* removed watchdog_device_create and added functionality in
* watchdog_dev_register.
* optimized nowayout_show
* Now no -EOPNOTSUPP return for timeout read in case of
* wdd->timeout = 0.

Pratyush Anand (2):
watchdog: Use static struct class watchdog_class in stead of pointer
watchdog: Read device status through sysfs attributes

 Documentation/ABI/testing/sysfs-class-watchdog |  51 +++++++++
 drivers/watchdog/Kconfig                       |   7 ++
 drivers/watchdog/watchdog_core.c               |  17 +--
 drivers/watchdog/watchdog_core.h               |   2 +-
 drivers/watchdog/watchdog_dev.c                | 140 ++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-watchdog

This patchset has been added to linux-watchdog-next.
The second patch has been altered as suggested by Guenter (with the #else ; #define wdt_groups NULL statements).