2003-02-13 03:31:31

by Rusty Lynch

[permalink] [raw]
Subject: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

The following is a proposal for a new sysfs based watchdog interface
to be used as a replacement for the current char device w/ ioctl api
as described in Documentation/watchdog-api.txt.

Basically, with the help of some watchdog infrastructure code, we could make
each watchdog device register as a platform_device named watchdog, so for
every watchdog on the system there is a /sys/devices/legacy/watchdogN/
directory created for it.

Inside each watchdog's platform device directory, we create a set of files
that corresponds with the IOCTL's used by the existing watchdog drivers.

For example, the first watchdog device registered on the system would add
the following to sysfs ==>

[rusty@penguin2 rusty]$ tree /sys/devices/legacy/watchdog0/
/sys/devices/legacy/watchdog0/
|-- bootstatus
|-- enable
|-- keepalive
|-- name
|-- nowayout
|-- power
|-- start
|-- statistics
|-- status
|-- stop
|-- temperature
|-- temppanic
`-- timeout

[rusty@penguin2 rusty]$ ls -l /sys/devices/legacy/watchdog0/
total 0
-r--r--r-- 1 root root 4096 Feb 12 11:00 bootstatus
-rw-r--r-- 1 root root 4096 Feb 12 11:00 enable
-r-------- 1 root root 4096 Feb 12 11:00 keepalive
-r--r--r-- 1 root root 4096 Feb 12 11:00 name
-rw-r--r-- 1 root root 4096 Feb 12 11:00 nowayout
-rw-r--r-- 1 root root 4096 Feb 12 11:00 power
-r-------- 1 root root 4096 Feb 12 11:00 start
-rw-r--r-- 1 root root 4096 Feb 12 11:00 statistics
-r--r--r-- 1 root root 4096 Feb 12 11:00 status
-rw------- 1 root root 4096 Feb 12 11:00 stop
-r--r--r-- 1 root root 4096 Feb 12 11:00 temperature
-rw-r--r-- 1 root root 4096 Feb 12 11:00 temppanic
-rw-r--r-- 1 root root 4096 Feb 12 11:00 timeout

Where each these files to the following ==>

start (RO)
- show: starts watchdog count

stop (RW)
- store: stops watchdog count, passes input as a magic string
that a driver might use to determine if it watchdog
should really be stopped.
- show: stops the watchdog count

timeout (RW)
- store: expects an integer, changes timeout
- show: prints timeout

keepalive (RO)
- show: pings watchdog, outputs nothing

nowayout (RW)
- store: expects 0 or 1, changes nowayout flag
- show: prints nowayout boolean flag

status (RO)
- show: prints the current status value

bootstatus (RO)
- show: same as 'status', but valid for just after the last reboot.

temperature (RO)
- show: prints temperature in degrees Fahrenheit

enable (RW)
- show: prints 0 or 1 to indicate if the wdt is enabled
- store: expects 0 or 1 to disable or enable the wdt

temppanic (RW)
- show: prints 0 or 1 to indicate if an overtemp triggers a panic
- store: expects 0 or 1 to disable or enable

* The 'statistics' file is an example of the device driver adding additional
files to it's platform directory.

To keep each driver from having to re-implement all the sysfs code, I created
a simple infrastructure where each watchdog driver would only have to fill
in a structure with function pointers for the features the driver implements.

For example, a driver would have the following:

static struct wdt_ops dummy_ops = {
.start = dummy_start,
.stop = dummy_stop,
.keepalive = dummy_keepalive,
.get_timeout = dummy_get_timeout,
.set_timeout = dummy_set_timeout,
.get_nowayout = dummy_get_nowayout,
.set_nowayout = dummy_set_nowayout,
.get_status = dummy_get_status,
.get_caps = dummy_get_caps,
.get_bootstatus = dummy_get_bootstatus,
.get_temperature = dummy_get_temperature,
.get_enable = dummy_get_enable,
.set_enable = dummy_set_enable,
.get_temppanic = dummy_get_temppanic,
.set_temppanic = dummy_set_temppanic,
};
decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);

where wdt_ops and wdt_device are defined as:

struct wdt_device {
struct wdt_ops *ops;
struct platform_device dev;
};

struct wdt_ops {
int (*start)(struct wdt_device *);
int (*stop)(struct wdt_device *, const char *);
int (*keepalive)(struct wdt_device *);
int (*get_timeout)(struct wdt_device *, int *);
int (*set_timeout)(struct wdt_device *, int);
int (*get_nowayout)(struct wdt_device *, int *);
int (*set_nowayout)(struct wdt_device *, int);
int (*get_status)(struct wdt_device *, int *);
int (*get_caps)(struct wdt_device *, int *);
int (*get_bootstatus)(struct wdt_device *, int *);
int (*get_temperature)(struct wdt_device *, int *);
int (*get_enable)(struct wdt_device *, int *);
int (*set_enable)(struct wdt_device *, int);
int (*get_temppanic)(struct wdt_device *, int *);
int (*set_temppanic)(struct wdt_device *, int);
};

The following patch adds the infrastructure to the 2.5.60 kernel.
I am also attaching a dummy watchdog driver file that uses this
infrastructure.

* NOTE: We could also add code to base.c that would enable one of the
registered watchdog devices to also be a misc char device using
the documented watchdog minor number without the driver doing
anything extra.

--rustyl

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1032 -> 1.1033
# drivers/char/watchdog/Makefile 1.5 -> 1.6
# include/linux/watchdog.h 1.4 -> 1.5
# (new) -> 1.1 drivers/char/watchdog/base.c
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/12 [email protected] 1.1033
# Adding a common infrastructure for watchdog timers to implement
# a sysfs interface
# --------------------------------------------
#
diff -Nru a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile Wed Feb 12 18:50:18 2003
+++ b/drivers/char/watchdog/Makefile Wed Feb 12 18:50:18 2003
@@ -7,6 +7,8 @@
# watchdog dies or is 'borrowed' for some reason the software watchdog
# still gives you some cover.

+obj-y += base.o
+
obj-$(CONFIG_PCWATCHDOG) += pcwd.o
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
diff -Nru a/drivers/char/watchdog/base.c b/drivers/char/watchdog/base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/base.c Wed Feb 12 18:50:18 2003
@@ -0,0 +1,327 @@
+/*
+ * base.c
+ *
+ * Base watchdog timer infrastructure
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#ifdef DEBUG
+#define trace(format, args...) \
+ printk(KERN_INFO "%s(" format ")\n", __FUNCTION__ , ## arg)
+#else
+#define trace(format, arg...) do { } while (0)
+#endif
+
+ssize_t start_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->start))
+ return -ENODEV;
+
+ if ((w->ops->start)(w))
+ return -EFAULT;
+
+ return 0;
+}
+DEVICE_ATTR(start,S_IRUSR,start_show,NULL);
+
+ssize_t stop_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, 0))
+ return -EFAULT;
+
+ return 0;
+}
+ssize_t stop_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, buf))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(stop,S_IRUSR|S_IWUSR,stop_show,stop_store);
+
+ssize_t timeout_show(struct device * dev, char * buf)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_timeout))
+ return -ENODEV;
+
+ if((w->ops->get_timeout)(w, &timeout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", timeout);
+}
+ssize_t timeout_store(struct device * dev, const char * buf, size_t count)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_timeout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&timeout))
+ return -EINVAL;
+
+ if ((w->ops->set_timeout)(w, timeout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(timeout,S_IRUGO|S_IWUSR,timeout_show,timeout_store);
+
+ssize_t keepalive_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->keepalive))
+ return -ENODEV;
+
+ if ((w->ops->keepalive)(w))
+ return -EFAULT;
+
+ return 0;
+}
+DEVICE_ATTR(keepalive,S_IRUSR,keepalive_show,NULL);
+
+ssize_t nowayout_show(struct device * dev, char * buf)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_nowayout))
+ return -ENODEV;
+
+ if ((w->ops->get_nowayout)(w, &nowayout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", nowayout);
+}
+ssize_t nowayout_store(struct device * dev, const char * buf, size_t count)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_nowayout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&nowayout))
+ return -EINVAL;
+
+ if ((w->ops->set_nowayout)(w, nowayout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(nowayout,S_IRUGO|S_IWUSR,nowayout_show,nowayout_store);
+
+ssize_t status_show(struct device * dev, char * buf)
+{
+ int status;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_status))
+ return -ENODEV;
+
+ if ((w->ops->get_status)(w, &status))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", status);
+}
+DEVICE_ATTR(status,S_IRUGO,status_show,NULL);
+
+ssize_t bootstatus_show(struct device * dev, char * buf)
+{
+ int bootstatus;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_bootstatus))
+ return -ENODEV;
+
+ if ((w->ops->get_bootstatus)(w, &bootstatus))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", bootstatus);
+}
+DEVICE_ATTR(bootstatus,S_IRUGO,bootstatus_show,NULL);
+
+ssize_t temperature_show(struct device * dev, char * buf)
+{
+ int temperature;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temperature))
+ return -ENODEV;
+
+ if ((w->ops->get_temperature)(w, &temperature))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temperature);
+}
+DEVICE_ATTR(temperature,S_IRUGO,temperature_show,NULL);
+
+ssize_t enable_show(struct device * dev, char * buf)
+{
+ int enable;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_enable))
+ return -ENODEV;
+
+ if ((w->ops->get_enable)(w, &enable))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", enable);
+}
+ssize_t enable_store(struct device * dev, const char * buf, size_t count)
+{
+ int enable;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_enable))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&enable))
+ return -EINVAL;
+
+ if ((w->ops->set_enable)(w, enable))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(enable,S_IRUGO|S_IWUSR,enable_show,enable_store);
+
+ssize_t temppanic_show(struct device * dev, char * buf)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temppanic))
+ return -ENODEV;
+
+ if ((w->ops->get_temppanic)(w, &temppanic))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temppanic);
+}
+ssize_t temppanic_store(struct device * dev, const char * buf, size_t count)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_temppanic))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&temppanic))
+ return -EINVAL;
+
+ if ((w->ops->set_temppanic)(w, temppanic))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(temppanic,S_IRUGO|S_IWUSR,temppanic_show,temppanic_store);
+
+static int wdt_create_default_files(struct wdt_device *d)
+{
+ device_create_file(&(d->dev.dev), &dev_attr_start);
+ device_create_file(&(d->dev.dev), &dev_attr_stop);
+ device_create_file(&(d->dev.dev), &dev_attr_timeout);
+ device_create_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_create_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_create_file(&(d->dev.dev), &dev_attr_status);
+ device_create_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_create_file(&(d->dev.dev), &dev_attr_temperature);
+ device_create_file(&(d->dev.dev), &dev_attr_enable);
+ device_create_file(&(d->dev.dev), &dev_attr_temppanic);
+
+ return 0;
+}
+
+static void wdt_remove_default_files(struct wdt_device *d)
+{
+ device_remove_file(&(d->dev.dev), &dev_attr_start);
+ device_remove_file(&(d->dev.dev), &dev_attr_stop);
+ device_remove_file(&(d->dev.dev), &dev_attr_timeout);
+ device_remove_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_remove_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_remove_file(&(d->dev.dev), &dev_attr_status);
+ device_remove_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_remove_file(&(d->dev.dev), &dev_attr_temperature);
+ device_remove_file(&(d->dev.dev), &dev_attr_enable);
+ device_remove_file(&(d->dev.dev), &dev_attr_temppanic);
+}
+
+int wdt_device_register(struct wdt_device *d)
+{
+ int rv;
+
+ rv = platform_device_register(&(d->dev));
+ if (!rv)
+ rv = wdt_create_default_files(d);
+
+ return rv;
+}
+
+void wdt_device_unregister(struct wdt_device *d)
+{
+ platform_device_unregister(&(d->dev));
+ wdt_remove_default_files(d);
+}
+
+EXPORT_SYMBOL_GPL(wdt_device_register);
+EXPORT_SYMBOL_GPL(wdt_device_unregister);
diff -Nru a/include/linux/watchdog.h b/include/linux/watchdog.h
--- a/include/linux/watchdog.h Wed Feb 12 18:50:18 2003
+++ b/include/linux/watchdog.h Wed Feb 12 18:50:18 2003
@@ -10,9 +10,28 @@
#define _LINUX_WATCHDOG_H

#include <linux/ioctl.h>
+#include <linux/device.h>

#define WATCHDOG_IOCTL_BASE 'W'

+#define decl_wdt_device(_name,_ops,_desc) \
+static struct wdt_device _name##_device = { \
+ .ops = &_ops, \
+ .dev = { \
+ .name = "watchdog", \
+ .id = 0, \
+ .dev = { \
+ .name = __stringify(_desc), \
+ }, \
+ }, \
+}
+
+#define to_pf_device(n) container_of(n, struct platform_device, dev)
+#define to_wdt_device(n) container_of(to_pf_device(n), struct wdt_device, dev)
+
+struct wdt_device;
+struct wdt_ops;
+
struct watchdog_info {
__u32 options; /* Options the card/driver supports */
__u32 firmware_version; /* Firmware version of the card */
@@ -45,5 +64,31 @@
#define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */
#define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */
#define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip */
+
+struct wdt_device {
+ struct wdt_ops *ops;
+ struct platform_device dev;
+};
+
+struct wdt_ops {
+ int (*start)(struct wdt_device *);
+ int (*stop)(struct wdt_device *, const char *);
+ int (*keepalive)(struct wdt_device *);
+ int (*get_timeout)(struct wdt_device *, int *);
+ int (*set_timeout)(struct wdt_device *, int);
+ int (*get_nowayout)(struct wdt_device *, int *);
+ int (*set_nowayout)(struct wdt_device *, int);
+ int (*get_status)(struct wdt_device *, int *);
+ int (*get_caps)(struct wdt_device *, int *);
+ int (*get_bootstatus)(struct wdt_device *, int *);
+ int (*get_temperature)(struct wdt_device *, int *);
+ int (*get_enable)(struct wdt_device *, int *);
+ int (*set_enable)(struct wdt_device *, int);
+ int (*get_temppanic)(struct wdt_device *, int *);
+ int (*set_temppanic)(struct wdt_device *, int);
+};
+
+int wdt_device_register(struct wdt_device *);
+void wdt_device_unregister(struct wdt_device *);

#endif /* ifndef _LINUX_WATCHDOG_H */


Attachments:
dummy.c (5.04 kB)

2003-02-13 04:17:27

by Daniel Pittman

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On 12 Feb 2003, Rusty Lynch wrote:
> The following is a proposal for a new sysfs based watchdog interface
> to be used as a replacement for the current char device w/ ioctl api
> as described in Documentation/watchdog-api.txt.

[...]

> Where each these files to the following ==>
>
> start (RO)
> - show: starts watchdog count

This would be much better as a store -- that way 'cat /.../watchdog0/*'
will not activate the watchdog. A more deliberate action is safer for
forgetful admins, such as me.

[...]

> status (RO)
> - show: prints the current status value
>
> bootstatus (RO)
> - show: same as 'status', but valid for just after the last reboot.

[...]

> enable (RW)
> - show: prints 0 or 1 to indicate if the wdt is enabled
> - store: expects 0 or 1 to disable or enable the wdt

Isn't this the same information as the 'status' and 'start' members?

Daniel

--
A cathedral, a wave of a storm, a dancer's leap,
never turn out to be as high as we had hoped.
-- Marcel Proust

2003-02-13 07:23:10

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Wed, 2003-02-12 at 20:27, Daniel Pittman wrote:
> On 12 Feb 2003, Rusty Lynch wrote:
> > The following is a proposal for a new sysfs based watchdog interface
> > to be used as a replacement for the current char device w/ ioctl api
> > as described in Documentation/watchdog-api.txt.
>
> [...]
>
> > Where each these files to the following ==>
> >
> > start (RO)
> > - show: starts watchdog count
>
> This would be much better as a store -- that way 'cat /.../watchdog0/*'
> will not activate the watchdog. A more deliberate action is safer for
> forgetful admins, such as me.
>

Sounds logical. My reasoning was more of a coin toss.

> [...]
>
> > status (RO)
> > - show: prints the current status value
> >
> > bootstatus (RO)
> > - show: same as 'status', but valid for just after the last reboot.
>
> [...]
>
> > enable (RW)
> > - show: prints 0 or 1 to indicate if the wdt is enabled
> > - store: expects 0 or 1 to disable or enable the wdt
>
> Isn't this the same information as the 'status' and 'start' members?
>
> Daniel

Now that think about it, enable really is the same as start/stop, but
the status is still something different. I didn't really talk about it,
but my intent was to provide the information currently available from
the WDIOC_GETSTATUS ioctl, which is a value composed of the following
flags:

#define WDIOF_OVERHEAT 0x0001 /* Reset due to CPU overheat */
#define WDIOF_FANFAULT 0x0002 /* Fan failed */
#define WDIOF_EXTERN1 0x0004 /* External relay 1 */
#define WDIOF_EXTERN2 0x0008 /* External relay 2 */
#define WDIOF_POWERUNDER 0x0010 /* Power bad/power fault */
#define WDIOF_CARDRESET 0x0020 /* Card previously reset the CPU */
#define WDIOF_POWEROVER 0x0040 /* Power over voltage */
#define WDIOF_SETTIMEOUT 0x0080 /* Set timeout (in seconds) */
#define WDIOF_MAGICCLOSE 0x0100 /* Supports magic close char */
#define WDIOF_KEEPALIVEPING 0x8000 /* Keep alive ping reply */

I debated with myself if each of these flags should be broke out into
their own file, and I'm still not sure if it is better to keep the
status as a single file.

>
> --
> A cathedral, a wave of a storm, a dancer's leap,
> never turn out to be as high as we had hoped.
> -- Marcel Proust
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


2003-02-13 11:51:06

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Wed, Feb 12, 2003 at 07:16:55PM -0800, Rusty Lynch wrote:
> Basically, with the help of some watchdog infrastructure code, we could make
> each watchdog device register as a platform_device named watchdog, so for
> every watchdog on the system there is a /sys/devices/legacy/watchdogN/
> directory created for it.

Why legacy ? That seems an odd place to be putting these.

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2003-02-13 15:45:27

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-13 at 03:55, Dave Jones wrote:
> On Wed, Feb 12, 2003 at 07:16:55PM -0800, Rusty Lynch wrote:
> > Basically, with the help of some watchdog infrastructure code, we could make
> > each watchdog device register as a platform_device named watchdog, so for
> > every watchdog on the system there is a /sys/devices/legacy/watchdogN/
> > directory created for it.
>
> Why legacy ? That seems an odd place to be putting these.
>
> Dave
>
> --
> | Dave Jones. http://www.codemonkey.org.uk
> | SuSE Labs

The watchdogN devices show up under the "legacy" directory because
they are platform devices. From reading the driver-model documentation,
I believe that platform devices are the correct way of categorizing
watchdog devices.

<pasting from Documentation/driver-model/platform.txt>

Platform devices
~~~~~~~~~~~~~~~~
Platform devices are devices that typically appear as autonomous
entities in the system. This includes legacy port-based devices and
host bridges to peripheral buses.


Platform drivers
~~~~~~~~~~~~~~~~
Drivers for platform devices have typically very simple and
unstructured. Either the device was present at a particular I/O port
and the driver was loaded, or there was not. There was no possibility
of hotplugging or alternative discovery besides probing at a specific
I/O address and expecting a specific response.

</pasting from Documentation/driver-model/platform.txt>


2003-02-13 15:59:34

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, Feb 13, 2003 at 07:34:35AM -0800, Rusty Lynch wrote:

> The watchdogN devices show up under the "legacy" directory because
> they are platform devices. From reading the driver-model documentation,
> I believe that platform devices are the correct way of categorizing
> watchdog devices.

My interpretation of legacy devices differs. I believe they are
onboard devices that we've had since just after the dinosaurs died.
FDC controller, dma controller, parport etc..

A plugin watchdog card doesn't fit this description.

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2003-02-13 16:01:05

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs


On 13 Feb 2003, Rusty Lynch wrote:

> On Thu, 2003-02-13 at 03:55, Dave Jones wrote:
> > On Wed, Feb 12, 2003 at 07:16:55PM -0800, Rusty Lynch wrote:
> > > Basically, with the help of some watchdog infrastructure code, we could make
> > > each watchdog device register as a platform_device named watchdog, so for
> > > every watchdog on the system there is a /sys/devices/legacy/watchdogN/
> > > directory created for it.
> >
> > Why legacy ? That seems an odd place to be putting these.
> >
> > Dave
> >
> > --
> > | Dave Jones. http://www.codemonkey.org.uk
> > | SuSE Labs
>
> The watchdogN devices show up under the "legacy" directory because
> they are platform devices. From reading the driver-model documentation,
> I believe that platform devices are the correct way of categorizing
> watchdog devices.
>
> <pasting from Documentation/driver-model/platform.txt>
>
> Platform devices
> ~~~~~~~~~~~~~~~~

You could regard them as 'system' devices, and have them show up in
devices/sys/, which would make more sense than 'legacy'.

-pat

2003-02-13 16:30:12

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-13 at 08:04, Patrick Mochel wrote:
>
> On 13 Feb 2003, Rusty Lynch wrote:
>
> > On Thu, 2003-02-13 at 03:55, Dave Jones wrote:
> > > On Wed, Feb 12, 2003 at 07:16:55PM -0800, Rusty Lynch wrote:
> > > > Basically, with the help of some watchdog infrastructure code, we could make
> > > > each watchdog device register as a platform_device named watchdog, so for
> > > > every watchdog on the system there is a /sys/devices/legacy/watchdogN/
> > > > directory created for it.
> > >
> > > Why legacy ? That seems an odd place to be putting these.
> > >
> > > Dave
> > >
> > > --
> > > | Dave Jones. http://www.codemonkey.org.uk
> > > | SuSE Labs
> >
> > The watchdogN devices show up under the "legacy" directory because
> > they are platform devices. From reading the driver-model documentation,
> > I believe that platform devices are the correct way of categorizing
> > watchdog devices.
> >
> > <pasting from Documentation/driver-model/platform.txt>
> >
> > Platform devices
> > ~~~~~~~~~~~~~~~~
>
> You could regard them as 'system' devices, and have them show up in
> devices/sys/, which would make more sense than 'legacy'.
>
> -pat

Ok, system device is the winner.

-rustyl

2003-02-13 19:05:17

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, Feb 13, 2003 at 07:51:45AM -0800, Rusty Lynch wrote:

> > You could regard them as 'system' devices, and have them show up in
> > devices/sys/, which would make more sense than 'legacy'.
> Ok, system device is the winner.

Why? Stop for a second and look what we have in those dirs.
They both contain things that are essentially motherboard resources.

These are add-on cards we're talking about. Surely a more sensible
place for them to live is somewhere under devices/pci0/ or whatever
bus-type said card is for.

Whilst there are some watchdogs which _are_ part of the motherboard
chipset (which is arguably 'system'), these still show up in PCI
space as regular PCI devices.

Lumping them all into the same category as things like rtc, pic,
fdd etc is just _wrong_.

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2003-02-13 19:13:44

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-13 at 11:07, Dave Jones wrote:
> On Thu, Feb 13, 2003 at 07:51:45AM -0800, Rusty Lynch wrote:
>
> > > You could regard them as 'system' devices, and have them show up in
> > > devices/sys/, which would make more sense than 'legacy'.
> > Ok, system device is the winner.
>
> Why? Stop for a second and look what we have in those dirs.
> They both contain things that are essentially motherboard resources.
>
> These are add-on cards we're talking about. Surely a more sensible
> place for them to live is somewhere under devices/pci0/ or whatever
> bus-type said card is for.
>
> Whilst there are some watchdogs which _are_ part of the motherboard
> chipset (which is arguably 'system'), these still show up in PCI
> space as regular PCI devices.
>
> Lumping them all into the same category as things like rtc, pic,
> fdd etc is just _wrong_.
>
> Dave
>

The thing I would like to see is an easy way for a user space
application to see the available watchdog devices without searching
every possible bus type. If we had that one location to find all
watchdog devices, then each device could just be a symbolic link to the
device in it's real bus.

Maybe the system bus is not right. Is there a right place, or does the
sysfs layout need a new concept? This could apply to any type of
device, not just watchdogs.

--rustyl

2003-02-13 19:03:38

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Here my previous proposal for a new sysfs based watchdog interface
revised to:
* change watchdogs to be system devices instead of platform devices
* remove the redundant 'enable' callbacks/files
* change the start/stop/keepalive files to be write-only

I updated the patch to do meet the revised proposal, and also added
two dummy watchdog drivers just to show how the interface would be used
and what it looks like in sysfs with more then one watchdog.

--rusty

The following is a proposal for a new sysfs based watchdog interface
to be used as a replacement for the current char device w/ ioctl api
as described in Documentation/watchdog-api.txt.

Basically, with the help of some watchdog infrastructure code, we could make
each watchdog device register as a sys_device named watchdog, so for
every watchdog on the system there is a /sys/devices/sys/watchdogN/
directory created for it.

Inside each watchdog's device directory, we create a set of files
that coorisponds with the IOCTL's used by the existing watchdog drivers.

For example, the first watchdog device registered on the system would add
the following to sysfs ==>

[root@penguin2 root]# tree /sys/devices/sys/watchdog0/
/sys/devices/sys/watchdog0/
|-- bootstatus
|-- keepalive
|-- name
|-- nowayout
|-- power
|-- start
|-- statistics
|-- status
|-- stop
|-- temperature
|-- temppanic
`-- timeout

0 directories, 12 files
[root@penguin2 root]# ls -l /sys/devices/sys/watchdog0
total 0
-r--r--r-- 1 root root 4096 Feb 13 02:38 bootstatus
--w------- 1 root root 4096 Feb 13 02:38 keepalive
-r--r--r-- 1 root root 4096 Feb 13 02:38 name
-rw-r--r-- 1 root root 4096 Feb 13 02:38 nowayout
-rw-r--r-- 1 root root 4096 Feb 13 02:38 power
--w------- 1 root root 4096 Feb 13 02:38 start
-rw-r--r-- 1 root root 4096 Feb 13 02:38 statistics
-r--r--r-- 1 root root 4096 Feb 13 02:38 status
--w------- 1 root root 4096 Feb 13 02:38 stop
-r--r--r-- 1 root root 4096 Feb 13 02:38 temperature
-rw-r--r-- 1 root root 4096 Feb 13 02:38 temppanic
-rw-r--r-- 1 root root 4096 Feb 13 02:38 timeout

Where each these files to the following ==>

start (WO)
- store: starts watchdog count, input is ignored

stop (WO)
- store: stops watchdog count, passes input as a magic string
that a driver might use to determine if it watchdog
should really be stopped.

timeout (RW)
- store: expects an integer, changes timeout
- show: prints timeout

keepalive (WO)
- store: pings watchdog, ignores input

nowayout (RW)
- store: expects 0 or 1, changes nowayout flag
- show: prints nowayout boolean flag

status (RO)
- show: prints the current status value

bootstatus (RO)
- show: same as 'status', but valid for just after the last reboot.

temperature (RO)
- show: prints temperature in degrees farenheit

temppanic (RW)
- show: prints 0 or 1 to indicate if an overtemp triggers a panic
- store: expects 0 or 1 to disable or enable

* The 'statistics' file is an example of the device driver adding additional
files to it's device directory.

To keep each driver from having to reimplement all the sysfs code, I created
a simple infrastructure where each watchdog driver would only have to fill
in a structure with function pointers for the features the driver implements.

For example, a driver would have the following:

static struct wdt_ops dummy_ops = {
.start = dummy_start,
.stop = dummy_stop,
.keepalive = dummy_keepalive,
.get_timeout = dummy_get_timeout,
.set_timeout = dummy_set_timeout,
.get_nowayout = dummy_get_nowayout,
.set_nowayout = dummy_set_nowayout,
.get_status = dummy_get_status,
.get_caps = dummy_get_caps,
.get_bootstatus = dummy_get_bootstatus,
.get_temperature = dummy_get_temperature,
.get_temppanic = dummy_get_temppanic,
.set_temppanic = dummy_set_temppanic,
};
decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);

where wdt_ops and wdt_device are defined as:

struct wdt_device {
struct wdt_ops *ops;
struct sys_device dev;
};

struct wdt_ops {
int (*start)(struct wdt_device *);
int (*stop)(struct wdt_device *, const char *);
int (*keepalive)(struct wdt_device *);
int (*get_timeout)(struct wdt_device *, int *);
int (*set_timeout)(struct wdt_device *, int);
int (*get_nowayout)(struct wdt_device *, int *);
int (*set_nowayout)(struct wdt_device *, int);
int (*get_status)(struct wdt_device *, int *);
int (*get_caps)(struct wdt_device *, int *);
int (*get_bootstatus)(struct wdt_device *, int *);
int (*get_temperature)(struct wdt_device *, int *);
int (*get_temppanic)(struct wdt_device *, int *);
int (*set_temppanic)(struct wdt_device *, int);
};

The following patch adds the infrastructure to the 2.5.60 kernel.
I am also attaching a dummy watchdog driver file that uses this
infrastructure.

* NOTE: We could also add code to base.c that would enable one of the
registered watchdog devices to also be a misc char device using
the documented watchdog minor number without the driver doing
anything extra.

--rustyl

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1032 -> 1.1035
# drivers/char/watchdog/Makefile 1.5 -> 1.7
# include/linux/watchdog.h 1.4 -> 1.6
# (new) -> 1.1 drivers/char/watchdog/dummy2.c
# (new) -> 1.1 drivers/char/watchdog/dummy1.c
# (new) -> 1.2 drivers/char/watchdog/base.c
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/12 [email protected] 1.1033
# Adding a common infrastructure for watchdog timers to implement
# a sysfs interface
# --------------------------------------------
# 03/02/13 [email protected] 1.1034
# Moving wdt_device from a platform_device to a sys_device,
# making stop/start/keepalive be write-only, and removing the get_enable
# and set_enable callbacks.
# --------------------------------------------
# 03/02/13 [email protected] 1.1035
# Changes that should have been adde to the last changeset
# --------------------------------------------
#
diff -Nru a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile Thu Feb 13 10:54:28 2003
+++ b/drivers/char/watchdog/Makefile Thu Feb 13 10:54:28 2003
@@ -7,6 +7,8 @@
# watchdog dies or is 'borrowed' for some reason the software watchdog
# still gives you some cover.

+obj-y += base.o
+
obj-$(CONFIG_PCWATCHDOG) += pcwd.o
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
@@ -29,3 +31,5 @@
obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
obj-$(CONFIG_SC1200_WDT) += sc1200wdt.o
obj-$(CONFIG_WAFER_WDT) += wafer5823wdt.o
+obj-y += dummy1.o
+obj-y += dummy2.o
diff -Nru a/drivers/char/watchdog/base.c b/drivers/char/watchdog/base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/base.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,282 @@
+/*
+ * base.c
+ *
+ * Base watchdog timer infrastructure
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#ifdef DEBUG
+#define trace(format, args...) \
+ printk(KERN_INFO "%s(" format ")\n", __FUNCTION__ , ## arg)
+#else
+#define trace(format, arg...) do { } while (0)
+#endif
+
+static int device_count = 0;
+
+ssize_t start_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->start))
+ return -ENODEV;
+
+ if ((w->ops->start)(w))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(start,S_IWUSR,NULL,start_store);
+
+ssize_t stop_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, buf))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(stop,S_IWUSR,NULL,stop_store);
+
+ssize_t timeout_show(struct device * dev, char * buf)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_timeout))
+ return -ENODEV;
+
+ if((w->ops->get_timeout)(w, &timeout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", timeout);
+}
+ssize_t timeout_store(struct device * dev, const char * buf, size_t count)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_timeout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&timeout))
+ return -EINVAL;
+
+ if ((w->ops->set_timeout)(w, timeout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(timeout,S_IRUGO|S_IWUSR,timeout_show,timeout_store);
+
+ssize_t keepalive_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->keepalive))
+ return -ENODEV;
+
+ if ((w->ops->keepalive)(w))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(keepalive,S_IWUSR,NULL,keepalive_store);
+
+ssize_t nowayout_show(struct device * dev, char * buf)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_nowayout))
+ return -ENODEV;
+
+ if ((w->ops->get_nowayout)(w, &nowayout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", nowayout);
+}
+ssize_t nowayout_store(struct device * dev, const char * buf, size_t count)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_nowayout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&nowayout))
+ return -EINVAL;
+
+ if ((w->ops->set_nowayout)(w, nowayout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(nowayout,S_IRUGO|S_IWUSR,nowayout_show,nowayout_store);
+
+ssize_t status_show(struct device * dev, char * buf)
+{
+ int status;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_status))
+ return -ENODEV;
+
+ if ((w->ops->get_status)(w, &status))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", status);
+}
+DEVICE_ATTR(status,S_IRUGO,status_show,NULL);
+
+ssize_t bootstatus_show(struct device * dev, char * buf)
+{
+ int bootstatus;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_bootstatus))
+ return -ENODEV;
+
+ if ((w->ops->get_bootstatus)(w, &bootstatus))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", bootstatus);
+}
+DEVICE_ATTR(bootstatus,S_IRUGO,bootstatus_show,NULL);
+
+ssize_t temperature_show(struct device * dev, char * buf)
+{
+ int temperature;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temperature))
+ return -ENODEV;
+
+ if ((w->ops->get_temperature)(w, &temperature))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temperature);
+}
+DEVICE_ATTR(temperature,S_IRUGO,temperature_show,NULL);
+
+ssize_t temppanic_show(struct device * dev, char * buf)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temppanic))
+ return -ENODEV;
+
+ if ((w->ops->get_temppanic)(w, &temppanic))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temppanic);
+}
+ssize_t temppanic_store(struct device * dev, const char * buf, size_t count)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_temppanic))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&temppanic))
+ return -EINVAL;
+
+ if ((w->ops->set_temppanic)(w, temppanic))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(temppanic,S_IRUGO|S_IWUSR,temppanic_show,temppanic_store);
+
+static int wdt_create_default_files(struct wdt_device *d)
+{
+ device_create_file(&(d->dev.dev), &dev_attr_start);
+ device_create_file(&(d->dev.dev), &dev_attr_stop);
+ device_create_file(&(d->dev.dev), &dev_attr_timeout);
+ device_create_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_create_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_create_file(&(d->dev.dev), &dev_attr_status);
+ device_create_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_create_file(&(d->dev.dev), &dev_attr_temperature);
+ device_create_file(&(d->dev.dev), &dev_attr_temppanic);
+
+ return 0;
+}
+
+static void wdt_remove_default_files(struct wdt_device *d)
+{
+ device_remove_file(&(d->dev.dev), &dev_attr_start);
+ device_remove_file(&(d->dev.dev), &dev_attr_stop);
+ device_remove_file(&(d->dev.dev), &dev_attr_timeout);
+ device_remove_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_remove_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_remove_file(&(d->dev.dev), &dev_attr_status);
+ device_remove_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_remove_file(&(d->dev.dev), &dev_attr_temperature);
+ device_remove_file(&(d->dev.dev), &dev_attr_temppanic);
+}
+
+int wdt_device_register(struct wdt_device *d)
+{
+ int rv;
+
+ d->dev.id = device_count++;
+ rv = sys_device_register(&(d->dev));
+ if (!rv)
+ rv = wdt_create_default_files(d);
+
+ return rv;
+}
+
+void wdt_device_unregister(struct wdt_device *d)
+{
+ sys_device_unregister(&(d->dev));
+ wdt_remove_default_files(d);
+}
+
+EXPORT_SYMBOL_GPL(wdt_device_register);
+EXPORT_SYMBOL_GPL(wdt_device_unregister);
diff -Nru a/drivers/char/watchdog/dummy1.c b/drivers/char/watchdog/dummy1.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy1.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,195 @@
+/*
+ * dummy1.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+static int dummy_start(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_stop(struct wdt_device *d, const char *m)
+{
+ trace("%s, %p", d->dev.name, m);
+ return 0;
+}
+
+static int dummy_keepalive(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_get_timeout(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_set_timeout(struct wdt_device *d, int t)
+{
+ trace("%s, %i", d->dev.name, t);
+
+ return 0;
+}
+
+static int dummy_get_nowayout(struct wdt_device *d, int *n)
+{
+ trace("%s, %p", d->dev.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy_set_nowayout(struct wdt_device *d, int n)
+{
+ trace("%s, %i", d->dev.name, n);
+ return 0;
+}
+
+static int dummy_get_status(struct wdt_device *d, int *s)
+{
+ trace("%s, %p", d->dev.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy_get_caps(struct wdt_device *d, int *c)
+{
+ trace("%s, %p", d->dev.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy_get_bootstatus(struct wdt_device *d, int *b)
+{
+ trace("%s, %p", d->dev.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy_get_temperature(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_get_temppanic(struct wdt_device *d, int *p)
+{
+ trace("%s, %p", d->dev.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy_set_temppanic(struct wdt_device *d, int p)
+{
+ trace("%s, %i", d->dev.name, p);
+ return 0;
+}
+
+/* Additional Sysfs File */
+static ssize_t statistics_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ return sprintf(buf, "%s: dummy statistics data\n", w->dev.name);
+}
+static DEVICE_ATTR(statistics,0644,statistics_show,NULL);
+
+static struct wdt_ops dummy_ops = {
+ .start = dummy_start,
+ .stop = dummy_stop,
+ .keepalive = dummy_keepalive,
+ .get_timeout = dummy_get_timeout,
+ .set_timeout = dummy_set_timeout,
+ .get_nowayout = dummy_get_nowayout,
+ .set_nowayout = dummy_set_nowayout,
+ .get_status = dummy_get_status,
+ .get_caps = dummy_get_caps,
+ .get_bootstatus = dummy_get_bootstatus,
+ .get_temperature = dummy_get_temperature,
+ .get_temppanic = dummy_get_temppanic,
+ .set_temppanic = dummy_set_temppanic,
+};
+decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);
+
+static int __init dummy_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = wdt_device_register(&dummy_device);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ device_create_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ return 0;
+}
+
+static void __exit dummy_exit(void)
+{
+ trace();
+ device_remove_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ wdt_device_unregister(&dummy_device);
+}
+
+module_init(dummy_init);
+module_exit(dummy_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/watchdog/dummy2.c b/drivers/char/watchdog/dummy2.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy2.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,195 @@
+/*
+ * dummy2.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+static int dummy_start(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_stop(struct wdt_device *d, const char *m)
+{
+ trace("%s, %p", d->dev.name, m);
+ return 0;
+}
+
+static int dummy_keepalive(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_get_timeout(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_set_timeout(struct wdt_device *d, int t)
+{
+ trace("%s, %i", d->dev.name, t);
+
+ return 0;
+}
+
+static int dummy_get_nowayout(struct wdt_device *d, int *n)
+{
+ trace("%s, %p", d->dev.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy_set_nowayout(struct wdt_device *d, int n)
+{
+ trace("%s, %i", d->dev.name, n);
+ return 0;
+}
+
+static int dummy_get_status(struct wdt_device *d, int *s)
+{
+ trace("%s, %p", d->dev.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy_get_caps(struct wdt_device *d, int *c)
+{
+ trace("%s, %p", d->dev.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy_get_bootstatus(struct wdt_device *d, int *b)
+{
+ trace("%s, %p", d->dev.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy_get_temperature(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_get_temppanic(struct wdt_device *d, int *p)
+{
+ trace("%s, %p", d->dev.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy_set_temppanic(struct wdt_device *d, int p)
+{
+ trace("%s, %i", d->dev.name, p);
+ return 0;
+}
+
+/* Additional Sysfs File */
+static ssize_t statistics_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ return sprintf(buf, "%s: dummy statistics data\n", w->dev.name);
+}
+static DEVICE_ATTR(statistics,0644,statistics_show,NULL);
+
+static struct wdt_ops dummy_ops = {
+ .start = dummy_start,
+ .stop = dummy_stop,
+ .keepalive = dummy_keepalive,
+ .get_timeout = dummy_get_timeout,
+ .set_timeout = dummy_set_timeout,
+ .get_nowayout = dummy_get_nowayout,
+ .set_nowayout = dummy_set_nowayout,
+ .get_status = dummy_get_status,
+ .get_caps = dummy_get_caps,
+ .get_bootstatus = dummy_get_bootstatus,
+ .get_temperature = dummy_get_temperature,
+ .get_temppanic = dummy_get_temppanic,
+ .set_temppanic = dummy_set_temppanic,
+};
+decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);
+
+static int __init dummy_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = wdt_device_register(&dummy_device);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ device_create_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ return 0;
+}
+
+static void __exit dummy_exit(void)
+{
+ trace();
+ device_remove_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ wdt_device_unregister(&dummy_device);
+}
+
+module_init(dummy_init);
+module_exit(dummy_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/include/linux/watchdog.h b/include/linux/watchdog.h
--- a/include/linux/watchdog.h Thu Feb 13 10:54:28 2003
+++ b/include/linux/watchdog.h Thu Feb 13 10:54:28 2003
@@ -10,9 +10,28 @@
#define _LINUX_WATCHDOG_H

#include <linux/ioctl.h>
+#include <linux/device.h>

#define WATCHDOG_IOCTL_BASE 'W'

+#define decl_wdt_device(_name,_ops,_desc) \
+static struct wdt_device _name##_device = { \
+ .ops = &_ops, \
+ .dev = { \
+ .name = "watchdog", \
+ .id = 0, \
+ .dev = { \
+ .name = __stringify(_desc), \
+ }, \
+ }, \
+}
+
+#define to_pf_device(n) container_of(n, struct sys_device, dev)
+#define to_wdt_device(n) container_of(to_pf_device(n), struct wdt_device, dev)
+
+struct wdt_device;
+struct wdt_ops;
+
struct watchdog_info {
__u32 options; /* Options the card/driver supports */
__u32 firmware_version; /* Firmware version of the card */
@@ -45,5 +64,29 @@
#define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */
#define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */
#define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip */
+
+struct wdt_device {
+ struct wdt_ops *ops;
+ struct sys_device dev;
+};
+
+struct wdt_ops {
+ int (*start)(struct wdt_device *);
+ int (*stop)(struct wdt_device *, const char *);
+ int (*keepalive)(struct wdt_device *);
+ int (*get_timeout)(struct wdt_device *, int *);
+ int (*set_timeout)(struct wdt_device *, int);
+ int (*get_nowayout)(struct wdt_device *, int *);
+ int (*set_nowayout)(struct wdt_device *, int);
+ int (*get_status)(struct wdt_device *, int *);
+ int (*get_caps)(struct wdt_device *, int *);
+ int (*get_bootstatus)(struct wdt_device *, int *);
+ int (*get_temperature)(struct wdt_device *, int *);
+ int (*get_temppanic)(struct wdt_device *, int *);
+ int (*set_temppanic)(struct wdt_device *, int);
+};
+
+int wdt_device_register(struct wdt_device *);
+void wdt_device_unregister(struct wdt_device *);

#endif /* ifndef _LINUX_WATCHDOG_H */

2003-02-13 19:17:58

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs


On 13 Feb 2003, Rusty Lynch wrote:

> On Thu, 2003-02-13 at 11:07, Dave Jones wrote:
> > On Thu, Feb 13, 2003 at 07:51:45AM -0800, Rusty Lynch wrote:
> >
> > > > You could regard them as 'system' devices, and have them show up in
> > > > devices/sys/, which would make more sense than 'legacy'.
> > > Ok, system device is the winner.
> >
> > Why? Stop for a second and look what we have in those dirs.
> > They both contain things that are essentially motherboard resources.
> >
> > These are add-on cards we're talking about. Surely a more sensible
> > place for them to live is somewhere under devices/pci0/ or whatever
> > bus-type said card is for.
> >
> > Whilst there are some watchdogs which _are_ part of the motherboard
> > chipset (which is arguably 'system'), these still show up in PCI
> > space as regular PCI devices.
> >
> > Lumping them all into the same category as things like rtc, pic,
> > fdd etc is just _wrong_.
> >
> > Dave
> >
>
> The thing I would like to see is an easy way for a user space
> application to see the available watchdog devices without searching
> every possible bus type. If we had that one location to find all
> watchdog devices, then each device could just be a symbolic link to the
> device in it's real bus.

Create a watchdog timer class. That will contain all watchdog timers, no
matter what bus they are on.

I apologize for leading you astray with suggesting you treat them as
system devices; I was under the assumption they were more important. :)
They should always be in the most accurate place in the tree. Don't worry
about what the user sees; consistency and accuracy are more important..

-pat

2003-02-13 19:23:17

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Here my previous proposal for a new sysfs based watchdog interface
revised to:
* change watchdogs to be system devices instead of platform devices
* remove the redundant 'enable' callbacks/files
* change the start/stop/keepalive files to be write-only

I updated the patch to do meet the revised proposal, and also added
two dummy watchdog drivers just to show how the interface would be used
and what it looks like in sysfs with more then one watchdog.

--rusty

The following is a proposal for a new sysfs based watchdog interface
to be used as a replacement for the current char device w/ ioctl api
as described in Documentation/watchdog-api.txt.

Basically, with the help of some watchdog infrastructure code, we could make
each watchdog device register as a sys_device named watchdog, so for
every watchdog on the system there is a /sys/devices/sys/watchdogN/
directory created for it.

Inside each watchdog's device directory, we create a set of files
that coorisponds with the IOCTL's used by the existing watchdog drivers.

For example, the first watchdog device registered on the system would add
the following to sysfs ==>

[root@penguin2 root]# tree /sys/devices/sys/watchdog0/
/sys/devices/sys/watchdog0/
|-- bootstatus
|-- keepalive
|-- name
|-- nowayout
|-- power
|-- start
|-- statistics
|-- status
|-- stop
|-- temperature
|-- temppanic
`-- timeout

0 directories, 12 files
[root@penguin2 root]# ls -l /sys/devices/sys/watchdog0
total 0
-r--r--r-- 1 root root 4096 Feb 13 02:38 bootstatus
--w------- 1 root root 4096 Feb 13 02:38 keepalive
-r--r--r-- 1 root root 4096 Feb 13 02:38 name
-rw-r--r-- 1 root root 4096 Feb 13 02:38 nowayout
-rw-r--r-- 1 root root 4096 Feb 13 02:38 power
--w------- 1 root root 4096 Feb 13 02:38 start
-rw-r--r-- 1 root root 4096 Feb 13 02:38 statistics
-r--r--r-- 1 root root 4096 Feb 13 02:38 status
--w------- 1 root root 4096 Feb 13 02:38 stop
-r--r--r-- 1 root root 4096 Feb 13 02:38 temperature
-rw-r--r-- 1 root root 4096 Feb 13 02:38 temppanic
-rw-r--r-- 1 root root 4096 Feb 13 02:38 timeout

Where each these files to the following ==>

start (WO)
- store: starts watchdog count, input is ignored

stop (WO)
- store: stops watchdog count, passes input as a magic string
that a driver might use to determine if it watchdog
should really be stopped.

timeout (RW)
- store: expects an integer, changes timeout
- show: prints timeout

keepalive (WO)
- store: pings watchdog, ignores input

nowayout (RW)
- store: expects 0 or 1, changes nowayout flag
- show: prints nowayout boolean flag

status (RO)
- show: prints the current status value

bootstatus (RO)
- show: same as 'status', but valid for just after the last reboot.

temperature (RO)
- show: prints temperature in degrees farenheit

temppanic (RW)
- show: prints 0 or 1 to indicate if an overtemp triggers a panic
- store: expects 0 or 1 to disable or enable

* The 'statistics' file is an example of the device driver adding additional
files to it's device directory.

To keep each driver from having to reimplement all the sysfs code, I created
a simple infrastructure where each watchdog driver would only have to fill
in a structure with function pointers for the features the driver implements.

For example, a driver would have the following:

static struct wdt_ops dummy_ops = {
.start = dummy_start,
.stop = dummy_stop,
.keepalive = dummy_keepalive,
.get_timeout = dummy_get_timeout,
.set_timeout = dummy_set_timeout,
.get_nowayout = dummy_get_nowayout,
.set_nowayout = dummy_set_nowayout,
.get_status = dummy_get_status,
.get_caps = dummy_get_caps,
.get_bootstatus = dummy_get_bootstatus,
.get_temperature = dummy_get_temperature,
.get_temppanic = dummy_get_temppanic,
.set_temppanic = dummy_set_temppanic,
};
decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);

where wdt_ops and wdt_device are defined as:

struct wdt_device {
struct wdt_ops *ops;
struct sys_device dev;
};

struct wdt_ops {
int (*start)(struct wdt_device *);
int (*stop)(struct wdt_device *, const char *);
int (*keepalive)(struct wdt_device *);
int (*get_timeout)(struct wdt_device *, int *);
int (*set_timeout)(struct wdt_device *, int);
int (*get_nowayout)(struct wdt_device *, int *);
int (*set_nowayout)(struct wdt_device *, int);
int (*get_status)(struct wdt_device *, int *);
int (*get_caps)(struct wdt_device *, int *);
int (*get_bootstatus)(struct wdt_device *, int *);
int (*get_temperature)(struct wdt_device *, int *);
int (*get_temppanic)(struct wdt_device *, int *);
int (*set_temppanic)(struct wdt_device *, int);
};

The following patch adds the infrastructure to the 2.5.60 kernel.
I am also attaching a dummy watchdog driver file that uses this
infrastructure.

* NOTE: We could also add code to base.c that would enable one of the
registered watchdog devices to also be a misc char device using
the documented watchdog minor number without the driver doing
anything extra.

--rustyl

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1032 -> 1.1035
# drivers/char/watchdog/Makefile 1.5 -> 1.7
# include/linux/watchdog.h 1.4 -> 1.6
# (new) -> 1.1 drivers/char/watchdog/dummy2.c
# (new) -> 1.1 drivers/char/watchdog/dummy1.c
# (new) -> 1.2 drivers/char/watchdog/base.c
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/12 [email protected] 1.1033
# Adding a common infrastructure for watchdog timers to implement
# a sysfs interface
# --------------------------------------------
# 03/02/13 [email protected] 1.1034
# Moving wdt_device from a platform_device to a sys_device,
# making stop/start/keepalive be write-only, and removing the get_enable
# and set_enable callbacks.
# --------------------------------------------
# 03/02/13 [email protected] 1.1035
# Changes that should have been adde to the last changeset
# --------------------------------------------
#
diff -Nru a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile Thu Feb 13 10:54:28 2003
+++ b/drivers/char/watchdog/Makefile Thu Feb 13 10:54:28 2003
@@ -7,6 +7,8 @@
# watchdog dies or is 'borrowed' for some reason the software watchdog
# still gives you some cover.

+obj-y += base.o
+
obj-$(CONFIG_PCWATCHDOG) += pcwd.o
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
@@ -29,3 +31,5 @@
obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
obj-$(CONFIG_SC1200_WDT) += sc1200wdt.o
obj-$(CONFIG_WAFER_WDT) += wafer5823wdt.o
+obj-y += dummy1.o
+obj-y += dummy2.o
diff -Nru a/drivers/char/watchdog/base.c b/drivers/char/watchdog/base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/base.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,282 @@
+/*
+ * base.c
+ *
+ * Base watchdog timer infrastructure
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#ifdef DEBUG
+#define trace(format, args...) \
+ printk(KERN_INFO "%s(" format ")\n", __FUNCTION__ , ## arg)
+#else
+#define trace(format, arg...) do { } while (0)
+#endif
+
+static int device_count = 0;
+
+ssize_t start_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->start))
+ return -ENODEV;
+
+ if ((w->ops->start)(w))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(start,S_IWUSR,NULL,start_store);
+
+ssize_t stop_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, buf))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(stop,S_IWUSR,NULL,stop_store);
+
+ssize_t timeout_show(struct device * dev, char * buf)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_timeout))
+ return -ENODEV;
+
+ if((w->ops->get_timeout)(w, &timeout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", timeout);
+}
+ssize_t timeout_store(struct device * dev, const char * buf, size_t count)
+{
+ int timeout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_timeout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&timeout))
+ return -EINVAL;
+
+ if ((w->ops->set_timeout)(w, timeout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(timeout,S_IRUGO|S_IWUSR,timeout_show,timeout_store);
+
+ssize_t keepalive_store(struct device * dev, const char * buf, size_t count)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->keepalive))
+ return -ENODEV;
+
+ if ((w->ops->keepalive)(w))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(keepalive,S_IWUSR,NULL,keepalive_store);
+
+ssize_t nowayout_show(struct device * dev, char * buf)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_nowayout))
+ return -ENODEV;
+
+ if ((w->ops->get_nowayout)(w, &nowayout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", nowayout);
+}
+ssize_t nowayout_store(struct device * dev, const char * buf, size_t count)
+{
+ int nowayout;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_nowayout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&nowayout))
+ return -EINVAL;
+
+ if ((w->ops->set_nowayout)(w, nowayout))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(nowayout,S_IRUGO|S_IWUSR,nowayout_show,nowayout_store);
+
+ssize_t status_show(struct device * dev, char * buf)
+{
+ int status;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_status))
+ return -ENODEV;
+
+ if ((w->ops->get_status)(w, &status))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", status);
+}
+DEVICE_ATTR(status,S_IRUGO,status_show,NULL);
+
+ssize_t bootstatus_show(struct device * dev, char * buf)
+{
+ int bootstatus;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_bootstatus))
+ return -ENODEV;
+
+ if ((w->ops->get_bootstatus)(w, &bootstatus))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", bootstatus);
+}
+DEVICE_ATTR(bootstatus,S_IRUGO,bootstatus_show,NULL);
+
+ssize_t temperature_show(struct device * dev, char * buf)
+{
+ int temperature;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temperature))
+ return -ENODEV;
+
+ if ((w->ops->get_temperature)(w, &temperature))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temperature);
+}
+DEVICE_ATTR(temperature,S_IRUGO,temperature_show,NULL);
+
+ssize_t temppanic_show(struct device * dev, char * buf)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temppanic))
+ return -ENODEV;
+
+ if ((w->ops->get_temppanic)(w, &temppanic))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temppanic);
+}
+ssize_t temppanic_store(struct device * dev, const char * buf, size_t count)
+{
+ int temppanic;
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_temppanic))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&temppanic))
+ return -EINVAL;
+
+ if ((w->ops->set_temppanic)(w, temppanic))
+ return -EFAULT;
+
+ return count;
+}
+DEVICE_ATTR(temppanic,S_IRUGO|S_IWUSR,temppanic_show,temppanic_store);
+
+static int wdt_create_default_files(struct wdt_device *d)
+{
+ device_create_file(&(d->dev.dev), &dev_attr_start);
+ device_create_file(&(d->dev.dev), &dev_attr_stop);
+ device_create_file(&(d->dev.dev), &dev_attr_timeout);
+ device_create_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_create_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_create_file(&(d->dev.dev), &dev_attr_status);
+ device_create_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_create_file(&(d->dev.dev), &dev_attr_temperature);
+ device_create_file(&(d->dev.dev), &dev_attr_temppanic);
+
+ return 0;
+}
+
+static void wdt_remove_default_files(struct wdt_device *d)
+{
+ device_remove_file(&(d->dev.dev), &dev_attr_start);
+ device_remove_file(&(d->dev.dev), &dev_attr_stop);
+ device_remove_file(&(d->dev.dev), &dev_attr_timeout);
+ device_remove_file(&(d->dev.dev), &dev_attr_keepalive);
+ device_remove_file(&(d->dev.dev), &dev_attr_nowayout);
+ device_remove_file(&(d->dev.dev), &dev_attr_status);
+ device_remove_file(&(d->dev.dev), &dev_attr_bootstatus);
+ device_remove_file(&(d->dev.dev), &dev_attr_temperature);
+ device_remove_file(&(d->dev.dev), &dev_attr_temppanic);
+}
+
+int wdt_device_register(struct wdt_device *d)
+{
+ int rv;
+
+ d->dev.id = device_count++;
+ rv = sys_device_register(&(d->dev));
+ if (!rv)
+ rv = wdt_create_default_files(d);
+
+ return rv;
+}
+
+void wdt_device_unregister(struct wdt_device *d)
+{
+ sys_device_unregister(&(d->dev));
+ wdt_remove_default_files(d);
+}
+
+EXPORT_SYMBOL_GPL(wdt_device_register);
+EXPORT_SYMBOL_GPL(wdt_device_unregister);
diff -Nru a/drivers/char/watchdog/dummy1.c b/drivers/char/watchdog/dummy1.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy1.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,195 @@
+/*
+ * dummy1.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+static int dummy_start(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_stop(struct wdt_device *d, const char *m)
+{
+ trace("%s, %p", d->dev.name, m);
+ return 0;
+}
+
+static int dummy_keepalive(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_get_timeout(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_set_timeout(struct wdt_device *d, int t)
+{
+ trace("%s, %i", d->dev.name, t);
+
+ return 0;
+}
+
+static int dummy_get_nowayout(struct wdt_device *d, int *n)
+{
+ trace("%s, %p", d->dev.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy_set_nowayout(struct wdt_device *d, int n)
+{
+ trace("%s, %i", d->dev.name, n);
+ return 0;
+}
+
+static int dummy_get_status(struct wdt_device *d, int *s)
+{
+ trace("%s, %p", d->dev.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy_get_caps(struct wdt_device *d, int *c)
+{
+ trace("%s, %p", d->dev.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy_get_bootstatus(struct wdt_device *d, int *b)
+{
+ trace("%s, %p", d->dev.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy_get_temperature(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_get_temppanic(struct wdt_device *d, int *p)
+{
+ trace("%s, %p", d->dev.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy_set_temppanic(struct wdt_device *d, int p)
+{
+ trace("%s, %i", d->dev.name, p);
+ return 0;
+}
+
+/* Additional Sysfs File */
+static ssize_t statistics_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ return sprintf(buf, "%s: dummy statistics data\n", w->dev.name);
+}
+static DEVICE_ATTR(statistics,0644,statistics_show,NULL);
+
+static struct wdt_ops dummy_ops = {
+ .start = dummy_start,
+ .stop = dummy_stop,
+ .keepalive = dummy_keepalive,
+ .get_timeout = dummy_get_timeout,
+ .set_timeout = dummy_set_timeout,
+ .get_nowayout = dummy_get_nowayout,
+ .set_nowayout = dummy_set_nowayout,
+ .get_status = dummy_get_status,
+ .get_caps = dummy_get_caps,
+ .get_bootstatus = dummy_get_bootstatus,
+ .get_temperature = dummy_get_temperature,
+ .get_temppanic = dummy_get_temppanic,
+ .set_temppanic = dummy_set_temppanic,
+};
+decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);
+
+static int __init dummy_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = wdt_device_register(&dummy_device);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ device_create_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ return 0;
+}
+
+static void __exit dummy_exit(void)
+{
+ trace();
+ device_remove_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ wdt_device_unregister(&dummy_device);
+}
+
+module_init(dummy_init);
+module_exit(dummy_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/watchdog/dummy2.c b/drivers/char/watchdog/dummy2.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy2.c Thu Feb 13 10:54:28 2003
@@ -0,0 +1,195 @@
+/*
+ * dummy2.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+static int dummy_start(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_stop(struct wdt_device *d, const char *m)
+{
+ trace("%s, %p", d->dev.name, m);
+ return 0;
+}
+
+static int dummy_keepalive(struct wdt_device *d)
+{
+ trace("%s", d->dev.name);
+ return 0;
+}
+
+static int dummy_get_timeout(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_set_timeout(struct wdt_device *d, int t)
+{
+ trace("%s, %i", d->dev.name, t);
+
+ return 0;
+}
+
+static int dummy_get_nowayout(struct wdt_device *d, int *n)
+{
+ trace("%s, %p", d->dev.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy_set_nowayout(struct wdt_device *d, int n)
+{
+ trace("%s, %i", d->dev.name, n);
+ return 0;
+}
+
+static int dummy_get_status(struct wdt_device *d, int *s)
+{
+ trace("%s, %p", d->dev.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy_get_caps(struct wdt_device *d, int *c)
+{
+ trace("%s, %p", d->dev.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy_get_bootstatus(struct wdt_device *d, int *b)
+{
+ trace("%s, %p", d->dev.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy_get_temperature(struct wdt_device *d, int *t)
+{
+ trace("%s, %p", d->dev.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy_get_temppanic(struct wdt_device *d, int *p)
+{
+ trace("%s, %p", d->dev.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy_set_temppanic(struct wdt_device *d, int p)
+{
+ trace("%s, %i", d->dev.name, p);
+ return 0;
+}
+
+/* Additional Sysfs File */
+static ssize_t statistics_show(struct device * dev, char * buf)
+{
+ struct wdt_device *w = to_wdt_device(dev);
+
+ trace("%p, %p", dev, buf);
+ return sprintf(buf, "%s: dummy statistics data\n", w->dev.name);
+}
+static DEVICE_ATTR(statistics,0644,statistics_show,NULL);
+
+static struct wdt_ops dummy_ops = {
+ .start = dummy_start,
+ .stop = dummy_stop,
+ .keepalive = dummy_keepalive,
+ .get_timeout = dummy_get_timeout,
+ .set_timeout = dummy_set_timeout,
+ .get_nowayout = dummy_get_nowayout,
+ .set_nowayout = dummy_set_nowayout,
+ .get_status = dummy_get_status,
+ .get_caps = dummy_get_caps,
+ .get_bootstatus = dummy_get_bootstatus,
+ .get_temperature = dummy_get_temperature,
+ .get_temppanic = dummy_get_temppanic,
+ .set_temppanic = dummy_set_temppanic,
+};
+decl_wdt_device(dummy, dummy_ops, Dummy Watchdog Timer);
+
+static int __init dummy_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = wdt_device_register(&dummy_device);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ device_create_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ return 0;
+}
+
+static void __exit dummy_exit(void)
+{
+ trace();
+ device_remove_file(&dummy_device.dev.dev, &dev_attr_statistics);
+ wdt_device_unregister(&dummy_device);
+}
+
+module_init(dummy_init);
+module_exit(dummy_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/include/linux/watchdog.h b/include/linux/watchdog.h
--- a/include/linux/watchdog.h Thu Feb 13 10:54:28 2003
+++ b/include/linux/watchdog.h Thu Feb 13 10:54:28 2003
@@ -10,9 +10,28 @@
#define _LINUX_WATCHDOG_H

#include <linux/ioctl.h>
+#include <linux/device.h>

#define WATCHDOG_IOCTL_BASE 'W'

+#define decl_wdt_device(_name,_ops,_desc) \
+static struct wdt_device _name##_device = { \
+ .ops = &_ops, \
+ .dev = { \
+ .name = "watchdog", \
+ .id = 0, \
+ .dev = { \
+ .name = __stringify(_desc), \
+ }, \
+ }, \
+}
+
+#define to_pf_device(n) container_of(n, struct sys_device, dev)
+#define to_wdt_device(n) container_of(to_pf_device(n), struct wdt_device, dev)
+
+struct wdt_device;
+struct wdt_ops;
+
struct watchdog_info {
__u32 options; /* Options the card/driver supports */
__u32 firmware_version; /* Firmware version of the card */
@@ -45,5 +64,29 @@
#define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */
#define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */
#define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip */
+
+struct wdt_device {
+ struct wdt_ops *ops;
+ struct sys_device dev;
+};
+
+struct wdt_ops {
+ int (*start)(struct wdt_device *);
+ int (*stop)(struct wdt_device *, const char *);
+ int (*keepalive)(struct wdt_device *);
+ int (*get_timeout)(struct wdt_device *, int *);
+ int (*set_timeout)(struct wdt_device *, int);
+ int (*get_nowayout)(struct wdt_device *, int *);
+ int (*set_nowayout)(struct wdt_device *, int);
+ int (*get_status)(struct wdt_device *, int *);
+ int (*get_caps)(struct wdt_device *, int *);
+ int (*get_bootstatus)(struct wdt_device *, int *);
+ int (*get_temperature)(struct wdt_device *, int *);
+ int (*get_temppanic)(struct wdt_device *, int *);
+ int (*set_temppanic)(struct wdt_device *, int);
+};
+
+int wdt_device_register(struct wdt_device *);
+void wdt_device_unregister(struct wdt_device *);

#endif /* ifndef _LINUX_WATCHDOG_H */

2003-02-13 21:02:43

by Scott Murray

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 13 Feb 2003, Patrick Mochel wrote:
>
[snip]
> Create a watchdog timer class. That will contain all watchdog timers, no
> matter what bus they are on.
>
> I apologize for leading you astray with suggesting you treat them as
> system devices; I was under the assumption they were more important. :)
> They should always be in the most accurate place in the tree. Don't worry
> about what the user sees; consistency and accuracy are more important..

I like this idea, since it means my init scripts wouldn't have to dig
around looking for watchdog directories/files on various flavours of cPCI
CPU cards. :)

Scott


--
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: [email protected]


2003-02-13 22:48:40

by Matt Porter

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, Feb 13, 2003 at 04:12:28PM -0500, Scott Murray wrote:
> On Thu, 13 Feb 2003, Patrick Mochel wrote:
> >
> [snip]
> > Create a watchdog timer class. That will contain all watchdog timers, no
> > matter what bus they are on.
> >
> > I apologize for leading you astray with suggesting you treat them as
> > system devices; I was under the assumption they were more important. :)
> > They should always be in the most accurate place in the tree. Don't worry
> > about what the user sees; consistency and accuracy are more important..
>
> I like this idea, since it means my init scripts wouldn't have to dig
> around looking for watchdog directories/files on various flavours of cPCI
> CPU cards. :)

Yes, and on embedded SoC devices we have watchdog facilities sitting
on an internal chip bus. It would be nice to find access points in
a uniform place on any Linux system. i.e. PCI watchdog on my x86 desktop
is in the same place as the on-chip watchdog on my PPC44x system.

IMHO, anything else would be a logical step backwards from accessing
/dev/watchdog across platforms.

Regards,
--
Matt Porter
[email protected]
This is Linux Country. On a quiet night, you can hear Windows reboot.

2003-02-13 23:04:03

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-13 at 14:58, Matt Porter wrote:
> On Thu, Feb 13, 2003 at 04:12:28PM -0500, Scott Murray wrote:
> > On Thu, 13 Feb 2003, Patrick Mochel wrote:
> > >
> > [snip]
> > > Create a watchdog timer class. That will contain all watchdog timers, no
> > > matter what bus they are on.
> > >
> > > I apologize for leading you astray with suggesting you treat them as
> > > system devices; I was under the assumption they were more important. :)
> > > They should always be in the most accurate place in the tree. Don't worry
> > > about what the user sees; consistency and accuracy are more important..
> >
> > I like this idea, since it means my init scripts wouldn't have to dig
> > around looking for watchdog directories/files on various flavours of cPCI
> > CPU cards. :)
>
> Yes, and on embedded SoC devices we have watchdog facilities sitting
> on an internal chip bus. It would be nice to find access points in
> a uniform place on any Linux system. i.e. PCI watchdog on my x86 desktop
> is in the same place as the on-chip watchdog on my PPC44x system.
>
> IMHO, anything else would be a logical step backwards from accessing
> /dev/watchdog across platforms.
>
> Regards,
> --
> Matt Porter
> [email protected]
> This is Linux Country. On a quiet night, you can hear Windows reboot.

I'm reworking the proposal and the patch to create a new class... but I
need to do some experimenting to make sure I really understand how
classes/interfaces/devices/drivers are suppose to work.

--rustyl

2003-02-14 01:53:23

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Ok, I had to go and read the driver-model documentation a couple of more
times, but after I actually started writing some code it finally started
to make sense.

The following patch implements a new "watchdog_devclass" and a couple of
registration functions:

int watchdog_driver_register(struct watchdog_driver *);
void watchdog_driver_unregister(struct watchdog_driver *);

where ==>

struct watchdog_driver {
struct watchdog_ops *ops;
struct device_driver driver;
};

struct watchdog_ops {
int (*start)(struct watchdog_driver *);
int (*stop)(struct watchdog_driver *, const char *);
int (*keepalive)(struct watchdog_driver *);
int (*get_timeout)(struct watchdog_driver *, int *);
int (*set_timeout)(struct watchdog_driver *, int);
int (*get_nowayout)(struct watchdog_driver *, int *);
int (*set_nowayout)(struct watchdog_driver *, int);
int (*get_status)(struct watchdog_driver *, int *);
int (*get_caps)(struct watchdog_driver *, int *);
int (*get_bootstatus)(struct watchdog_driver *, int *);
int (*get_temperature)(struct watchdog_driver *, int *);
int (*get_temppanic)(struct watchdog_driver *, int *);
int (*set_temppanic)(struct watchdog_driver *, int);
};

So, now instead of artificially making all watchdog drivers popup in
the sys or legacy directories, each driver will be registered with it's
correct bus and will also associate itself with watchdog_devclass.

This is done by ==>

static struct watchdog_ops dummy1_ops = {
.start = dummy1_start,
.stop = dummy1_stop,
.keepalive = dummy1_keepalive,
.get_timeout = dummy1_get_timeout,
.set_timeout = dummy1_set_timeout,
.get_nowayout = dummy1_get_nowayout,
.set_nowayout = dummy1_set_nowayout,
.get_status = dummy1_get_status,
.get_caps = dummy1_get_caps,
.get_bootstatus = dummy1_get_bootstatus,
.get_temperature = dummy1_get_temperature,
.get_temppanic = dummy1_get_temppanic,
.set_temppanic = dummy1_set_temppanic,
};

static struct watchdog_driver dummy1_driver = {
.ops = &dummy1_ops,
.driver = {
.name = "dummy1",
.bus = &ADD_CORRECT_BUS_HERE,
.devclass = &watchdog_devclass,
.probe = dummy1_probe,
.remove = dummy1_remove,
.suspend = dummy1_suspend,
.resume = dummy1_resume,
}
};

And then let the watchdog infrastructure register the driver and
create the sysfs control files by:

watchdog_driver_register(&dummy1_driver);

I created two dummy drivers that I associated with the "system" bus
because they are not really connected to a real bus. This causes the
following to be created in sysfs:

[root@penguin2 root]# tree /sys/class/watchdog/drivers/
/sys/class/watchdog/drivers/
|-- system:dummy1 -> ../../../bus/system/drivers/dummy1
`-- system:dummy2 -> ../../../bus/system/drivers/dummy2

2 directories, 0 files
[root@penguin2 root]# tree /sys/class/watchdog/drivers/system:dummy1/
/sys/class/watchdog/drivers/system:dummy1/
|-- bootstatus
|-- keepalive
|-- nowayout
|-- start
|-- status
|-- stop
|-- temperature
|-- temppanic
`-- timeout

0 directories, 9 files

Here is a new patch that implements this.

--rustyl

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1032 -> 1.1036
# drivers/char/watchdog/Makefile 1.5 -> 1.7
# include/linux/watchdog.h 1.4 -> 1.7
# (new) -> 1.2 drivers/char/watchdog/dummy2.c
# (new) -> 1.2 drivers/char/watchdog/dummy1.c
# (new) -> 1.3 drivers/char/watchdog/base.c
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/12 [email protected] 1.1033
# Adding a common infrastructure for watchdog timers to implement
# a sysfs interface
# --------------------------------------------
# 03/02/13 [email protected] 1.1034
# Moving wdt_device from a platform_device to a sys_device,
# making stop/start/keepalive be write-only, and removing the get_enable
# and set_enable callbacks.
# --------------------------------------------
# 03/02/13 [email protected] 1.1035
# Changes that should have been adde to the last changeset
# --------------------------------------------
# 03/02/13 [email protected] 1.1036
# Reworked the watchdog infrastructure implementation to use the sysfs
# class concept.
# --------------------------------------------
#
diff -Nru a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile Thu Feb 13 17:38:25 2003
+++ b/drivers/char/watchdog/Makefile Thu Feb 13 17:38:25 2003
@@ -7,6 +7,8 @@
# watchdog dies or is 'borrowed' for some reason the software watchdog
# still gives you some cover.

+obj-y += base.o
+
obj-$(CONFIG_PCWATCHDOG) += pcwd.o
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
@@ -29,3 +31,5 @@
obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
obj-$(CONFIG_SC1200_WDT) += sc1200wdt.o
obj-$(CONFIG_WAFER_WDT) += wafer5823wdt.o
+obj-y += dummy1.o
+obj-y += dummy2.o
diff -Nru a/drivers/char/watchdog/base.c b/drivers/char/watchdog/base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/base.c Thu Feb 13 17:38:25 2003
@@ -0,0 +1,309 @@
+/*
+ * base.c
+ *
+ * Base watchdog timer infrastructure
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+
+#ifdef DEBUG
+#define trace(format, args...) \
+ printk(KERN_INFO "%s(" format ")\n", __FUNCTION__ , ## arg)
+#else
+#define trace(format, arg...) do { } while (0)
+#endif
+
+static int device_count = 0;
+
+ssize_t start_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->start))
+ return -ENODEV;
+
+ if ((w->ops->start)(w))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(start,S_IWUSR,NULL,start_store);
+
+ssize_t stop_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, buf))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(stop,S_IWUSR,NULL,stop_store);
+
+ssize_t timeout_show(struct device_driver *d, char *buf)
+{
+ int timeout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_timeout))
+ return -ENODEV;
+
+ if((w->ops->get_timeout)(w, &timeout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", timeout);
+}
+ssize_t timeout_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int timeout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_timeout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&timeout))
+ return -EINVAL;
+
+ if ((w->ops->set_timeout)(w, timeout))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(timeout,S_IRUGO|S_IWUSR,timeout_show,timeout_store);
+
+ssize_t keepalive_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->keepalive))
+ return -ENODEV;
+
+ if ((w->ops->keepalive)(w))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(keepalive,S_IWUSR,NULL,keepalive_store);
+
+ssize_t nowayout_show(struct device_driver *d, char *buf)
+{
+ int nowayout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_nowayout))
+ return -ENODEV;
+
+ if ((w->ops->get_nowayout)(w, &nowayout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", nowayout);
+}
+ssize_t nowayout_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int nowayout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_nowayout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&nowayout))
+ return -EINVAL;
+
+ if ((w->ops->set_nowayout)(w, nowayout))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(nowayout,S_IRUGO|S_IWUSR,nowayout_show,nowayout_store);
+
+ssize_t status_show(struct device_driver *d, char *buf)
+{
+ int status;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_status))
+ return -ENODEV;
+
+ if ((w->ops->get_status)(w, &status))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", status);
+}
+DRIVER_ATTR(status,S_IRUGO,status_show,NULL);
+
+ssize_t bootstatus_show(struct device_driver *d, char *buf)
+{
+ int bootstatus;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_bootstatus))
+ return -ENODEV;
+
+ if ((w->ops->get_bootstatus)(w, &bootstatus))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", bootstatus);
+}
+DRIVER_ATTR(bootstatus,S_IRUGO,bootstatus_show,NULL);
+
+ssize_t temperature_show(struct device_driver *d, char *buf)
+{
+ int temperature;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temperature))
+ return -ENODEV;
+
+ if ((w->ops->get_temperature)(w, &temperature))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temperature);
+}
+DRIVER_ATTR(temperature,S_IRUGO,temperature_show,NULL);
+
+ssize_t temppanic_show(struct device_driver *d, char *buf)
+{
+ int temppanic;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", dev, buf);
+ if (!(w->ops->get_temppanic))
+ return -ENODEV;
+
+ if ((w->ops->get_temppanic)(w, &temppanic))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temppanic);
+}
+ssize_t temppanic_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int temppanic;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", dev, buf, count);
+ if (!(w->ops->set_temppanic))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&temppanic))
+ return -EINVAL;
+
+ if ((w->ops->set_temppanic)(w, temppanic))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(temppanic,S_IRUGO|S_IWUSR,temppanic_show,temppanic_store);
+
+static int watchdog_create_default_files(struct watchdog_driver *d)
+{
+ driver_create_file(&(d->driver), &driver_attr_start);
+ driver_create_file(&(d->driver), &driver_attr_stop);
+ driver_create_file(&(d->driver), &driver_attr_timeout);
+ driver_create_file(&(d->driver), &driver_attr_keepalive);
+ driver_create_file(&(d->driver), &driver_attr_nowayout);
+ driver_create_file(&(d->driver), &driver_attr_status);
+ driver_create_file(&(d->driver), &driver_attr_bootstatus);
+ driver_create_file(&(d->driver), &driver_attr_temperature);
+ driver_create_file(&(d->driver), &driver_attr_temppanic);
+
+ return 0;
+}
+
+static void watchdog_remove_default_files(struct watchdog_driver *d)
+{
+ driver_remove_file(&(d->driver), &driver_attr_start);
+ driver_remove_file(&(d->driver), &driver_attr_stop);
+ driver_remove_file(&(d->driver), &driver_attr_timeout);
+ driver_remove_file(&(d->driver), &driver_attr_keepalive);
+ driver_remove_file(&(d->driver), &driver_attr_nowayout);
+ driver_remove_file(&(d->driver), &driver_attr_status);
+ driver_remove_file(&(d->driver), &driver_attr_bootstatus);
+ driver_remove_file(&(d->driver), &driver_attr_temperature);
+ driver_remove_file(&(d->driver), &driver_attr_temppanic);
+}
+
+int watchdog_driver_register(struct watchdog_driver *d)
+{
+ int rv;
+
+ rv = driver_register(&(d->driver));
+ if (!rv)
+ rv = watchdog_create_default_files(d);
+
+ return rv;
+}
+
+void watchdog_driver_unregister(struct watchdog_driver *d)
+{
+ driver_unregister(&(d->driver));
+ watchdog_remove_default_files(d);
+}
+
+struct device_class watchdog_devclass = {
+ .name = "watchdog",
+};
+
+static int __init watchdog_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = devclass_register(&watchdog_devclass);
+ if (ret)
+ return -ENODEV;
+
+ return 0;
+}
+
+static void __exit watchdog_exit(void)
+{
+ trace();
+ devclass_unregister(&watchdog_devclass);
+}
+
+module_init(watchdog_init);
+module_exit(watchdog_exit);
+EXPORT_SYMBOL_GPL(watchdog_driver_register);
+EXPORT_SYMBOL_GPL(watchdog_driver_unregister);
+EXPORT_SYMBOL_GPL(watchdog_devclass);
diff -Nru a/drivers/char/watchdog/dummy1.c b/drivers/char/watchdog/dummy1.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy1.c Thu Feb 13 17:38:25 2003
@@ -0,0 +1,222 @@
+/*
+ * dummy1.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+
+static int dummy1_start(struct watchdog_driver *d)
+{
+ trace("%s", d->driver.name);
+ return 0;
+}
+
+static int dummy1_stop(struct watchdog_driver *d, const char *m)
+{
+ trace("%s, %p", d->driver.name, m);
+ return 0;
+}
+
+static int dummy1_keepalive(struct watchdog_driver *d)
+{
+ trace("%s", d->driver.name);
+ return 0;
+}
+
+static int dummy1_get_timeout(struct watchdog_driver *d, int *t)
+{
+ trace("%s, %p", d->driver.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy1_set_timeout(struct watchdog_driver *d, int t)
+{
+ trace("%s, %i", d->driver.name, t);
+
+ return 0;
+}
+
+static int dummy1_get_nowayout(struct watchdog_driver *d, int *n)
+{
+ trace("%s, %p", d->driver.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy1_set_nowayout(struct watchdog_driver *d, int n)
+{
+ trace("%s, %i", d->driver.name, n);
+ return 0;
+}
+
+static int dummy1_get_status(struct watchdog_driver *d, int *s)
+{
+ trace("%s, %p", d->driver.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy1_get_caps(struct watchdog_driver *d, int *c)
+{
+ trace("%s, %p", d->driver.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy1_get_bootstatus(struct watchdog_driver *d, int *b)
+{
+ trace("%s, %p", d->driver.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy1_get_temperature(struct watchdog_driver *d, int *t)
+{
+ trace("%s, %p", d->driver.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy1_get_temppanic(struct watchdog_driver *d, int *p)
+{
+ trace("%s, %p", d->driver.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy1_set_temppanic(struct watchdog_driver *d, int p)
+{
+ trace("%s, %i", d->driver.name, p);
+ return 0;
+}
+
+static int dummy1_probe(struct device * dev)
+{
+ trace("%s", dev->name);
+ return 0;
+}
+static int dummy1_remove(struct device * dev)
+{
+ trace("%s", dev->name);
+ return 0;
+}
+static int dummy1_suspend(struct device * dev, u32 state, u32 level)
+{
+ trace("%s, %i, %i", dev->name, state, level);
+ return 0;
+}
+static int dummy1_resume(struct device * dev, u32 level)
+{
+ trace("%s, %i", dev->name, level);
+ return 0;
+}
+
+static void dummy1_release(struct device_driver * drv)
+{
+ trace("%s", drv->name);
+}
+
+static struct watchdog_ops dummy1_ops = {
+ .start = dummy1_start,
+ .stop = dummy1_stop,
+ .keepalive = dummy1_keepalive,
+ .get_timeout = dummy1_get_timeout,
+ .set_timeout = dummy1_set_timeout,
+ .get_nowayout = dummy1_get_nowayout,
+ .set_nowayout = dummy1_set_nowayout,
+ .get_status = dummy1_get_status,
+ .get_caps = dummy1_get_caps,
+ .get_bootstatus = dummy1_get_bootstatus,
+ .get_temperature = dummy1_get_temperature,
+ .get_temppanic = dummy1_get_temppanic,
+ .set_temppanic = dummy1_set_temppanic,
+};
+
+static struct watchdog_driver dummy1_driver = {
+ .ops = &dummy1_ops,
+ .driver = {
+ .name = "dummy1",
+ .bus = &system_bus_type,
+ .devclass = &watchdog_devclass,
+ .probe = dummy1_probe,
+ .remove = dummy1_remove,
+ .suspend = dummy1_suspend,
+ .resume = dummy1_resume,
+ }
+};
+
+static int __init dummy1_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = watchdog_driver_register(&dummy1_driver);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void __exit dummy1_exit(void)
+{
+ trace();
+ watchdog_driver_unregister(&dummy1_driver);
+}
+
+module_init(dummy1_init);
+module_exit(dummy1_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/watchdog/dummy2.c b/drivers/char/watchdog/dummy2.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/dummy2.c Thu Feb 13 17:38:25 2003
@@ -0,0 +1,222 @@
+/*
+ * dummy2.c
+ *
+ * Dummy watchdog timer created for testing base.c
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+#include <asm/io.h>
+
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#define trace(format, arg...) \
+ printk(KERN_INFO "%s(" format ")\n", \
+ __FUNCTION__ , ## arg);
+#define err(format, arg...) \
+ printk(KERN_ERR "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define info(format, arg...) \
+ printk(KERN_INFO "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+#define warn(format, arg...) \
+ printk(KERN_WARNING "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+
+static int dummy2_start(struct watchdog_driver *d)
+{
+ trace("%s", d->driver.name);
+ return 0;
+}
+
+static int dummy2_stop(struct watchdog_driver *d, const char *m)
+{
+ trace("%s, %p", d->driver.name, m);
+ return 0;
+}
+
+static int dummy2_keepalive(struct watchdog_driver *d)
+{
+ trace("%s", d->driver.name);
+ return 0;
+}
+
+static int dummy2_get_timeout(struct watchdog_driver *d, int *t)
+{
+ trace("%s, %p", d->driver.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy2_set_timeout(struct watchdog_driver *d, int t)
+{
+ trace("%s, %i", d->driver.name, t);
+
+ return 0;
+}
+
+static int dummy2_get_nowayout(struct watchdog_driver *d, int *n)
+{
+ trace("%s, %p", d->driver.name, n);
+
+ *n = 911;
+ return 0;
+}
+
+static int dummy2_set_nowayout(struct watchdog_driver *d, int n)
+{
+ trace("%s, %i", d->driver.name, n);
+ return 0;
+}
+
+static int dummy2_get_status(struct watchdog_driver *d, int *s)
+{
+ trace("%s, %p", d->driver.name, s);
+ *s = 911;
+ return 0;
+}
+
+static int dummy2_get_caps(struct watchdog_driver *d, int *c)
+{
+ trace("%s, %p", d->driver.name, c);
+ *c = 911;
+ return 0;
+}
+
+static int dummy2_get_bootstatus(struct watchdog_driver *d, int *b)
+{
+ trace("%s, %p", d->driver.name, b);
+ *b = 911;
+ return 0;
+}
+
+static int dummy2_get_temperature(struct watchdog_driver *d, int *t)
+{
+ trace("%s, %p", d->driver.name, t);
+ *t = 911;
+ return 0;
+}
+
+static int dummy2_get_temppanic(struct watchdog_driver *d, int *p)
+{
+ trace("%s, %p", d->driver.name, p);
+ *p = 911;
+ return 0;
+}
+
+static int dummy2_set_temppanic(struct watchdog_driver *d, int p)
+{
+ trace("%s, %i", d->driver.name, p);
+ return 0;
+}
+
+static int dummy2_probe(struct device * dev)
+{
+ trace("%s", dev->name);
+ return 0;
+}
+static int dummy2_remove(struct device * dev)
+{
+ trace("%s", dev->name);
+ return 0;
+}
+static int dummy2_suspend(struct device * dev, u32 state, u32 level)
+{
+ trace("%s, %i, %i", dev->name, state, level);
+ return 0;
+}
+static int dummy2_resume(struct device * dev, u32 level)
+{
+ trace("%s, %i", dev->name, level);
+ return 0;
+}
+
+static void dummy2_release(struct device_driver * drv)
+{
+ trace("%s", drv->name);
+}
+
+static struct watchdog_ops dummy2_ops = {
+ .start = dummy2_start,
+ .stop = dummy2_stop,
+ .keepalive = dummy2_keepalive,
+ .get_timeout = dummy2_get_timeout,
+ .set_timeout = dummy2_set_timeout,
+ .get_nowayout = dummy2_get_nowayout,
+ .set_nowayout = dummy2_set_nowayout,
+ .get_status = dummy2_get_status,
+ .get_caps = dummy2_get_caps,
+ .get_bootstatus = dummy2_get_bootstatus,
+ .get_temperature = dummy2_get_temperature,
+ .get_temppanic = dummy2_get_temppanic,
+ .set_temppanic = dummy2_set_temppanic,
+};
+
+static struct watchdog_driver dummy2_driver = {
+ .ops = &dummy2_ops,
+ .driver = {
+ .name = "dummy2",
+ .bus = &system_bus_type,
+ .devclass = &watchdog_devclass,
+ .probe = dummy2_probe,
+ .remove = dummy2_remove,
+ .suspend = dummy2_suspend,
+ .resume = dummy2_resume,
+ }
+};
+
+static int __init dummy2_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = watchdog_driver_register(&dummy2_driver);
+ if (ret) {
+ err("failed to register watchdog device");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void __exit dummy2_exit(void)
+{
+ trace();
+ watchdog_driver_unregister(&dummy2_driver);
+}
+
+module_init(dummy2_init);
+module_exit(dummy2_exit);
+MODULE_LICENSE("GPL");
diff -Nru a/include/linux/watchdog.h b/include/linux/watchdog.h
--- a/include/linux/watchdog.h Thu Feb 13 17:38:25 2003
+++ b/include/linux/watchdog.h Thu Feb 13 17:38:25 2003
@@ -10,9 +10,17 @@
#define _LINUX_WATCHDOG_H

#include <linux/ioctl.h>
+#include <linux/device.h>

#define WATCHDOG_IOCTL_BASE 'W'

+#define to_watchdog_driver(n) container_of(n, struct watchdog_driver, driver)
+
+struct watchdog_driver;
+struct watchdog_ops;
+
+extern struct device_class watchdog_devclass;
+
struct watchdog_info {
__u32 options; /* Options the card/driver supports */
__u32 firmware_version; /* Firmware version of the card */
@@ -45,5 +53,29 @@
#define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */
#define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */
#define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip */
+
+struct watchdog_driver {
+ struct watchdog_ops *ops;
+ struct device_driver driver;
+};
+
+struct watchdog_ops {
+ int (*start)(struct watchdog_driver *);
+ int (*stop)(struct watchdog_driver *, const char *);
+ int (*keepalive)(struct watchdog_driver *);
+ int (*get_timeout)(struct watchdog_driver *, int *);
+ int (*set_timeout)(struct watchdog_driver *, int);
+ int (*get_nowayout)(struct watchdog_driver *, int *);
+ int (*set_nowayout)(struct watchdog_driver *, int);
+ int (*get_status)(struct watchdog_driver *, int *);
+ int (*get_caps)(struct watchdog_driver *, int *);
+ int (*get_bootstatus)(struct watchdog_driver *, int *);
+ int (*get_temperature)(struct watchdog_driver *, int *);
+ int (*get_temppanic)(struct watchdog_driver *, int *);
+ int (*set_temppanic)(struct watchdog_driver *, int);
+};
+
+int watchdog_driver_register(struct watchdog_driver *);
+void watchdog_driver_unregister(struct watchdog_driver *);

#endif /* ifndef _LINUX_WATCHDOG_H */

2003-02-14 13:39:13

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 00:47, Rusty Lynch wrote:
> Ok, I had to go and read the driver-model documentation a couple of more
> times, but after I actually started writing some code it finally started
> to make sense.

The watchdog_ops is probably a good thing anyway. If you also use that
same structure with the base watchdog module having the ioctl parser all
the ioctl handling funnies and quirks in the drivers go away except
for driver private stuff.

Two for the price of one

Alan

2003-02-14 13:38:31

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 13 Feb 2003 15:58:17 MST, Matt Porter said:

> Yes, and on embedded SoC devices we have watchdog facilities sitting
> on an internal chip bus. It would be nice to find access points in
> a uniform place on any Linux system. i.e. PCI watchdog on my x86 desktop
> is in the same place as the on-chip watchdog on my PPC44x system.
>
> IMHO, anything else would be a logical step backwards from accessing
> /dev/watchdog across platforms.

I admit not having thoroughly read the source to check - is the userspace API
for accessing all these chips fairly uniform and rational, so that a user
program can be reasonably sure that if stat("/dev/watchdog") returns zero, that
it knows how to deal with it? Or are they all sufficiently close to the "keep
reloading a countdown timer from userspace, and if it ever doesn't get reloaded,
kick the kernel in the seat of the pants" programming model? Of course, even
a disagreement on the units of the timer could be bad - a seconds/milliseconds
clash could result is a *real* fast lack-of-joy situation.. ;)


Attachments:
(No filename) (226.00 B)

2003-02-14 13:47:47

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 13:48, [email protected] wrote:
> I admit not having thoroughly read the source to check - is the userspace API
> for accessing all these chips fairly uniform and rational, so that a user
> program can be reasonably sure that if stat("/dev/watchdog") returns zero, that
> it knows how to deal with it? Or are they all sufficiently close to the "keep
> reloading a countdown timer from userspace, and if it ever doesn't get reloaded,
> kick the kernel in the seat of the pants" programming model? Of course, even
> a disagreement on the units of the timer could be bad - a seconds/milliseconds
> clash could result is a *real* fast lack-of-joy situation.. ;)

watchdog interfaces have a defined API, which they all follow fairly closely. That
makes adding watchdogs as a device class nice and easy

2003-02-14 16:34:58

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 06:48, Alan Cox wrote:
> On Fri, 2003-02-14 at 00:47, Rusty Lynch wrote:
> > Ok, I had to go and read the driver-model documentation a couple of more
> > times, but after I actually started writing some code it finally started
> > to make sense.
>
> The watchdog_ops is probably a good thing anyway. If you also use that
> same structure with the base watchdog module having the ioctl parser all
> the ioctl handling funnies and quirks in the drivers go away except
> for driver private stuff.
>
> Two for the price of one
>
> Alan
>

Since only one driver can register as the /dev/watchdog (ie
major=10/minor=130 char device), would it be better if:

* the first watchdog driver to register with the base also gets
registered as the watchdog misc device, and when that driver unregisters
then the second watchdog to register now gets registered as the misc
device, etc.
* each watchdog driver gets an additional sysfs file named 'misc', where
writing a '1' to the file causes the driver to become the registered
misc watchdog device.
* something else

--rustyl


2003-02-14 16:46:08

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 15:32, Rusty Lynch wrote:
> Since only one driver can register as the /dev/watchdog (ie
> major=10/minor=130 char device), would it be better if:
>
> * the first watchdog driver to register with the base also gets
> registered as the watchdog misc device, and when that driver unregisters
> then the second watchdog to register now gets registered as the misc
> device, etc.
> * each watchdog driver gets an additional sysfs file named 'misc', where
> writing a '1' to the file causes the driver to become the registered
> misc watchdog device.
> * something else

I had hoped we'd get some kind of sanity and 32bit dev_t by now at which
point watchdogs belong on a major with /dev/watchdog0/1/2/3/... I dont
think you need to care about that for now. Sysfs doesn't help here in
the general case as it lacks persistant file permissions, but where it
is used the user can simply make /dev/watchdog a link into sysfs and
nothing has to be done by the driver

Alan

2003-02-14 18:52:58

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 09:55, Alan Cox wrote:
> On Fri, 2003-02-14 at 15:32, Rusty Lynch wrote:
> > Since only one driver can register as the /dev/watchdog (ie
> > major=10/minor=130 char device), would it be better if:
> >
> > * the first watchdog driver to register with the base also gets
> > registered as the watchdog misc device, and when that driver unregisters
> > then the second watchdog to register now gets registered as the misc
> > device, etc.
> > * each watchdog driver gets an additional sysfs file named 'misc', where
> > writing a '1' to the file causes the driver to become the registered
> > misc watchdog device.
> > * something else
>
> I had hoped we'd get some kind of sanity and 32bit dev_t by now at which
> point watchdogs belong on a major with /dev/watchdog0/1/2/3/... I dont
> think you need to care about that for now. Sysfs doesn't help here in
> the general case as it lacks persistant file permissions, but where it
> is used the user can simply make /dev/watchdog a link into sysfs and
> nothing has to be done by the driver
>
> Alan
>

If /dev/watchdog is a link to a sysfs file, then (at least in sysfs's
current state) you loose the ability to handle the documented watchdog
ioctl's. That is why I assumed that the watchdog base.c could implement
a miscdev registered for the watchdog minor (130), and then translate
the documented ioctl's into the watchdog_ops calls for the the specific
driver that is currently associated with the miscdevice.

Or... are you suggesting the ioctl interface is deprecated and
/dev/watchdog/ is a symbolic link to a given watchdog driver directory
in syfs?

--rustyl

2003-02-14 19:33:53

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 19:02, Rusty Lynch wrote:
> If /dev/watchdog is a link to a sysfs file, then (at least in sysfs's
> current state) you loose the ability to handle the documented watchdog
> ioctl's. That is why I assumed that the watchdog base.c could implement
> a miscdev registered for the watchdog minor (130), and then translate
> the documented ioctl's into the watchdog_ops calls for the the specific
> driver that is currently associated with the miscdevice.

sysfs is not capable of handling ioctl forwarding ? Oops, then I guess
the mess has to live on

2003-02-14 20:38:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Hi!

> temperature (RO)
> - show: prints temperature in degrees farenheit

Please, use degrees celsius to keep it consistent with ACPI and
lm-sensors.

Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2003-02-14 21:03:27

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-13 at 23:04, Pavel Machek wrote:
> Hi!
>
> > temperature (RO)
> > - show: prints temperature in degrees farenheit
>
> Please, use degrees celsius to keep it consistent with ACPI and
> lm-sensors.

The ioctl interface is farenheit and has been since before Linux 2.0
That may not have been smart but we are stuck with it there at
least.

2003-02-14 21:05:23

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, Feb 14, 2003 at 05:55:53PM +0000, Alan Cox wrote:
> think you need to care about that for now. Sysfs doesn't help here in
> the general case as it lacks persistant file permissions, but where it

Oh, junk. I liked this proposal a lot, but lack of persistent
permissions kills it pretty dead.

Joel


--

Life's Little Instruction Book #182

"Be romantic."

Joel Becker
Senior Member of Technical Staff
Oracle Corporation
E-mail: [email protected]
Phone: (650) 506-8127

2003-02-14 21:26:38

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Hi!

> > > temperature (RO)
> > > - show: prints temperature in degrees farenheit
> >
> > Please, use degrees celsius to keep it consistent with ACPI and
> > lm-sensors.
>
> The ioctl interface is farenheit and has been since before Linux 2.0
> That may not have been smart but we are stuck with it there at
> least.

Oops, that's bad.

But I believe we should make it celsius in /sys, even if it means
conversion somewhere.

Pavel

--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

2003-02-14 23:18:29

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 13:35, Pavel Machek wrote:
> Hi!
>
> > > > temperature (RO)
> > > > - show: prints temperature in degrees farenheit
> > >
> > > Please, use degrees celsius to keep it consistent with ACPI and
> > > lm-sensors.
> >
> > The ioctl interface is farenheit and has been since before Linux 2.0
> > That may not have been smart but we are stuck with it there at
> > least.
>
> Oops, that's bad.
>
> But I believe we should make it celsius in /sys, even if it means
> conversion somewhere.
>
> Pavel

The watchdog infrastructure would just show what ever integer the driver
provides via the watchdog_ops.get_temperature() function pointer, so it
would be up to the driver developer to decide if the data is really
Fahrenheit or whatever.

--rustyl

2003-02-15 00:44:06

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Fri, 2003-02-14 at 23:17, Rusty Lynch wrote:
> The watchdog infrastructure would just show what ever integer the driver
> provides via the watchdog_ops.get_temperature() function pointer, so it
> would be up to the driver developer to decide if the data is really
> Fahrenheit or whatever.

We do need to be sure they all agree about it however 8)

2003-02-15 08:21:02

by Cort Dougan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

} On Fri, 2003-02-14 at 23:17, Rusty Lynch wrote:
} > The watchdog infrastructure would just show what ever integer the driver
} > provides via the watchdog_ops.get_temperature() function pointer, so it
} > would be up to the driver developer to decide if the data is really
} > Fahrenheit or whatever.
}
} We do need to be sure they all agree about it however 8)

Just to make sure no-one is happy except physicists, I suggest Kelvin. I
also suggest we spell disk/disc as "disck".

2003-02-15 09:04:54

by John Bradford

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

> } > The watchdog infrastructure would just show what ever integer the driver
> } > provides via the watchdog_ops.get_temperature() function pointer, so it
> } > would be up to the driver developer to decide if the data is really
> } > Fahrenheit or whatever.
> }
> } We do need to be sure they all agree about it however 8)
>
> Just to make sure no-one is happy except physicists, I suggest
> Kelvin.

Degrees C is the best choice, because the range of values that fit in
to a signed int is then useful (-127 -> 128). Storing as K or F means
that you can't store a useful range in a single byte.

> I also suggest we spell disk/disc as "disck".

Magnetic media -> disk
Optical media -> disc
Combination media, (magneto-optical) -> disk

John.

2003-02-15 12:51:49

by Ingo Oeser

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Hi,

On Sat, Feb 15, 2003 at 01:27:07AM -0700, Cort Dougan wrote:
> Just to make sure no-one is happy except physicists, I suggest Kelvin. I
> also suggest we spell disk/disc as "disck".

I suggest leaving out the "s" also, to keep the string length
constant, reduce kernel bloat and adding more fun to the live of
sysadmins.

Regards

Ingo Oeser ;-)
--
Science is what we can tell a computer. Art is everything else. --- D.E.Knuth

2003-02-15 17:16:13

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

Hi!


> } > The watchdog infrastructure would just show what ever integer the driver
> } > provides via the watchdog_ops.get_temperature() function pointer, so it
> } > would be up to the driver developer to decide if the data is really
> } > Fahrenheit or whatever.
> }
> } We do need to be sure they all agree about it however 8)
>
> Just to make sure no-one is happy except physicists, I suggest Kelvin. I
> also suggest we spell disk/disc as "disck".

Do I miss smiley around here?

Actually Kelvin is not such a bad idea, as deciKelvin is already used
by ACPI internally. It is converted to celsius before userspace sees
it, through.

Pavel
--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

2003-02-15 19:53:09

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Sat, 2003-02-15 at 09:13, John Bradford wrote:
> Magnetic media -> disk
> Optical media -> disc
> Combination media, (magneto-optical) -> disk

It varies by place. Optincal media is "disc" primarily because the
trademarks forced the one spelling on the funny foreigners. As everyone
knows the proper spelling is "disg" for all three.

Its not a useful argument to have.

2003-02-15 20:28:06

by John Bradford

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

> > Magnetic media -> disk
> > Optical media -> disc
> > Combination media, (magneto-optical) -> disk
>
> It varies by place. Optincal media is "disc" primarily because the
^^^^^^^^

> As everyone knows the proper spelling is "disg" for all three.

Ah, but not everyone knows the proper spelling of optical :-).

John.

2003-02-19 23:23:46

by Rusty Lynch

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

My original proposal raised a couple of issues with sysfs that make it
difficult to move completely from the current watchdog model documented in
watchdog-api to a completely sysfs based implementation.

Specifically, sysfs needs:
* persistent file permissions
* a way to forward ioctl's or in some way represent a device node in sysfs

Both of these are non-trivial topics that from what I understand are being
addressed, but will not be arriving soon.

>From the feedback that I have heard, it is my understanding that people are
not put off by the concept of a sysfs representation of watchdog
devices/drivers as I have described with the class based approach, but we
need to support the existing char device as described in watchdog-api.txt,
and it would be nice if each driver did not have to re-implement the miscdevice
code (open/close/ioctl/etc.)

The following patch adds the ability for the watchdog infrastructure code
to register as _the_ watchdog misc device, and forward appropriate calls
to the first watchdog device to register with the infrastructure.

This approach is just as functional as the current model since we currently
only allow the first driver to call misc_register with the watchdog minor
to successfully register. Once sysfs has persistent file permissions
and the ability to forward ioctl's, then then miscdevice code can be removed
from base.c and the drivers will not need to be touched.

--rustyl

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1032 -> 1.1038
# drivers/char/watchdog/Makefile 1.5 -> 1.8
# include/linux/watchdog.h 1.4 -> 1.8
# (new) -> 1.4 drivers/char/watchdog/base.c
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/02/12 [email protected] 1.1033
# Adding a common infrastructure for watchdog timers to implement
# a sysfs interface
# --------------------------------------------
# 03/02/13 [email protected] 1.1034
# Moving wdt_device from a platform_device to a sys_device,
# making stop/start/keepalive be write-only, and removing the get_enable
# and set_enable callbacks.
# --------------------------------------------
# 03/02/13 [email protected] 1.1035
# Changes that should have been adde to the last changeset
# --------------------------------------------
# 03/02/13 [email protected] 1.1036
# Reworked the watchdog infrastructure implementation to use the sysfs
# class concept.
# --------------------------------------------
# 03/02/19 [email protected] 1.1037
# Added support for /dev/watchdog by registering the first watchdog
# as a miscdevice using the watchdog minor. Also added a new callback
# to enable the firmware version of the watchdog to be queried as
# specified in watchdog-api.txt.
# --------------------------------------------
# 03/02/19 [email protected] 1.1038
# Removed the dummy1 and dummy2 testing files
# --------------------------------------------
#
diff -Nru a/drivers/char/watchdog/Makefile b/drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile Wed Feb 19 13:05:58 2003
+++ b/drivers/char/watchdog/Makefile Wed Feb 19 13:05:58 2003
@@ -7,6 +7,8 @@
# watchdog dies or is 'borrowed' for some reason the software watchdog
# still gives you some cover.

+obj-y += base.o
+
obj-$(CONFIG_PCWATCHDOG) += pcwd.o
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
diff -Nru a/drivers/char/watchdog/base.c b/drivers/char/watchdog/base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/watchdog/base.c Wed Feb 19 13:05:58 2003
@@ -0,0 +1,578 @@
+/*
+ * base.c
+ *
+ * Base watchdog timer infrastructure
+ *
+ * Copyright (C) 2003 Rusty Lynch <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <[email protected]>
+ */
+
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/watchdog.h>
+
+#include <linux/miscdevice.h>
+
+#ifdef DEBUG
+#define trace(format, args...) \
+ printk(KERN_INFO "%s(" format ")\n", __FUNCTION__ , ## args)
+#define dbg(format, arg...) \
+ printk (KERN_DEBUG "%s: " format "\n", \
+ __FUNCTION__, ## arg);
+#else
+#define trace(format, arg...) do { } while (0)
+#define dbg(format, arg...) do { } while (0)
+#endif
+
+#define crit(format, arg...) \
+ printk(KERN_CRIT "%s: " format "\n", \
+ __FUNCTION__ , ## arg)
+
+static struct watchdog_driver *miscdev = 0;
+static int expect_close = 0;
+
+static DECLARE_MUTEX(watchdog_sem);
+
+static int miscdev_open(struct inode *inode, struct file *file)
+{
+ trace("%p, %p", inode, file);
+ if (!miscdev || !miscdev->ops->start)
+ return -ENODEV;
+
+ if (miscdev->ops->start(miscdev))
+ return -EBUSY;
+
+ return 0;
+}
+
+static int miscdev_release(struct inode *inode, struct file *file)
+{
+ trace("%p, %p", inode, file);
+ if (!miscdev || !miscdev->ops->stop)
+ return -ENODEV;
+
+ if (!expect_close) {
+ crit("watchdog device closed unexpectedly, allowing timeout!");
+ return 0;
+ }
+
+ if (miscdev->ops->stop(miscdev, 0))
+ return -EBUSY;
+
+ return 0;
+}
+
+static ssize_t miscdev_write(struct file *file, const char *data,
+ size_t len, loff_t *ppos)
+{
+ trace("%p, %p, %i, %p", file, data, len, ppos);
+ int nway;
+
+ /* Can't seek (pwrite) on this device */
+ if (ppos != &file->f_pos)
+ return -ESPIPE;
+
+ if (!miscdev || !miscdev->ops->keepalive)
+ return -ENODEV;
+
+ /*
+ * Refresh the timer.
+ */
+ if(len) {
+
+ if (miscdev->ops->get_nowayout &&
+ miscdev->ops->get_nowayout(miscdev, &nway) == 0 &&
+ nway == 0) {
+ size_t i;
+
+ /* In case it was set long ago */
+ expect_close = 0;
+
+ for (i = 0; i != len; i++) {
+ char c;
+
+ if (get_user(c, data + i))
+ return -EFAULT;
+ dbg("looking for magic char: %c", c);
+ if (c == 'V')
+ expect_close = 1;
+ }
+ }
+ miscdev->ops->keepalive(miscdev);
+ return 1;
+ }
+ return 0;
+}
+
+static int miscdev_ioctl(struct inode *inode, struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ int tmp;
+ static struct watchdog_info ident = {
+ .identity = "Watchdog Device",
+ };
+
+ trace("%p, %p, %i, %li", inode, file, cmd, arg);
+ if (!miscdev)
+ return -ENODEV;
+
+ switch (cmd) {
+ default:
+ return -ENOTTY;
+ case WDIOC_GETSUPPORT:
+ if (miscdev->ops->get_firmware_version &&
+ !miscdev->ops->get_firmware_version(miscdev, &tmp))
+ ident.firmware_version = tmp;
+
+ if (miscdev->ops->get_options &&
+ !miscdev->ops->get_options(miscdev, &tmp))
+ ident.options = tmp;
+
+ if(copy_to_user((struct watchdog_info *)arg, &ident,
+ sizeof(ident)))
+ return -EFAULT;
+ return 0;
+ case WDIOC_GETSTATUS:
+ if (!miscdev->ops->get_status ||
+ miscdev->ops->get_status(miscdev, &tmp))
+ return -EFAULT;
+
+ if(copy_to_user((struct watchdog_info *)arg,&tmp,sizeof(tmp)))
+ return -EFAULT;
+
+ return 0;
+ case WDIOC_GETBOOTSTATUS:
+ if (!miscdev->ops->get_bootstatus ||
+ miscdev->ops->get_bootstatus(miscdev, &tmp))
+ return -EFAULT;
+
+ if(copy_to_user((struct watchdog_info *)arg,&tmp,sizeof(tmp)))
+ return -EFAULT;
+
+ return 0;
+ case WDIOC_GETTEMP:
+ if (!miscdev->ops->get_temperature ||
+ miscdev->ops->get_temperature(miscdev, &tmp))
+ return -EFAULT;
+
+ if(copy_to_user((struct watchdog_info *)arg,&tmp,sizeof(tmp)))
+ return -EFAULT;
+
+ return 0;
+ case WDIOC_SETOPTIONS:
+ if (get_user(tmp, (int *)arg))
+ return -EFAULT;
+
+ if (tmp & WDIOS_DISABLECARD) {
+ if (!miscdev->ops->stop ||
+ miscdev->ops->stop(miscdev, 0))
+ return -EFAULT;
+ }
+
+ if (tmp & WDIOS_ENABLECARD) {
+ if (!miscdev->ops->start ||
+ miscdev->ops->start(miscdev))
+ return -EFAULT;
+ }
+
+ if (tmp & WDIOS_TEMPPANIC) {
+ if (!miscdev->ops->set_temppanic ||
+ miscdev->ops->set_temppanic(miscdev, 1))
+ return -EFAULT;
+ }
+
+ return 0;
+ case WDIOC_KEEPALIVE:
+ if (!miscdev->ops->keepalive ||
+ miscdev->ops->keepalive(miscdev))
+ return -EFAULT;
+
+ return 0;
+ case WDIOC_SETTIMEOUT:
+ if (!miscdev->ops->set_timeout)
+ return -EFAULT;
+ if (get_user(tmp, (int *)arg))
+ return -EFAULT;
+ if (tmp < 1)
+ return -EINVAL;
+
+ if (miscdev->ops->set_timeout(miscdev, tmp))
+ return -EFAULT;
+
+ if (!miscdev->ops->keepalive ||
+ miscdev->ops->keepalive(miscdev))
+ return -EFAULT;
+
+ return 0;
+ case WDIOC_GETTIMEOUT:
+ if (!miscdev->ops->get_timeout ||
+ miscdev->ops->get_timeout(miscdev, &tmp))
+ return -EFAULT;
+
+ if(copy_to_user((struct watchdog_info *)arg,&tmp,sizeof(tmp)))
+ return -EFAULT;
+
+ return 0;
+ }
+}
+
+static struct file_operations watchdog_fops = {
+ .owner = THIS_MODULE,
+ .write = miscdev_write,
+ .ioctl = miscdev_ioctl,
+ .open = miscdev_open,
+ .release = miscdev_release,
+};
+
+static struct miscdevice watchdog_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "Watchdog Timer",
+ .fops = &watchdog_fops,
+};
+
+ssize_t start_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->start))
+ return -ENODEV;
+
+ if ((w->ops->start)(w))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(start,S_IWUSR,NULL,start_store);
+
+ssize_t stop_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->stop))
+ return -ENODEV;
+
+ if ((w->ops->stop)(w, buf))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(stop,S_IWUSR,NULL,stop_store);
+
+ssize_t timeout_show(struct device_driver *d, char *buf)
+{
+ int timeout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_timeout))
+ return -ENODEV;
+
+ if((w->ops->get_timeout)(w, &timeout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", timeout);
+}
+ssize_t timeout_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int timeout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->set_timeout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&timeout))
+ return -EINVAL;
+
+ if ((w->ops->set_timeout)(w, timeout))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(timeout,S_IRUGO|S_IWUSR,timeout_show,timeout_store);
+
+ssize_t keepalive_store(struct device_driver *d, const char *buf, size_t count)
+{
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->keepalive))
+ return -ENODEV;
+
+ if ((w->ops->keepalive)(w))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(keepalive,S_IWUSR,NULL,keepalive_store);
+
+ssize_t nowayout_show(struct device_driver *d, char *buf)
+{
+ int nowayout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_nowayout))
+ return -ENODEV;
+
+ if ((w->ops->get_nowayout)(w, &nowayout))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", nowayout);
+}
+ssize_t nowayout_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int nowayout;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->set_nowayout))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&nowayout))
+ return -EINVAL;
+
+ if ((w->ops->set_nowayout)(w, nowayout))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(nowayout,S_IRUGO|S_IWUSR,nowayout_show,nowayout_store);
+
+ssize_t status_show(struct device_driver *d, char *buf)
+{
+ int status;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_status))
+ return -ENODEV;
+
+ if ((w->ops->get_status)(w, &status))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", status);
+}
+DRIVER_ATTR(status,S_IRUGO,status_show,NULL);
+
+ssize_t bootstatus_show(struct device_driver *d, char *buf)
+{
+ int bootstatus;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_bootstatus))
+ return -ENODEV;
+
+ if ((w->ops->get_bootstatus)(w, &bootstatus))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", bootstatus);
+}
+DRIVER_ATTR(bootstatus,S_IRUGO,bootstatus_show,NULL);
+
+ssize_t options_show(struct device_driver *d, char *buf)
+{
+ int options;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_options))
+ return -ENODEV;
+
+ if ((w->ops->get_options)(w, &options))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", options);
+}
+DRIVER_ATTR(options,S_IRUGO,options_show,NULL);
+
+ssize_t temperature_show(struct device_driver *d, char *buf)
+{
+ int temperature;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_temperature))
+ return -ENODEV;
+
+ if ((w->ops->get_temperature)(w, &temperature))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temperature);
+}
+DRIVER_ATTR(temperature,S_IRUGO,temperature_show,NULL);
+
+ssize_t temppanic_show(struct device_driver *d, char *buf)
+{
+ int temppanic;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_temppanic))
+ return -ENODEV;
+
+ if ((w->ops->get_temppanic)(w, &temppanic))
+ return -EFAULT;
+
+ return sprintf(buf, "%i\n", temppanic);
+}
+ssize_t temppanic_store(struct device_driver *d, const char *buf, size_t count)
+{
+ int temppanic;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p, %i", d, buf, count);
+ if (!(w->ops->set_temppanic))
+ return -ENODEV;
+
+ if (!sscanf(buf,"%i",&temppanic))
+ return -EINVAL;
+
+ if ((w->ops->set_temppanic)(w, temppanic))
+ return -EFAULT;
+
+ return count;
+}
+DRIVER_ATTR(temppanic,S_IRUGO|S_IWUSR,temppanic_show,temppanic_store);
+
+ssize_t firmware_version_show(struct device_driver *d, char *buf)
+{
+ int version;
+ struct watchdog_driver *w = to_watchdog_driver(d);
+
+ trace("%p, %p", d, buf);
+ if (!(w->ops->get_firmware_version))
+ return -ENODEV;
+
+ if ((w->ops->get_firmware_version)(w, &version))
+ return -EFAULT;
+
+ return sprintf(buf, "0x%08x\n", version);
+}
+DRIVER_ATTR(firmware_version,S_IRUGO,firmware_version_show,NULL);
+
+static int watchdog_create_default_files(struct watchdog_driver *d)
+{
+ driver_create_file(&(d->driver), &driver_attr_start);
+ driver_create_file(&(d->driver), &driver_attr_stop);
+ driver_create_file(&(d->driver), &driver_attr_timeout);
+ driver_create_file(&(d->driver), &driver_attr_keepalive);
+ driver_create_file(&(d->driver), &driver_attr_nowayout);
+ driver_create_file(&(d->driver), &driver_attr_status);
+ driver_create_file(&(d->driver), &driver_attr_bootstatus);
+ driver_create_file(&(d->driver), &driver_attr_options);
+ driver_create_file(&(d->driver), &driver_attr_temperature);
+ driver_create_file(&(d->driver), &driver_attr_temppanic);
+ driver_create_file(&(d->driver), &driver_attr_firmware_version);
+
+ return 0;
+}
+
+static void watchdog_remove_default_files(struct watchdog_driver *d)
+{
+ driver_remove_file(&(d->driver), &driver_attr_start);
+ driver_remove_file(&(d->driver), &driver_attr_stop);
+ driver_remove_file(&(d->driver), &driver_attr_timeout);
+ driver_remove_file(&(d->driver), &driver_attr_keepalive);
+ driver_remove_file(&(d->driver), &driver_attr_nowayout);
+ driver_remove_file(&(d->driver), &driver_attr_status);
+ driver_remove_file(&(d->driver), &driver_attr_bootstatus);
+ driver_remove_file(&(d->driver), &driver_attr_options);
+ driver_remove_file(&(d->driver), &driver_attr_temperature);
+ driver_remove_file(&(d->driver), &driver_attr_temppanic);
+ driver_remove_file(&(d->driver), &driver_attr_firmware_version);
+}
+
+int watchdog_driver_register(struct watchdog_driver *d)
+{
+ int rv;
+
+ down(&watchdog_sem);
+ rv = driver_register(&(d->driver));
+ if (!rv)
+ rv = watchdog_create_default_files(d);
+
+ dbg("miscdev pointing to %s",miscdev? miscdev->driver.name:"nobody");
+ if (!miscdev) {
+ if (misc_register(&watchdog_miscdev) == 0) {
+ dbg("registered %s as the miscdev", d->driver.name);
+ miscdev = d;
+ } else {
+ crit("%s failed misc_register()", d->driver.name);
+ }
+ }
+ up(&watchdog_sem);
+
+ return rv;
+}
+
+void watchdog_driver_unregister(struct watchdog_driver *d)
+{
+ down(&watchdog_sem);
+ if (d && miscdev == d) {
+ dbg("deregistering %s as the misc device", d->driver.name);
+ misc_deregister(&watchdog_miscdev);
+ }
+
+ driver_unregister(&(d->driver));
+ watchdog_remove_default_files(d);
+ up(&watchdog_sem);
+}
+
+struct device_class watchdog_devclass = {
+ .name = "watchdog",
+};
+
+static int __init watchdog_init(void)
+{
+ int ret = 0;
+
+ trace();
+ ret = devclass_register(&watchdog_devclass);
+ if (ret)
+ return -ENODEV;
+
+ return 0;
+}
+
+static void __exit watchdog_exit(void)
+{
+ trace();
+
+ if (miscdev)
+ misc_deregister(&watchdog_miscdev);
+
+ devclass_unregister(&watchdog_devclass);
+}
+
+module_init(watchdog_init);
+module_exit(watchdog_exit);
+EXPORT_SYMBOL_GPL(watchdog_driver_register);
+EXPORT_SYMBOL_GPL(watchdog_driver_unregister);
+EXPORT_SYMBOL_GPL(watchdog_devclass);
diff -Nru a/include/linux/watchdog.h b/include/linux/watchdog.h
--- a/include/linux/watchdog.h Wed Feb 19 13:05:58 2003
+++ b/include/linux/watchdog.h Wed Feb 19 13:05:58 2003
@@ -10,9 +10,17 @@
#define _LINUX_WATCHDOG_H

#include <linux/ioctl.h>
+#include <linux/device.h>

#define WATCHDOG_IOCTL_BASE 'W'

+#define to_watchdog_driver(n) container_of(n, struct watchdog_driver, driver)
+
+struct watchdog_driver;
+struct watchdog_ops;
+
+extern struct device_class watchdog_devclass;
+
struct watchdog_info {
__u32 options; /* Options the card/driver supports */
__u32 firmware_version; /* Firmware version of the card */
@@ -45,5 +53,30 @@
#define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */
#define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */
#define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip */
+
+struct watchdog_driver {
+ struct watchdog_ops *ops;
+ struct device_driver driver;
+};
+
+struct watchdog_ops {
+ int (*start)(struct watchdog_driver *);
+ int (*stop)(struct watchdog_driver *, const char *);
+ int (*keepalive)(struct watchdog_driver *);
+ int (*get_timeout)(struct watchdog_driver *, int *);
+ int (*set_timeout)(struct watchdog_driver *, int);
+ int (*get_nowayout)(struct watchdog_driver *, int *);
+ int (*set_nowayout)(struct watchdog_driver *, int);
+ int (*get_status)(struct watchdog_driver *, int *);
+ int (*get_bootstatus)(struct watchdog_driver *, int *);
+ int (*get_options)(struct watchdog_driver *, int *);
+ int (*get_temperature)(struct watchdog_driver *, int *);
+ int (*get_temppanic)(struct watchdog_driver *, int *);
+ int (*set_temppanic)(struct watchdog_driver *, int);
+ int (*get_firmware_version)(struct watchdog_driver *, int *);
+};
+
+int watchdog_driver_register(struct watchdog_driver *);
+void watchdog_driver_unregister(struct watchdog_driver *);

#endif /* ifndef _LINUX_WATCHDOG_H */

2003-02-20 21:09:38

by Jakob Oestergaard

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Tue, Feb 18, 2003 at 09:24:15PM -0800, Rusty Lynch wrote:
> My original proposal raised a couple of issues with sysfs that make it
> difficult to move completely from the current watchdog model documented in
> watchdog-api to a completely sysfs based implementation.
>
> Specifically, sysfs needs:
> * persistent file permissions
> * a way to forward ioctl's or in some way represent a device node in sysfs

Something as simple as a watchdog should not need ioctls, IMO. (Nothing
should need them, but let's take one battle at a time...)

How about using sysfs and specifically - now that we have a collection
of drivers which are simple enough to not need the mistake that ioctls
are - making sure that they work as before, using only device files and
no magic ioctls ?

I shall happily volunteer to delete the 10 lines of code from
sbc60xxwdt.c to make it compliant to the "no ioctls - equivalent
functionality" idea ;)

I know that there is ioctl support in the existing drivers - but I have
not yet seen a driver which needed it. "needed" in the sense that
equivalent functionality could not have been created using dev files
alone.

Also, the amount of userspace which will break because of missing ioctl
functionality will be absolutely *minimal*. There's not a lot of
watchdog software out there, and porting whatever software uses ioctls
to use sane interfaces instead, should be doable. I don't think anyone
would get terribly upset if this change was made as a 2.4->2.6
transition thing.

If ioctls are kept in watchdog drivers for 2.6, they can't go away until
2.8/3.0/whatever.

--
................................................................
: [email protected] : And I see the elder races, :
:.........................: putrid forms of man :
: Jakob ?stergaard : See him rise and claim the earth, :
: OZ9ABN : his downfall is at hand. :
:.........................:............{Konkhra}...............:

2003-02-20 21:26:39

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFC] Proposal for a new watchdog interface using sysfs

On Thu, 2003-02-20 at 21:19, Jakob Oestergaard wrote:
> I know that there is ioctl support in the existing drivers - but I have
> not yet seen a driver which needed it. "needed" in the sense that
> equivalent functionality could not have been created using dev files
> alone.
>
> Also, the amount of userspace which will break because of missing ioctl
> functionality will be absolutely *minimal*. There's not a lot of
> watchdog software out there, and porting whatever software uses ioctls
> to use sane interfaces instead, should be doable. I don't think anyone
> would get terribly upset if this change was made as a 2.4->2.6
> transition thing.

There is a lot of watchdog using stuff, some quite proprietary and
embedded into big apps. Even then you have to solve the persistence
issue.

Losing the old api is a 2.8/3.0 thing perhaps, even then its a big
break by Linux standards