[lm-sensors] THMC50 and ADM1022 support for 2.6 kernel (improved)

[Krzysztof Helt]

On Sun, 1 Jul 2007 20:50:03 +0200

Thank you for valuable input. I removed wrappers and changed data types

Jean Delvare <khali at linux-fr.org> wrote:
> 
> > > +	struct thmc50_data *data;
> > > +	struct device *dev;
> > > +	int err = 0;
> > > +	const char *type_name = "";
> > > +
> > > +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > 
> > Some type of warning would be nice here, instead of silently failing.
> > 
> > > +		goto exit;
> 
> I disagree. There may be several I2C buses on the system, for example
> one with no THMC50 chip and which doesn't support this functionality,
> and one with a THMC50 chip and which does support this functionality.
> Printing a warning when the first bus is being probed would be quite
> misleading.
> 
> As a matter of fact, none of the other i2c-based hardware monitoring
> drivers print error messages in this case. One driver (max6650) prints
> a debugging message.
> 

The driver will print debugging message to keep you and Mark Hoffman happy.
> 
> > > +static void thmc50_init_client(struct i2c_client *client)
> > > +{
> > > +	struct thmc50_data *data = i2c_get_clientdata(client);
> > > +
> > > +	data->config_reg |= 0x23;
> > > +	thmc50_write_value(client, THMC50_REG_CONF, data->config_reg);
> > > +	data->analog_out = thmc50_read_value(client, THMC50_REG_ANALOG_OUT);
> > > +	/* set up to at least 1 */
> > > +	if (data->analog_out == 0 ) {
> > > +		data->analog_out = 1;
> > > +		thmc50_write_value(client, THMC50_REG_ANALOG_OUT, data->analog_out);
> > > +	}
> > > +}
> 
> Why do you change the value of the analog output? Loading a hardware
> monitoring driver should NOT change the state of the system.
> 

The analog output has range from 0 to 255. The 0 value is the least value possible but
it is not 0 voltage on output pin. It simply must be adjusted to the range 1-255
to conform with sysfs. I choose this way as the simplest. Another possibility is
to scale this value when set and displayed from 1-255 (sysfs) to 0-255 (chip) range.

The sysfs 0 value for pwm1 is translated to setting a special bit which cuts off the
fan power completely (this works on my motherboard and it is given in reference
design, but I cannot guarantee it is connected this way on all boards). 

Regards,
Krzysztof