Re: [RFC] Watchdog device class

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux