On Fri, Sep 18, 2009 at 04:49:02PM +0200, Jean Delvare wrote: > 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. Yes, sorry. Fixed. > > 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. Both fixed. > I'll go on with the review when you provide a patch I can build. Ok, thanks. Rodolfo -- GNU/Linux Solutions e-mail: giometti@xxxxxxxxxxxx Linux Device Driver giometti@xxxxxxxx Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
