Messages in this thread | | | Date | Tue, 18 Apr 2006 20:35:55 +0200 | From | Rudolf Marek <> | Subject | Re: [RFC] Watchdog device class |
| |
Hello Alan,
>>The char device of watchdog class is compatible with existing watchdog API, >>so no need to change the user applications. There is just one exception >>and this is temperature handling. I belive it should be implemented not >>via IOCTL but using the HWMON class. (100% compatibility can be restored >>with the ioctl class op) > > > Then it should be kept.
Ok I think best would be to create ops->ioctl callback for not so common ioctls (like the temp one) and let the driver to deal with them.
> The watchdog API simply pre-dates the sysfs world, it goes back to the > 1.0-1.2 era and has remained very consistent since that time. > > If you expose it in sysfs somewhere (which I think is a good idea) then > the units should probably also be fixed in the sysfs case to be metric > (ie Kelvin or Centigrade float values) [or scaled int]
Yep I have this in mind. But not yet the topic of the day. So far I have:
name - this is not need perhaps it can be found in another place in sysfs timeout - timeout value in sec - convert it to ms perhaps? ping - "ping" file, to replace the /dev/watchdog writes boot_status - the boot status - meaning same as IOCTL has, generaly the reset reason status - current status - same as ioctl equivalent
And optional firmware_ver to reflect the IOCTL equvalent.
This is needed to be discussed with Wim first. I hope he will speak up ;)
As for the temps/fans I belive the driver should register in hwmon class and use hwmon class sysfs iterface and then just create sort of relation between the sysfs files/classes, so the watchdog app can find the temps.
>> int (*set_timeout)(struct device *, int sec); > > > Pass the usual time structures instead. Seconds is a field so it is free > but it means all the signed/unsigned stuff and any future subsecond > watchdogs for embedded environments don't break stuff.
Good idea, I will change it. Thanks.
>> int (*notify_reboot)(struct notifier_block *this, unsigned long code, >> void *unused); > Can this not use the power management callbacks from the device model > instead
Ah I knew about the suspend/resume and it seems there is also a shutdown one.
>> /* this may be removed in the future */ >> struct watchdog_info legacy_info; > > This wants breaking out into sysfs, but again the ioctls are expected > and standardised for years now. > > People have talked about sorting out a watchdog helper library for years > so this is overdue, and doing it with the class model in mind is even > better.
I was quite amazed when I saw same code copied 40x in one directory ;)
Thanks for the comments, Regards Rudolf - 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/
| |