Re: [PATCH 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

From: Pali RohÃr
Date: Sat Mar 28 2015 - 18:05:00 EST


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.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.