[lm-sensors] [PATCH] hwmon: Add w83791d support

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Charles Spirakis]

Add support for the w83791d sensor chip. The w83791d hardware is
somewhere between the w83781d and the w83792d and this driver code
is derived from the code that supports those chips.

Signed-off by: Charles Spirakis <bezaur at gmail.com>

---

  Documentation/hwmon/w83791d |  106 +++
  drivers/hwmon/Kconfig       |   10
  drivers/hwmon/Makefile      |    1
  drivers/hwmon/w83791d.c     | 1247
++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1364 insertions(+)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: w83791d_support.patch
Type: text/x-patch
Size: 44837 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060405/ac649306/w83791d_support-0001.bin

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Jean Delvare]

Hi Charles,

> Add support for the w83791d sensor chip. The w83791d hardware is
> somewhere between the w83781d and the w83792d and this driver code
> is derived from the code that supports those chips.

Sorry for having been rather silent and slow about your driver to date.
I can see that Greg KH and Rudolf Marek did some review on your code
already, so it should be quite good already, and hopefully not far from
being mergeable.

Here are my own comments.

> +Author: Charles Spirakis
> +Contact: bezaur at gmail.com

Put it all on one line please (makes it easier to cut'n'paste into an
e-mail client to contact the author.)

> +static int init;
> +module_param(init, bool, 0);
> +MODULE_PARM_DESC(init, "Set to one to force chip initialization");

This parameter would be better named "reset", as you still do some
initialization regardless of it being set to 0. What really changes is
the reset step.

> +#define IN_TO_REG(nr,val)	(SENSORS_LIMIT((((val) * 10 + 8) / 16), 0, 255))
> +#define IN_FROM_REG(nr,val)	(((val) * 16) / 10)
> (...)
> +	return sprintf(buf,"%ld\n", \
> +			(long)(IN_FROM_REG(nr, (data->reg[nr] * 10)))); \
> (...)
> +	val = simple_strtoul(buf, NULL, 10) / 10; \
> +	 \
> +	mutex_lock(&data->update_lock); \
> +	data->in_##reg[nr] = SENSORS_LIMIT(IN_TO_REG(nr, val), 0, 255); \

This is no good, you're losing resolution by doing opposite operations
inside and outside the macros. Please simplify it all.

> +/* for temp1 only */
> +#define TEMP1_FROM_REG(val)	(((val) & 0x80 ? \
> +					(val) - 0x100 : \
> +					(val)) * 1000)
> +#define TEMP1_TO_REG(val)	(SENSORS_LIMIT(((val) < 0 ? \
> +					(val) + (0x100 * 1000) : \
> +					(val)) / 1000, 0, 0xff))

What you are doing here with your negative value check and offset
addition, is converting from u8 to s8 and vice-versa. You can let the
hardware do it for you, this will be both more readable and more
efficient. All you have to do is declare w83791d_data.temp1 as an array
of s8 instead of u8 (see the lm90 driver for an example.)

Same goes for temp2 and temp3, except that this is a bit more complex
due to the additional resolution bit. Usually we store each temperature
value in an s16 (left padded, 7 LSB are always 0) in this case.

> +#define PWM_FROM_REG(val)		(val)
> +#define PWM_TO_REG(val)			(SENSORS_LIMIT((val),0,255))

You are not using these for now, so you shouldn't define them.

> #define BEEP_ENABLE_TO_REG(val)		((val) ? 1 : 0)
> #define BEEP_ENABLE_FROM_REG(val)	((val) ? 1 : 0)

These ones are not especially useful, at least the way they are
implemented. They would make sense if you had no separate
w83791d_data.beep_enable member, but you do. So, please either drop
that separate member and keep the macros (with some changes, obviously),
or drop the macros.

> +static u8 div_to_reg(long val)
> +{
> +	int i;
> +	val = SENSORS_LIMIT(val, 1, 128) >> 1;
> +	for (i = 0; i < 6; i++) {
> +		if (val = 0)
> +			break;
> +		val >>= 1;
> +	}
> +	return (u8) i;
> +}

This code has a bug which was fixed in the w83792d driver recently: fan
clock divider value of 128 can't be set, because the end condition
should be "i < 7" instead of "i < 6". Please fix.

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h–32051963cb6e6f7412990f8b962209b9334e13

In a more general way, you may want to take a look at the recent
changes which were made to the w83792d driver, and see if they could
apply to your code too.

+struct w83791d_data {
+	struct i2c_client client;
+	struct class_device *class_dev;
+	struct mutex update_lock;
+	enum chips type;

You don't use the type value anywhere at the moment, and the chances
that your driver will have to support more chips in the future are low,
so you could drop it to save some memory.

> +#define show_fan_reg(reg) \
> +static ssize_t show_##reg(struct device *dev, struct device_attribute *attr, \
> +				char *buf) \
> +{ \
> +	struct sensor_device_attribute *sensor_attr = \
> +						to_sensor_dev_attr(attr); \
> +	int nr = sensor_attr->index - 1; \

You could define the sysfs files attribute differently so that you
don't need to apply that offset on every access to every file. That
would be both shorter and more efficient.

> +	default:
> +		dev_warn(dev, "store_fan_div: Unexpected nr seen: %d\n", nr);
> +		count = -EINVAL;
> +		goto err_exit;

This is so unexpected that this just cannot happen, unless you have an
internal (code) error. So this part should either not exist at all (now
that the driver is working) or be only compiled in in debug mode.

> +	tmp_fan_div = ((data->fan_div[nr] << new_shift) & (~keep_mask));

The outermost pair of parentheses could be omitted.

> +	for (i = 0; i < 3; i++) {
> +		w83791d_write(client, W83791D_REG_BEEP_CTRL[i], val);
> +		val >>= 8;
> +	}

I think Rudolf told you to omit the "& 0xff" masking... but I don't
agree. I think the code is much clearer with the masking, and less
likely to break on future changes too. So feel free to add the mask
back.

> +static ssize_t store_vrm_reg(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct w83791d_data *data = i2c_get_clientdata(client);
> +	u32 val;
> +
> +	val = simple_strtoul(buf, NULL, 10);
> +	mutex_lock(&data->update_lock);
> +	data->vrm = val;
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}

You don't actually need to lock here. vrm is internal to the driver,
it's not read from a chip register.

> +	(*sub_cli) = sub_client > +			kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
> (...)
> +	sub_client->flags = 0;

Not needed, as kzalloc already zeroed the whole structure.

> +	new_client->flags = 0;

Ditto. Also, I would appreciate if you could rename "new_client" to
just "client" in the detect function. I wish the first driver did not
use "new_client", so we wouldn't have it in all drivers now...

> +	data->valid = 0;

And here too.

> +	if (kind < 0) {
> +		if (w83791d_read(new_client, W83791D_REG_CONFIG) & 0x80) {
> +			dev_dbg(dev, "Detection failed at step 3\n");
> +			goto error1;
> +		}
> +		val1 = w83791d_read(new_client, W83791D_REG_BANK);
> +		val2 = w83791d_read(new_client, W83791D_REG_CHIPMAN);
> +		/* Check for Winbond ID if in bank 0 */
> +		if (!(val1 & 0x07)) {
> +			/* yes it is Bank0 */
> +			if (((!(val1 & 0x80)) && (val2 != 0xa3)) ||
> +			    ((val1 & 0x80) && (val2 != 0x5c))) {
> +				goto error1;
> +			}
> +		}
> +		/* If Winbond chip, address of chip and W83791D_REG_I2C_ADDR
> +		   should match */
> +		if (w83791d_read(new_client, W83791D_REG_I2C_ADDR) != address) {
> +			dev_dbg(dev, "Detection failed at step 5\n");
> +			goto error1;
> +		}
> +	}

Please renumber the debug messages, and add one for the second possible
failure. I have a pending patch doing that for the w83792d driver:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/i2c/hwmon-w83792d-quiet-on-misdetection.patch

> +	/* A few vars need to be filled upon startup */
> +	for (i = 1; i <= NUMBER_OF_FANIN; i++) {
> +		data->fan_min[i - 1] = w83791d_read(new_client,
> +						W83791D_REG_FAN_MIN[i]);
> +	}

Isn't it a nice off-by-one I'm spotting here? Please fix!

+	for (i = 0; i < 2; i++) {
+		device_create_file(dev, &sda_beep_ctrl[i].dev_attr);
+	}

You could use ARRAY_SIZE(sda_beep_ctrl) instead of hardcoding 2. This
is easier to understand for the reader, and more robust to future
changes too. I'm not sure if this array really adds anything though.

> +static int w83791d_read(struct i2c_client *client, u8 reg)
> +{
> +	return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +static int w83791d_write(struct i2c_client *client, u8 reg, u8 value)
> +{
> +	return i2c_smbus_write_byte_data(client, reg, value);
> +}

Please inline these.
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h
0ab7fe4c009c40dc485731f9ad98e1d336ddae

> +	temp2_cfg = w83791d_read(client, W83791D_REG_TEMP2_CONFIG);
> +	temp3_cfg = w83791d_read(client, W83791D_REG_TEMP3_CONFIG);
> +	w83791d_write(client, W83791D_REG_TEMP2_CONFIG, temp2_cfg & 0xe6);
> +	w83791d_write(client, W83791D_REG_TEMP3_CONFIG, temp3_cfg & 0xe6);

What are these doing? This needs a comment. Also note that you could do
with a single temporary variable (or even without one.)

> +	if (time_after(jiffies, data->last_updated + (HZ * 3))
> +			|| time_before(jiffies, data->last_updated)
> +			|| !data->valid) {

The time_before() check isn't needed. time_after() takes care of the
jiffies count wrapping on its own. Looks like the w83792d driver is
broken in that respect, a patch would be welcome. See the lm90 driver
for the proper way.

> +	dev_dbg(dev, "beep_enable is: 0x%08x\n", data->beep_enable);

0x%08x to display an u8?

> +	dev_dbg(dev, "vid is: 0x%04x\n", data->vid);
> +	dev_dbg(dev, "vrm is: 0x%04x\n", data->vrm);

These are u8 too.

> +}
> +
> +#endif

No blank line here please.

> +MODULE_AUTHOR("Charles Spirakis<bezaur at gmail.com>");

Add a space between your name and the address.

> +MODULE_DESCRIPTION("W83791D driver for linux-2.6");

The "for linux-2.6" is a bit redundant ;)

OK, that's it. Please fix the issues I mentioned (or explain why you
won't if you don't agree with my comments.) Then resubmit the driver,
and I should accept it and get it merged. Thanks for the good work!

Other things which need to be done about this driver:

* Userspace part. Does libsensors handle this driver properly already?

* Linux 2.4. For now the W83791D chip is handled by the w83781d driver.
This might be a bit confusing to the user that we have a dedicated
driver in 2.6 and not in 2.4. OTOH I don't have the time to split
support to a separate driver myself. If anyone wants to do it, this is
welcome, but probably not required.

* sensors-detect needs to be updated, as for now it points the W83791D
owners to the w83781d driver regardless of the kernel version. Patch
anyone?

* The website needs to be updated to mention the new driver. I'll take
care of that.

Thanks again,
-- 
Jean Delvare

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Charles Spirakis]

Jean --

Thanks for taking the time to look this over. I long ago learned the
value of code reviews and got another reminder on this one  :)

In general, I have no issues with the changes you are proposing. I do
have a couple of questions/comments (below):

On 4/5/06, Jean Delvare <khali at linux-fr.org> wrote:
...
> In a more general way, you may want to take a look at the recent
> changes which were made to the w83792d driver, and see if they could
> apply to your code too.
I'll grab an older kernel tree and compare this file with a recent git
tree. Are there more changes that are not yet in git20 that I should
be aware of? Should I be comparing against a different tree (like the
-mm tree) or is there a special lmsensor archive/tree?

>
> > +#define show_fan_reg(reg) \
> > +static ssize_t show_##reg(struct device *dev, struct device_attribute *attr, \
> > +                             char *buf) \
> > +{ \
> > +     struct sensor_device_attribute *sensor_attr = \
> > +                                             to_sensor_dev_attr(attr); \
> > +     int nr = sensor_attr->index - 1; \
>
> You could define the sysfs files attribute differently so that you
> don't need to apply that offset on every access to every file. That
> would be both shorter and more efficient.
I was following what the w83792d driver was doing in this regard.
Also, having the index start with one was a little easier mentally
during debugging for the fans and temps as the sysfs names starts with
one. I will change to zero based...

>
> > +     default:
> > +             dev_warn(dev, "store_fan_div: Unexpected nr seen: %d\n", nr);
> > +             count = -EINVAL;
> > +             goto err_exit;
>
> This is so unexpected that this just cannot happen, unless you have an
> internal (code) error. So this part should either not exist at all (now
> that the driver is working) or be only compiled in in debug mode.
Will #ifdef  DEBUG this. Having this saved me much grief during
debugging and I'd hate to make a code change in the future that falls
through and not notice it...

> > +     /* A few vars need to be filled upon startup */
> > +     for (i = 1; i <= NUMBER_OF_FANIN; i++) {
> > +             data->fan_min[i - 1] = w83791d_read(new_client,
> > +                                             W83791D_REG_FAN_MIN[i]);
> > +     }
>
> Isn't it a nice off-by-one I'm spotting here? Please fix!
Guilty your honor. I throw myself upon the mercy of the court...

>
> +       for (i = 0; i < 2; i++) {
> +               device_create_file(dev, &sda_beep_ctrl[i].dev_attr);
> +       }
>
> You could use ARRAY_SIZE(sda_beep_ctrl) instead of hardcoding 2. This
> is easier to understand for the reader, and more robust to future
> changes too. I'm not sure if this array really adds anything though.
It seemed like the array structure "grouped" things a bit and provided
some consistency given all the other "groups" were now arrays
(voltage, fan, temp). I can go either way on this if you have a
preference.

> > +     temp2_cfg = w83791d_read(client, W83791D_REG_TEMP2_CONFIG);
> > +     temp3_cfg = w83791d_read(client, W83791D_REG_TEMP3_CONFIG);
> > +     w83791d_write(client, W83791D_REG_TEMP2_CONFIG, temp2_cfg & 0xe6);
> > +     w83791d_write(client, W83791D_REG_TEMP3_CONFIG, temp3_cfg & 0xe6);
>
> What are these doing? This needs a comment. Also note that you could do
> with a single temporary variable (or even without one.)
>
It is making sure the two temp sensors are enabled while preserving
the reserved bits (the w83792 driver is doing this also). If you reset
the chip the HW takes care of this. Guess I can remove the code and
assume the BIOS is doing the right thing when reset=0...

> OK, that's it. Please fix the issues I mentioned (or explain why you
> won't if you don't agree with my comments.) Then resubmit the driver,
> and I should accept it and get it merged. Thanks for the good work!
>
> Other things which need to be done about this driver:
>
> * Userspace part. Does libsensors handle this driver properly already?
I'm using sensors 2.9.2 and the corresponding libsensors and things
seem to work. I can update to the latest release to verify.

>
> * Linux 2.4. For now the W83791D chip is handled by the w83781d driver.
> This might be a bit confusing to the user that we have a dedicated
> driver in 2.6 and not in 2.4. OTOH I don't have the time to split
> support to a separate driver myself. If anyone wants to do it, this is
> welcome, but probably not required.
>
> * sensors-detect needs to be updated, as for now it points the W83791D
> owners to the w83781d driver regardless of the kernel version. Patch
> anyone?
Yes, it detects the w83791d correctly and tells you to load the w83781d driver:

Driver `w83781d' (should be inserted):
  Detects correctly:
  * Bus `SMBus I801 adapter at 0540'
    Busdriver `i2c-i801', I2C address 0x2c (and 0x48 0x49)
    Chip `Winbond W83791D' (confidence: 7)

I haven't looked at that code/scripts, but I can update depending on
how you want to handle it (check for 2.4 vs. 2.6 and output as
appropriate?)

-- charles

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Ymu]

Hi Charles,

Thanks for your hardwork first:)
 i have one question about your mail.
 
> > > +     temp2_cfg = w83791d_read(client, W83791D_REG_TEMP2_CONFIG);
> > > +     temp3_cfg = w83791d_read(client, W83791D_REG_TEMP3_CONFIG);
> > > +     w83791d_write(client, W83791D_REG_TEMP2_CONFIG, 
> temp2_cfg & 0xe6);
> > > +     w83791d_write(client, W83791D_REG_TEMP3_CONFIG, 
> temp3_cfg & 0xe6);
> >
> > What are these doing? This needs a comment. Also note that 
> you could do
> > with a single temporary variable (or even without one.)
> >
> It is making sure the two temp sensors are enabled while preserving
> the reserved bits (the w83792 driver is doing this also). If you reset
> the chip the HW takes care of this. Guess I can remove the code and
> assume the BIOS is doing the right thing when reset=0...
> 

I dont like this "reset" or "init", i would like to drop this parameter
and do nothing about temp2,temp3 configuration.
because we may lose MB specific configuration if we reset the chip,
What's your opinion?

Best Regards
Yuan Mu


=============================================The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such  a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Winbond is strictly prohibited; and any information in this email irrelevant to the official business of Winbond shall be deemed as neither given nor endorsed by Winbond.

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Ymu]

Hi Charles,

Thanks for your hardwork first:)
 i have one question about your mail.
 
> > > +     temp2_cfg = w83791d_read(client, W83791D_REG_TEMP2_CONFIG);
> > > +     temp3_cfg = w83791d_read(client, W83791D_REG_TEMP3_CONFIG);
> > > +     w83791d_write(client, W83791D_REG_TEMP2_CONFIG, 
> temp2_cfg & 0xe6);
> > > +     w83791d_write(client, W83791D_REG_TEMP3_CONFIG, 
> temp3_cfg & 0xe6);
> >
> > What are these doing? This needs a comment. Also note that 
> you could do
> > with a single temporary variable (or even without one.)
> >
> It is making sure the two temp sensors are enabled while preserving
> the reserved bits (the w83792 driver is doing this also). If you reset
> the chip the HW takes care of this. Guess I can remove the code and
> assume the BIOS is doing the right thing when reset=0...
> 

I dont like this "reset" or "init", i would like to drop this parameter
and do nothing about temp2,temp3 configuration.
because we may lose MB specific configuration if we reset the chip,
What's your opinion?

Best Regards
Yuan Mu


=============================================The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such  a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Winbond is strictly prohibited; and any information in this email irrelevant to the official business of Winbond shall be deemed as neither given nor endorsed by Winbond.

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Jean Delvare]

Hi Yuan,

> > It is making sure the two temp sensors are enabled while preserving
> > the reserved bits (the w83792 driver is doing this also). If you reset
> > the chip the HW takes care of this. Guess I can remove the code and
> > assume the BIOS is doing the right thing when reset=0...
> 
> I dont like this "reset" or "init", i would like to drop this parameter
> and do nothing about temp2,temp3 configuration.
> because we may lose MB specific configuration if we reset the chip,
> What's your opinion?

I share your worries about possibly losing the configuration set by the
BIOS. I believe that the BIOS should always configure the hardware
monitoring chips properly (because they know exactly what features the
board has) and even if most BIOSes unfortunately don't do that, we must
respect the ones that do.

Now, having a module parameter to control what kind of reset and/or
initialization is done is fine by me, as long as the default is to
preserve the BIOS settings as much as possible.

So, enabling temp2 and temp3 should indeed not be done unconditionally.

Charles, you might want to have three degrees of initialization: only
start monitoring, or enable temp2 and temp3 beforehand, or reset the
chip beforehand. This can be done using two boolean module parameters
(reset and init), or just one parameter (init) with three values, at
your option.

-- 
Jean Delvare

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Jean Delvare]

Hi Charles,

> Thanks for taking the time to look this over. I long ago learned the
> value of code reviews and got another reminder on this one  :)

I'm doing my best with the little time I have now... I know there are
several drivers waiting for me to review them, but I just can't do
everything at once.

> > In a more general way, you may want to take a look at the recent
> > changes which were made to the w83792d driver, and see if they could
> > apply to your code too.
>
> I'll grab an older kernel tree and compare this file with a recent git
> tree. Are there more changes that are not yet in git20 that I should
> be aware of? Should I be comparing against a different tree (like the
> -mm tree) or is there a special lmsensor archive/tree?

Well you can use the web interface to git to get the history of the
w83792d driver:
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=history;f=drivers/hwmon/w83792d.c

As for the pending patches, there's only one I am aware of, and I
already pointed you to it if I remember correctly:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/i2c/hwmon-w83792d-quiet-on-misdetection.patch

> > +       for (i = 0; i < 2; i++) {
> > +               device_create_file(dev, &sda_beep_ctrl[i].dev_attr);
> > +       }
> >
> > You could use ARRAY_SIZE(sda_beep_ctrl) instead of hardcoding 2. This
> > is easier to understand for the reader, and more robust to future
> > changes too. I'm not sure if this array really adds anything though.
>
> It seemed like the array structure "grouped" things a bit and provided
> some consistency given all the other "groups" were now arrays
> (voltage, fan, temp). I can go either way on this if you have a
> preference.

No strong preference, do it the way you want.

> > > +     temp2_cfg = w83791d_read(client, W83791D_REG_TEMP2_CONFIG);
> > > +     temp3_cfg = w83791d_read(client, W83791D_REG_TEMP3_CONFIG);
> > > +     w83791d_write(client, W83791D_REG_TEMP2_CONFIG, temp2_cfg & 0xe6);
> > > +     w83791d_write(client, W83791D_REG_TEMP3_CONFIG, temp3_cfg & 0xe6);
> >
> > What are these doing? This needs a comment. Also note that you could do
> > with a single temporary variable (or even without one.)
>
> It is making sure the two temp sensors are enabled while preserving
> the reserved bits (the w83792 driver is doing this also). If you reset
> the chip the HW takes care of this. Guess I can remove the code and
> assume the BIOS is doing the right thing when reset=0...

See my other post (answering to Yuan) for this.

> > * sensors-detect needs to be updated, as for now it points the W83791D
> > owners to the w83781d driver regardless of the kernel version. Patch
> > anyone?
>
> Yes, it detects the w83791d correctly and tells you to load the w83781d driver:
> 
> Driver `w83781d' (should be inserted):
>   Detects correctly:
>   * Bus `SMBus I801 adapter at 0540'
>     Busdriver `i2c-i801', I2C address 0x2c (and 0x48 0x49)
>     Chip `Winbond W83791D' (confidence: 7)
> 
> I haven't looked at that code/scripts, but I can update depending on
> how you want to handle it (check for 2.4 vs. 2.6 and output as
> appropriate?)

I haven't look in deep either, but I remember that we have a similar
case for one SiS SMBus driver: it's named i2c-sis645 on Linux 2.4 and
i2c-sis96x on Linux 2.6, and we have some code to pick the right driver
depending on the kernel version. So let's do it the same way for the
W83791D case and it should be fine.

Thanks,
-- 
Jean Delvare

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Charles Spirakis]

From: Charles Spirakis <bezaur at gmail.com>

Add support for the w83791d sensor chip. The w83791d hardware is
somewhere between the w83781d and the w83792d and this driver code
is derived from the code that supports those chips.

Signed-off by: Charles Spirakis <bezaur at gmail.com>

---

 Documentation/hwmon/w83791d |  113 +++
 drivers/hwmon/Kconfig       |   10
 drivers/hwmon/Makefile      |    1
 drivers/hwmon/w83791d.c     | 1255 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 1379 insertions(+)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: w83791d_support.patch
Type: text/x-patch
Size: 46365 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060412/487f9186/w83791d_support-0001.bin

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Jean Delvare]

Hi Charles,

> Add support for the w83791d sensor chip. The w83791d hardware is
> somewhere between the w83781d and the w83792d and this driver code
> is derived from the code that supports those chips.

Applied, thanks. Good work!

Any future changes to the driver should be sent as incremental patches.

-- 
Jean Delvare

[lm-sensors] [PATCH] hwmon: Add w83791d support

[Charles Spirakis]

Will do.

I'll start work on the user-mode patch for detection and the per-file
alarm patches now that this is done.

Thanks for your eyes and your time!

-- charles


On 4/11/06, Jean Delvare <khali at linux-fr.org> wrote:
> Hi Charles,
>
> > Add support for the w83791d sensor chip. The w83791d hardware is
> > somewhere between the w83781d and the w83792d and this driver code
> > is derived from the code that supports those chips.
>
> Applied, thanks. Good work!
>
> Any future changes to the driver should be sent as incremental patches.
>
> --
> Jean Delvare
>