Re: [PATCH v3] hwmon: added kernel module for FTS BMC chip "Teutates"

["thilo.cestonaro@ts.fujitsu.com" <thilo.cestonaro@ts.fujitsu.com>]

[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]

Hey Guenter!

> > +config SENSORS_FTSTEUTATES
> > +	tristate "Fujitsu Technology Solutions sensor chip Teutates"
> > +	depends on I2C
> > +	help
> > +	  If you say yes here you get support for the Fujitsu Technology
> > +      Solutions (FTS) sensor chip "Teutates" including support for
> > +      the integrated watchdog.
> Did you try to enable this ? Probably not, because if I apply your patch
> and try to run "make allmodconfig" I get
> 
> drivers/hwmon/Kconfig:495: syntax error
> drivers/hwmon/Kconfig:494: unknown option "Solutions"
> drivers/hwmon/Kconfig:495: unknown option "the"
> drivers/hwmon/Kconfig:498: syntax error
> 
Sorry about that! Indention was wrong.

> > +	unsigned char page = reg>>8;
> 				reg << 8
> 
> checkpatch --strict helps finding such places.
> 
Thanks for the hint.
Next patch will be checked with --strict, promise! :)

> > 
> > +		fts_wdt.status |= BIT(WDOG_ACTIVE);
> WDOG_ACTIVE indicates that the watchdog device is opened. You need to set
> WDOG_HW_RUNNING instead.
> 
Testing with Kernel 4.4 which has no HW_RUNNING. Forgot to switch it back when
creating the patch.

> > 
> > +		count = reg;
> > +		goto error;
> > +	}
> > +
> > +	val = fts_write_byte(data->client, FTS_REG_FAN_CONTROL(index),
> > +			     reg | 0x1);
> > +	if (val < 0) {
> val is unsigned long. Why not use ret here ?
> 
> Please build your code with W=1 to see (and hopefully eliminate) all those
> errors.
> 
Yes! Will use W=1 from now on!

Cheers,
Thilo

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3847 bytes --]