On Fri, 18 Sep 2009 14:09:23 +0200, Rodolfo Giometti wrote: > Hi Jean, > > this new proposal patch sounds better to you? :) > > Ciao, > > Rodolfo > > -- > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 2d50166..bc7058f 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -894,6 +894,7 @@ config SENSORS_W83L786NG > > config SENSORS_W83627HF > tristate "Winbond W83627HF, W83627THF, W83637HF, W83687THF, W83697HF" > + depends on 83627HF Which doesn't exist, so the driver can't be selected. Obviously you meant: depends on MFD_W83627HF. But please also see my discussion with Samuel on depends vs. select. I'd prefer that you use select. Especially given that the MFD configuration entry is _after_ the hwmon configuration entry in Kconfig, I fear that even if users see and select MFD_W83627HF, they won't walk back to the W83627HF entry to select it. Samuel, wouldn't it make sense to move the MFD section up in the Device Drivers section? Even after fixing this so that I could select the driver, I'm hitting build problems: CC [M] drivers/hwmon/w83627hf.o drivers/hwmon/w83627hf.c: In function ‘w83627hf_probe’: drivers/hwmon/w83627hf.c:1121: warning: passing argument 1 of ‘acpi_check_resource_conflict’ from incompatible pointer type drivers/hwmon/w83627hf.c:1125: error: ‘WINB_REGION_SIZE’ undeclared (first use in this function) drivers/hwmon/w83627hf.c:1125: error: (Each undeclared identifier is reported only once drivers/hwmon/w83627hf.c:1125: error: for each function it appears in.) drivers/hwmon/w83627hf.c: In function ‘w83627hf_remove’: drivers/hwmon/w83627hf.c:1290: error: ‘WINB_REGION_SIZE’ undeclared (first use in this function) Please build-test your patches before you send them. > select HWMON_VID > help > If you say yes here you get support for the Winbond W836X7 series > diff --git a/drivers/hwmon/w83627hf.c b/drivers/hwmon/w83627hf.c > index 389150b..4757668 100644 > --- a/drivers/hwmon/w83627hf.c > +++ b/drivers/hwmon/w83627hf.c > @@ -51,18 +51,10 @@ > #include <linux/mutex.h> > #include <linux/ioport.h> > #include <linux/acpi.h> > -#include <asm/io.h> > +#include <linux/io.h> As I said before, this change is already upstream, so please don't include it in your patch. > #include "lm75.h" > +#include <linux/mfd/w83627hf.h> Global includes must be listed before local ones. I'll go on with the review when you provide a patch I can build. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors