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

[Pali RohÃr]

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.