Re: [RFC] watchdog: Add watchdog device control through sysfs attributes

[Pratyush Anand <panand@redhat.com>]

Hi Guenter,
On 30/08/2015:07:58:27 AM, Guenter Roeck wrote:
> 
> I can only see a single caller of watchdog_dev_register().
> Changing that one call should not be such a problem.

Sure..Probably I was sleeping when I greped watchdog_dev_register :(

> 
> Also, you could name the groups variable something like wdc_class_dev_groups,
> make it public, and assign it in watchdog_core.c as follows.

I will avoid making public whereever possible.

> 
> static struct class watchdog_class = {
> 	.name = "watchdog",
> 	.owner = THIS_MODULE,
> 	.dev_groups = wdc_class_dev_groups,
> };
> 
> Then use class_register() and class_unregister() instead of class_create()
> and class_destroy(), and you would be done w/o any further API change.
> 
> >>
> >>>+		return size;
> >>>+}
> >>>+
> >>>+static ssize_t timeout_show(struct device *dev,
> >>>+		struct device_attribute *attr, char *buf)
> >>>+{
> >>>+	struct watchdog_device *wdd = dev_get_drvdata(dev);
> >>>+	ssize_t status;
> >>>+
> >>>+	mutex_lock(&wdd->lock);
> >>>+	if (wdd->timeout == 0)
> >>>+		status = -EOPNOTSUPP;
> >>
> >>Why ?
> >
> >It has been copied from case WDIOC_GETTIMEOUT: which says:
> >timeout == 0 means that we don't know the timeout.
> >
> 
> Just return 0 to mean "the timeout is unknown", and make it part of the
> sysfs ABI. Returning an error just makes user space code more complicated.

Now the new code will check wdd->timeout == 0 in is_visible. So should be fine.
What do you say.?

~Pratyush