Re: RFC: drivers/char/watchdog/pcwd.c + drivers/char/watchdog/pcwd_pci.c

[Wim Van Sebroeck]

Hi Andrew,

> >  /* 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.

Greetings,
Wim.

-
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/