Re: [RFC] Watchdog device class

[Wim Van Sebroeck <wim@iguana.be>]

Hi Rudolf,

> Aha good. I will check it later.

You can have a check now. I uploaded some code. Need to retest it, but it
has been working on my v2.6.5 test machine.

> > 	int	(*get_timervalue)(struct watchdog_device *, int *);
> Good one.

We should add this indeed. it's usefull for testing drivers also :-)

> > 	int	(*sys_restart)(struct watchdog_device *);		/* operation = force a
> system_restart for rebooting */
> 
> Aha as for the cobalt stuff?

Yes and no: cobalt needs it in it's notifier reboot part which is not part
of the generic watchdog device. But I think the option should be available
if you go sysfs.

> > 	int	(*get_status)(struct watchdog_device *,int *);		/* operation = get the watchdog's status */
> > 	int	(*get_temperature)(struct watchdog_device *, int *);	/* operation = get the temperature in °F */
> 
> I had those there too but I eliminated them. I used following methods:
> For the status stuff I did a variable boot_status and status. I have
> a handler for this in the common IOCTL handling code.

Don't agree here: boot_status is a copy of the status at boot. the status
itself can change during normal operation. and thus get_status must return
the "devices status" at that moment.

> I have no such thing for the temp IOCTL but the new "ioctl" operation
> could be created to catch it.
> (and get called when no standard ioctl in watchdog-dev is used)

I think we want to review the temperature stuff in the kernel in general.

> As for sysfs, I would like to have the temps handled with the hwmon class
> and have some sort of "symlink" from the watchdog directory to corresponding
> hwmon directory. The status stuff might be handled via standard format sysfs file.

And I like the idea to look at it as a hwmon.

> > 	/* From here on everything is device dependent */
> > 	void	*private;
> 
> In the w83792d I used different approach, because the device is not only a
> watchog, the struct watchdog_device was a part of the common device structure,
> and for single purpose devices this *private makes sense. But I think there
> some per device private data pointer somewhere.

I still have to look at your driver in detail, but my first thought would
be that the private part here would be a link to this common device structure.
(see what I did with the example softdog implementation in the experimental tree).

> Now it seems we have two different approaches and a code, so which one will wim
> ? ;) If you want to talk to me you may find me on #linux-sensors on freenode.net.

It's not about different approaches: we have to find the best thing for watchdog
devicesi, so the best thing is to talk about pro's and con's and see what we should 
do best. (I for instance didn't come to the sysfs part yet of my code (which would
be in watchdog_sysfs.c)

> Also I would like to know your ideas about the sysfs file structure for
> watchdogs and also If you like to have more watchdogs active in the system or
> just one.

My view:
to start we should keep one /dev/watchdog, but we should create/define a suitable
sysfs interface that makes it possible to have multiple watchdog devices running 
in parallel. We will need this functionality in the future when a system will 
consist of different processors that all have their own memory and some basic
I/O interfacing.
Later on we will then see what we will do with /dev/watchdog.

Greetings,
Wim.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/