On Saturday 28 March 2015 15:23:15 Guenter Roeck wrote: > > + ---help--- > > + This hwmon driver adds support for reporting temperature > > of different + sensors and controls the fans on Dell > > laptops via System Management + Mode provided by Dell > > BIOS. > > + > > + When option I8K is also enabled this driver provides > > legacy /proc/i8k + userspace interface for i8kutils > > package. > > + > > Please add this in alphabetic order. >
ok
> > if ACPI
> >
> > comment "ACPI drivers"
> >
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 1c3e458..9eec614 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -155,7 +155,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) +=
> > w83l785ts.o
> >
> > obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> > obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> >
> > -obj-$(CONFIG_I8K) += dell-smm-hwmon.o
> > +obj-$(CONFIG_SENSORS_DELL_SMM) += dell-smm-hwmon.o
>
> Same here.
>
ok
> > - proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
> > - if (!proc_i8k)
> > + if (!proc_create("i8k", 0, NULL, &i8k_fops))
> >
> > return -ENOENT;
>
> I would prefer not to fail here but report a warning.
> This is no longer a fatal condition.
>
ok, no problem
>
> Can you move all the conditional functions and global
> variables together under a single #ifdef ? That should
> include functions to create the proc entries, and shim
> functions for the same if I8K is not configured.
>
ok, I will move procfs code to one location.
> Also, the #ifdef would not cover the case where I8K is
> configured as module (there is no reason to force it to
> bool). You should use "#if IS_ENABLED(CONFIG_I8K)" instead.
>
I think that setting CONFIG_I8K to tristate (Y/M/N) does not make
sense... CONFIG_I8K just control if /proc/i8k will be compiled
into dell-smm-hwmon or not.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
