Re: RFC: drivers/char/watchdog/pcwd.c + drivers/char/watchdog/pcwd_pci.c
From: Wim Van Sebroeck
Date: Tue Apr 24 2007 - 15:29:37 EST
> > /* module parameters */
> > +#define CELSIUS 0
> > +#define FAHRENHEIT 1
> > +static int proc_temp_mode = CELSIUS;
> > +module_param(proc_temp_mode, int, 0);
> > +MODULE_PARM_DESC(proc_temp_mode, "which temperature mode to use in /proc/ 0=Celsius, 1=Fahrenheit (default=0)");
> hm, is that a good idea? Making the contents of /proc files dependent upon
> some module parameter?
> I'd have thought that it would be better to remove this option and either
> a) offer two /proc files: one for Celcius, one for Fahrenheit
> b) Put both values into the same /proc file (one per line)
> c) Just remove the Fahrenheit option altogether - let it join cubits,
> furlongs, etc.
> I mean, how is an application to make sense of that information if its units
> depend upon some module parameter, or a kernel boot option?
My opinion: temperature measured in a watchdog device is created to be able to
react on a "system overheat" situation. For me this thus is a hwmon device and
thus should be done via the hwmon interface. And that seems to be in
millidegree Celsius... (where the watchdog drivers used to do things in
Fahrenheit). I propose to follow the hwmon interface setup.
(BTW: My plans with the pcwd drivers is to 1) get one driver that is maintained
in the kernel instead of a basic driver in the kernel and an extended one that
is maintained seperately, 2) convert the pcwd.c driver to the isa_driver,
3) change temperature stuff to hwmon interface and 4) create a sysfs equivalent
for the /proc interface.)
I will take all your other comments/input and rework the code so that it is up
to the kernel standards.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/