[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

I am working with Debian SARGE on a 2.6.8-2 kernel installation and, thanks
to Aaron Marsh, I now have a working vt8231 driver in my system.  (If anyone
has had problems compiling it for Debian Sarge then I can happily mail you a
copy of mine
).

 

I noticed that the vt8231 driver would not start if the i22-viapro module
was installed. I suspect this is because they both try and control the same
PCI device (the VT8231 chip) and therefore you can only have one installed
at a time.  If you install the i2c-viapro first then the vt8231 driver never
gets started and you never get the temp/voltage information.

 

The i2c-viaprobe driver allows the i2c bus to be accessed, so if it is
installed then I can see the RAMs that are on the bus.  If I remove the
i2c-viapro and install the vt8231 then I get the voltage and temperature
information.

 

Here are my questions:

 

1)       Does anyone know what the plan is for combining the vt8231 and
i2c-viapro modules so that a single module can access both the I2C bus and
the temperature/voltage information at the same time ?

2)       Does anyone know the CORRECT information for the scaling
coefficients for the temperature and voltage ?

 

If it helps any, below are the raw numbers that I get from my box:

 

edensnake:~# cat /sys/bus/i2c/drivers/vt8231/1-6000/*_input

0                                  // fan1

0                                  // fan2

204                               // in0

202                               // in1

201

120

213

201                               // in5

2040                             //temp2

747                               //temp3

0

0

0

0                                  // temp7

edensnake:~# 

 

They seem reasonable given the range that things should be running over, but
I don?t want to try and guess at the correct conversion table.  I have tried
the table from Aaron Marsh?s site
(http://www.stanford.edu/~amarsh/vt8231/sensors.conf) but that gives me the
result below, which looks a bit wrong to me
 (although they do seem to
fairly closely match what others have seen:
http://archives.andrew.net.au/lm-sensors/msg18667.html)

 

edensnake:~# sensors

vt8231-isa-6000

Adapter: ISA adapter

VCore1:    +0.30 V  (min =  -0.05 V, max =  +0.39 V)

+5V:       +0.23 V  (min =  -0.08 V, max =  +0.56 V)

+12V:      +1.09 V  (min =  -0.18 V, max =  +1.34 V)

fan1:        0 RPM  (min =    0 RPM, div = 2)

fan2:        0 RPM  (min =    0 RPM, div = 2)

ERROR: Can't get TEMP2 data!

Proc Temp:  -1.4?C  (high =    -3?C, hyst =    -1?C)

ERROR: Can't get TEMP4 data!

ERROR: Can't get TEMP5 data!

ERROR: Can't get TEMP6 data!

ERROR: Can't get TEMP7 data!

vid:      +2.050 V  (VRM Version 8.2)

edensnake:~#

 

Thanks in advance,

 

Roger Lucas

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20051012/6e02b885/attachment.html

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

I have done some more digging including probing voltages on the Eden 5000
MoBo.

 

IN0 is connected to the 12v rail using a potential divider from R54/R55.
The scaling factor is 0.1754.

IN1 is connected to the 5v rail using a potential divider from R51/R52.  The
scaling factor is 0.4167.

IN2 is connected to the 2.5v rail using a potential divider from R56/R57.
The scaling factor is 0.833.

IN3 is directly connected to the Vcore rail (1.2v).  The scaling factor is
1.000.

IN4 is unknown

IN5 is connected to the 3.3v rail within the VT8231.  The scaling factor is
0.6296.

 

The information in the sensors.conf file from Aaron Marsh is basically
correct here, but the INx values are in the wrong sequence.  This is
probably because he has set them up for a different motherboard.

 

The temperature information in TEMP2 appears to be useless as it is the
full-scale value and never changes.

 

The information in TEMP3 seems to be very different to that from any
information I could find on the WWW.  After placing a temperature probe at
the base of the CPU heatsink, then heating the whole heatsink with a
hairdryer and waiting for everything to stabilise, I was able to generate a
graph of the TEMP3 value against the register value (see below chart).

 


Temp (centigrade)

Register Value


33.9

725


34.1

712


34.8

735


35.5

732


36.5

745


40

760


41.6

780


42.5

780


44

807


45.6

817


46.1

825


46.3

812


47.1

820


47.9

830


50

865


52.9

862


53

887


53.9

870


55

882


56

890


57.1

907


58

925


61

935


62

942


63

962


64.2

950


65.5

975


66.9

980


70.5

997


70.5

997

 

This is basically linear (y=mx+c).  If temp=y, register=x then m=0.128 and
c=-58.

 

I?m not posting a sensors.conf file as I cannot get the ?sensors?+?vt8231?
combination to work properly.  For some reason, sensors insists on scaling
all the input values by 1/1000 and vt8231.c handles reads/writes of the max
and min settings differently (it divides all writes to max or min by 10
before applying them).  I cannot see any reason for this behaviour ? if
someone can enlighten me then please do, but it seems a bit weird from where
I am standing.  Also, ?sensors? complains about not being able to access
procfs/sysfs for writing even though the file system exists and is writable
(I have written the files directly when testing), even though it does manage
to write some of the max/min settings.

 

Given that lm-sensors is going through a major re-write through kernel
2.6.11 and the vt8231/viapro functionality isn?t integrated, it seems
pointless to expend major effort here as it will soon be obsolete ? I am
using a script and accessing the /sys/bus/i2c/
.. files directly to
determine what is going on.  I plan to move to the 2.6.11+ kernel as soon as
Debian takes it to stable from testing (I cannot install testing on my board
as the install crashes).

 

Hopefully this is of some use to someone, as I couldn?t find this kind of
information when I was looking.

 

Regards,

 

Roger

 

  _____  

From: lm-sensors-bounces@lm-sensors.org
[mailto:lm-sensors-bounces@lm-sensors.org] On Behalf Of Roger Lucas
Sent: 12 October 2005 15:22
To: lm-sensors@lm-sensors.org
Subject: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

 

I am working with Debian SARGE on a 2.6.8-2 kernel installation and, thanks
to Aaron Marsh, I now have a working vt8231 driver in my system.  (If anyone
has had problems compiling it for Debian Sarge then I can happily mail you a
copy of mine
).

 

I noticed that the vt8231 driver would not start if the i22-viapro module
was installed. I suspect this is because they both try and control the same
PCI device (the VT8231 chip) and therefore you can only have one installed
at a time.  If you install the i2c-viapro first then the vt8231 driver never
gets started and you never get the temp/voltage information.

 

The i2c-viaprobe driver allows the i2c bus to be accessed, so if it is
installed then I can see the RAMs that are on the bus.  If I remove the
i2c-viapro and install the vt8231 then I get the voltage and temperature
information.

 

Here are my questions:

 

1)       Does anyone know what the plan is for combining the vt8231 and
i2c-viapro modules so that a single module can access both the I2C bus and
the temperature/voltage information at the same time ?

2)       Does anyone know the CORRECT information for the scaling
coefficients for the temperature and voltage ?

 

If it helps any, below are the raw numbers that I get from my box:

 

edensnake:~# cat /sys/bus/i2c/drivers/vt8231/1-6000/*_input

0                                  // fan1

0                                  // fan2

204                               // in0

202                               // in1

201

120

213

201                               // in5

2040                             //temp2

747                               //temp3

0

0

0

0                                  // temp7

edensnake:~# 

 

They seem reasonable given the range that things should be running over, but
I don?t want to try and guess at the correct conversion table.  I have tried
the table from Aaron Marsh?s site
(http://www.stanford.edu/~amarsh/vt8231/sensors.conf) but that gives me the
result below, which looks a bit wrong to me
 (although they do seem to
fairly closely match what others have seen:
http://archives.andrew.net.au/lm-sensors/msg18667.html)

 

edensnake:~# sensors

vt8231-isa-6000

Adapter: ISA adapter

VCore1:    +0.30 V  (min =  -0.05 V, max =  +0.39 V)

+5V:       +0.23 V  (min =  -0.08 V, max =  +0.56 V)

+12V:      +1.09 V  (min =  -0.18 V, max =  +1.34 V)

fan1:        0 RPM  (min =    0 RPM, div = 2)

fan2:        0 RPM  (min =    0 RPM, div = 2)

ERROR: Can't get TEMP2 data!

Proc Temp:  -1.4?C  (high =    -3?C, hyst =    -1?C)

ERROR: Can't get TEMP4 data!

ERROR: Can't get TEMP5 data!

ERROR: Can't get TEMP6 data!

ERROR: Can't get TEMP7 data!

vid:      +2.050 V  (VRM Version 8.2)

edensnake:~#

 

Thanks in advance,

 

Roger Lucas

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20051013/291d02a8/attachment-0001.html

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

> I noticed that the vt8231 driver would not start if the i22-viapro
> module was installed. I suspect this is because they both try and
> control the same PCI device (the VT8231 chip) and therefore you can only
> have one installed at a time.  If you install the i2c-viapro first then
> the vt8231 driver never gets started and you never get the temp/voltage
> information.

Yes. Correct. This should be fixed in recent kernels. (from 2.6.11)

> 1)       Does anyone know what the plan is for combining the vt8231 and
> i2c-viapro modules so that a single module can access both the I2C bus
> and the temperature/voltage information at the same time ?

Yes see above. It should be good to free the vt8231 from usage of PCI device too:
Like done in this patch:
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-February/010440.html

> 
> 2)       Does anyone know the CORRECT information for the scaling
> coefficients for the temperature and voltage ?

No, please ask the manufacturer of motherboard.

Aaron,
Have you some more recent version of your driver somewhere (seems you do not)?  I think it would help people if we could get it into mainline.
Thanks,

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Aaron Marburg]

> Aaron,
> Have you some more recent version of your driver somewhere (seems
> you do not)?  I think it would help people if we could get it into
> mainline.
> Thanks,
> 
> Regards
> Rudolf

Hello Rudolf,

I do not have a patch for the (vt8231) driver for more recent kernels.
My EPIA-800 based development board was running Windows for the last
year or so.  It is back under Linux again, however, and I should be
able to make the necessary patches when I have some spare time.

- Aaron

-- 
amarburg@notetofutureself.org

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Roger,

Sorry for the delay.

> I noticed that the vt8231 driver would not start if the i2c-viapro module
> was installed. I suspect this is because they both try and control the same
> PCI device (the VT8231 chip) and therefore you can only have one installed
> at a time.  If you install the i2c-viapro first then the vt8231 driver never
> gets started and you never get the temp/voltage information.
> 
> The i2c-viapro driver allows the i2c bus to be accessed, so if it is
> installed then I can see the RAMs that are on the bus.  If I remove the
> i2c-viapro and install the vt8231 then I get the voltage and temperature
> information.

There was a similar conflict between i2c-viapro and via686a, but it was
solved in Linux 2.6.10. These drivers do not actually use the PCI
device, they only need it to read some resource values at load time, so
they now release the PCI device when they are done. That way, both
drivers can be loaded in sequence.

A similar change would be required for your vt8231 driver. Just look at
how the via686a driver does in Linux 2.6.14, and do the same. That
should work just fine.

> 1)       Does anyone know what the plan is for combining the vt8231 and
> i2c-viapro modules so that a single module can access both the I2C bus and
> the temperature/voltage information at the same time ?

We were able to fix the issue in a different way (see above) so merging
the drivers won't be necessary. Else we would have had to merge the
i2c-viapro, vt8231, vt1211 and via686a drivers all in one. What a mess!

-- 
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Roger,

Please answer to the list rather than to me only.

> I have a patch for the vt8231 driver now (attached).  It seems OK but I get
> a compilation warning:
> 
> drivers/hwmon/vt8231.c:63: warning: `addr_data' defined but not used
> 
> This is generated because I am using the macro "I2C_CLIENT_INSMOD_1(vt8231)"
> to register the module, but then never use the addr_data structure that is
> created by it.
> 
> I cannot see a reason to use this structure, so I suppose I should be using
> a different macro to register the driver...  Any thoughts ?

Don't use I2C_CLIENT_INSMOD_1. The VT8231 is not an I2C chip. It is
attached to the i2c subsystem for historical reasons, but we are trying
to separate such chips from the i2c subsystem now.

A few additional comments about your code:

> +config SENSORS_VT8231
> +	tristate "VT8231"
> +	depends on HWMON && I2C && PCI

You should make it depend on EXPERIMENTAL too.

> +/* Supports VIA VT8231 South Bridge embedded sensors 
> +**
> +** Change log:
> +**
> +**  2005-11-01: Roger Lucas (roger@planbit.co.uk)
> +**     Updated to work with the 2.6.14 kernel that I have.  This is the first
> +**     lm-sensors port that I have done, so there may be nasty mistakes in it.
> +**     That having been said, insmod and rmmod seem to work cleanly on my 
> +**	   system and the values reported are reasonable.
> +**     I merged the Aaron Marsh vt8231 port with the most recent via686a driver 
> +**     in the 2.6.14 kernel source to create this code.
> +**     VRM doesn't work on the Via Epia 5000 that I have as the kernel call
> +**	   vid_which_vrm() returns -1.  Apparently the Via CPU is unrecognised....
> +*/

Changelog doesn't belong to the code. You can add whatever comment you
want at the top of the patch, and it'll be used in the git commit.

The vid_which_vrm issue is interesting. Rudolf, can you please look
into it?

> +static int force_addr = 0;

Do not explicitely intialize static globals to 0, the compiler does it
for you.

> +MODULE_PARM(force_addr, "i");

Please use module_param instead. MODULE_PARM is deprecated.

> +static unsigned short normal_i2c[] = { I2C_CLIENT_END };

You shouldn't need this one.

> +static DEVICE_ATTR(alarms, S_IRUGO | S_IWUSR, show_alarms, NULL);

S_IWUSR is not correct, that file is read-only.

> +static DEVICE_ATTR(in0_ref, S_IRUGO, show_vid, NULL);

This attribute has been renamed to cpu0_vid.

> +        struct vt8231_data *data = vt8231_update_device(dev);

Indentation issue, you use spaces instead of a tab. Same in other
places, please fix them all.

> +	return sprintf(buf,"%ld\n", (long)data->vrm);

Please use a space after the comma. Same in other places, please fix
them all.

> +	printk("Setting VRM to \"%s\" = %d\n", buf, val );

No space before closing parenthesis.

> +	if (!request_region(isa_address, VT8231_EXTENT, "vt8231-sensors")) {

Please use the driver name to request the region.

> +	if (!(data = kmalloc(sizeof(struct vt8231_data), GFP_KERNEL))) {
> +		err = -ENOMEM;
> +		goto exit_release;
> +	}
> +
> +	memset(data, 0, sizeof(struct vt8231_data));

Please use kzalloc instead of kmalloc + memset.

> +	snprintf(new_client->name, I2C_NAME_SIZE, type_name);

strlcpy will do the same, more efficiently.

> +	/* TODO: vid and vrm */
> +       	device_create_file(&new_client->dev, &dev_attr_in0_ref);
> +	device_create_file(&new_client->dev, &dev_attr_vrm);

Bad indentation. Why this "TODO" comment?

This was only a quick and partial review, I've been reviewing too much
code today already, I'll go mad if I do more.

One last general comment: please cut long lines so that the code fits
in 80 columns.

Thanks,
-- 
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Attached is the updated patch file for the vt8231 with kernel 2.6.14.

I believe that this has all the corrections requested by Jean in his mail
below. Thanks for reviewing it, Jean, your quick response was appreciated. 

It insmod's, runs and rmmod's OK on my Via Epia 5000 box, but it needs
testing on a wider range of machines if possible...

- Roger

-----Original Message-----
From: Jean Delvare [mailto:khali@linux-fr.org] 
Sent: 01 November 2005 17:27
To: Roger Lucas
Cc: Rudolf Marek; LM Sensors
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

Hi Roger,

Please answer to the list rather than to me only.

> I have a patch for the vt8231 driver now (attached).  It seems OK but I
get
> a compilation warning:
> 
> drivers/hwmon/vt8231.c:63: warning: `addr_data' defined but not used
> 
> This is generated because I am using the macro
"I2C_CLIENT_INSMOD_1(vt8231)"
> to register the module, but then never use the addr_data structure that is
> created by it.
> 
> I cannot see a reason to use this structure, so I suppose I should be
using
> a different macro to register the driver...  Any thoughts ?

Don't use I2C_CLIENT_INSMOD_1. The VT8231 is not an I2C chip. It is
attached to the i2c subsystem for historical reasons, but we are trying
to separate such chips from the i2c subsystem now.

A few additional comments about your code:

> +config SENSORS_VT8231
> +	tristate "VT8231"
> +	depends on HWMON && I2C && PCI

You should make it depend on EXPERIMENTAL too.

> +/* Supports VIA VT8231 South Bridge embedded sensors 
> +**
> +** Change log:
> +**
> +**  2005-11-01: Roger Lucas (roger@planbit.co.uk)
> +**     Updated to work with the 2.6.14 kernel that I have.  This is the
first
> +**     lm-sensors port that I have done, so there may be nasty mistakes
in it.
> +**     That having been said, insmod and rmmod seem to work cleanly on my

> +**	   system and the values reported are reasonable.
> +**     I merged the Aaron Marsh vt8231 port with the most recent via686a
driver 
> +**     in the 2.6.14 kernel source to create this code.
> +**     VRM doesn't work on the Via Epia 5000 that I have as the kernel
call
> +**	   vid_which_vrm() returns -1.  Apparently the Via CPU is
unrecognised....
> +*/

Changelog doesn't belong to the code. You can add whatever comment you
want at the top of the patch, and it'll be used in the git commit.

The vid_which_vrm issue is interesting. Rudolf, can you please look
into it?

> +static int force_addr = 0;

Do not explicitely intialize static globals to 0, the compiler does it
for you.

> +MODULE_PARM(force_addr, "i");

Please use module_param instead. MODULE_PARM is deprecated.

> +static unsigned short normal_i2c[] = { I2C_CLIENT_END };

You shouldn't need this one.

> +static DEVICE_ATTR(alarms, S_IRUGO | S_IWUSR, show_alarms, NULL);

S_IWUSR is not correct, that file is read-only.

> +static DEVICE_ATTR(in0_ref, S_IRUGO, show_vid, NULL);

This attribute has been renamed to cpu0_vid.

> +        struct vt8231_data *data = vt8231_update_device(dev);

Indentation issue, you use spaces instead of a tab. Same in other
places, please fix them all.

> +	return sprintf(buf,"%ld\n", (long)data->vrm);

Please use a space after the comma. Same in other places, please fix
them all.

> +	printk("Setting VRM to \"%s\" = %d\n", buf, val );

No space before closing parenthesis.

> +	if (!request_region(isa_address, VT8231_EXTENT, "vt8231-sensors")) {

Please use the driver name to request the region.

> +	if (!(data = kmalloc(sizeof(struct vt8231_data), GFP_KERNEL))) {
> +		err = -ENOMEM;
> +		goto exit_release;
> +	}
> +
> +	memset(data, 0, sizeof(struct vt8231_data));

Please use kzalloc instead of kmalloc + memset.

> +	snprintf(new_client->name, I2C_NAME_SIZE, type_name);

strlcpy will do the same, more efficiently.

> +	/* TODO: vid and vrm */
> +       	device_create_file(&new_client->dev, &dev_attr_in0_ref);
> +	device_create_file(&new_client->dev, &dev_attr_vrm);

Bad indentation. Why this "TODO" comment?

This was only a quick and partial review, I've been reviewing too much
code today already, I'll go mad if I do more.

One last general comment: please cut long lines so that the code fits
in 80 columns.

Thanks,
-- 
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Roger,

> Attached is the updated patch file for the vt8231 with kernel 2.6.14.

It did not made it through the list. The list is configured to only
accept attachements of type text/plain and text/x-patch. Please try
resending the patch using one of these types.

Thanks,
-- 
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Retry... Hopefully Outlook will attach it as text since it now has a .TXT
extension...

-----Original Message-----
From: lm-sensors-bounces@lm-sensors.org
[mailto:lm-sensors-bounces@lm-sensors.org] On Behalf Of Roger Lucas
Sent: 01 November 2005 18:20
To: 'LM Sensors'
Subject: RE: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

Attached is the updated patch file for the vt8231 with kernel 2.6.14.

I believe that this has all the corrections requested by Jean in his mail
below. Thanks for reviewing it, Jean, your quick response was appreciated. 

It insmod's, runs and rmmod's OK on my Via Epia 5000 box, but it needs
testing on a wider range of machines if possible...

- Roger

-----Original Message-----
From: Jean Delvare [mailto:khali@linux-fr.org] 
Sent: 01 November 2005 17:27
To: Roger Lucas
Cc: Rudolf Marek; LM Sensors
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

Hi Roger,

Please answer to the list rather than to me only.

> I have a patch for the vt8231 driver now (attached).  It seems OK but I
get
> a compilation warning:
> 
> drivers/hwmon/vt8231.c:63: warning: `addr_data' defined but not used
> 
> This is generated because I am using the macro
"I2C_CLIENT_INSMOD_1(vt8231)"
> to register the module, but then never use the addr_data structure that is
> created by it.
> 
> I cannot see a reason to use this structure, so I suppose I should be
using
> a different macro to register the driver...  Any thoughts ?

Don't use I2C_CLIENT_INSMOD_1. The VT8231 is not an I2C chip. It is
attached to the i2c subsystem for historical reasons, but we are trying
to separate such chips from the i2c subsystem now.

A few additional comments about your code:

> +config SENSORS_VT8231
> +	tristate "VT8231"
> +	depends on HWMON && I2C && PCI

You should make it depend on EXPERIMENTAL too.

> +/* Supports VIA VT8231 South Bridge embedded sensors 
> +**
> +** Change log:
> +**
> +**  2005-11-01: Roger Lucas (roger@planbit.co.uk)
> +**     Updated to work with the 2.6.14 kernel that I have.  This is the
first
> +**     lm-sensors port that I have done, so there may be nasty mistakes
in it.
> +**     That having been said, insmod and rmmod seem to work cleanly on my

> +**	   system and the values reported are reasonable.
> +**     I merged the Aaron Marsh vt8231 port with the most recent via686a
driver 
> +**     in the 2.6.14 kernel source to create this code.
> +**     VRM doesn't work on the Via Epia 5000 that I have as the kernel
call
> +**	   vid_which_vrm() returns -1.  Apparently the Via CPU is
unrecognised....
> +*/

Changelog doesn't belong to the code. You can add whatever comment you
want at the top of the patch, and it'll be used in the git commit.

The vid_which_vrm issue is interesting. Rudolf, can you please look
into it?

> +static int force_addr = 0;

Do not explicitely intialize static globals to 0, the compiler does it
for you.

> +MODULE_PARM(force_addr, "i");

Please use module_param instead. MODULE_PARM is deprecated.

> +static unsigned short normal_i2c[] = { I2C_CLIENT_END };

You shouldn't need this one.

> +static DEVICE_ATTR(alarms, S_IRUGO | S_IWUSR, show_alarms, NULL);

S_IWUSR is not correct, that file is read-only.

> +static DEVICE_ATTR(in0_ref, S_IRUGO, show_vid, NULL);

This attribute has been renamed to cpu0_vid.

> +        struct vt8231_data *data = vt8231_update_device(dev);

Indentation issue, you use spaces instead of a tab. Same in other
places, please fix them all.

> +	return sprintf(buf,"%ld\n", (long)data->vrm);

Please use a space after the comma. Same in other places, please fix
them all.

> +	printk("Setting VRM to \"%s\" = %d\n", buf, val );

No space before closing parenthesis.

> +	if (!request_region(isa_address, VT8231_EXTENT, "vt8231-sensors")) {

Please use the driver name to request the region.

> +	if (!(data = kmalloc(sizeof(struct vt8231_data), GFP_KERNEL))) {
> +		err = -ENOMEM;
> +		goto exit_release;
> +	}
> +
> +	memset(data, 0, sizeof(struct vt8231_data));

Please use kzalloc instead of kmalloc + memset.

> +	snprintf(new_client->name, I2C_NAME_SIZE, type_name);

strlcpy will do the same, more efficiently.

> +	/* TODO: vid and vrm */
> +       	device_create_file(&new_client->dev, &dev_attr_in0_ref);
> +	device_create_file(&new_client->dev, &dev_attr_vrm);

Bad indentation. Why this "TODO" comment?

This was only a quick and partial review, I've been reviewing too much
code today already, I'll go mad if I do more.

One last general comment: please cut long lines so that the code fits
in 80 columns.

Thanks,
-- 
Jean Delvare
-------------- next part --------------
diff -Naur reference/drivers/hwmon/Kconfig linux-2.6.14/drivers/hwmon/Kconfig
--- reference/drivers/hwmon/Kconfig	2005-10-28 01:02:08.000000000 +0100
+++ linux-2.6.14/drivers/hwmon/Kconfig	2005-11-01 17:35:18.000000000 +0000
@@ -350,6 +350,18 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called via686a.
 
+config SENSORS_VT8231
+	tristate "VT8231"
+	depends on HWMON && I2C && PCI && EXPERIMENTAL
+	select HWMON_VID
+	select I2C_ISA
+	help
+	  If you say yes here then you get support for the integrated sensors
+	  in the Via VT8231 device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called vt8231.
+
 config SENSORS_W83781D
 	tristate "Winbond W83781D, W83782D, W83783S, W83627HF, Asus AS99127F"
 	depends on HWMON && I2C
diff -Naur reference/drivers/hwmon/Makefile linux-2.6.14/drivers/hwmon/Makefile
--- reference/drivers/hwmon/Makefile	2005-10-28 01:02:08.000000000 +0100
+++ linux-2.6.14/drivers/hwmon/Makefile	2005-11-01 10:59:10.000000000 +0000
@@ -40,6 +40,7 @@
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
 obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
+obj-$(CONFIG_SENSORS_VT8231)	+= vt8231.o
 obj-$(CONFIG_SENSORS_W83627EHF)	+= w83627ehf.o
 obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 
diff -Naur reference/drivers/hwmon/vt8231.c linux-2.6.14/drivers/hwmon/vt8231.c
--- reference/drivers/hwmon/vt8231.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.14/drivers/hwmon/vt8231.c	2005-11-01 18:06:17.000000000 +0000
@@ -0,0 +1,785 @@
+/*
+	vt8231.c - Part of lm_sensors, Linux kernel modules
+				for hardware monitoring
+				
+	Copyright (c) 2002 Mark D. Studebaker <mdsxyz123@yahoo.com>
+			   Aaron M. Marsh <amarsh@sdf.lonestar.org>
+
+	This program is free software; you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation; either version 2 of the License, or
+	(at your option) any later version.
+
+	This program is distributed in the hope that it will be useful, 
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with this program; if not, write to the Free Software
+	Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+/* Supports VIA VT8231 South Bridge embedded sensors 
+*/
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/i2c-isa.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-vid.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <asm/io.h>
+
+static int force_addr;
+module_param(force_addr, int,0644);
+MODULE_PARM_DESC(force_addr, "Initialize the base address of the sensors");
+
+/* Device address
+   Note that we can't determine the ISA address until we have initialized
+   our module */
+static unsigned short isa_address;
+
+#define VT8231_EXTENT 0x80
+#define VT8231_BASE_REG 0x70
+#define VT8231_ENABLE_REG 0x74
+
+/* pwm numbered 1-2 */
+#define VT8231_REG_PWM(nr) (0x5f + (nr))
+#define VT8231_REG_PWM_CTL 0x51
+
+/* The VT8231 registers */
+/* We define the sensors as follows. Somewhat convoluted to minimize
+   changes from via686a.
+	Sensor		Voltage Mode	Temp Mode
+	--------	------------	---------
+	Reading 1			temp3
+	Reading 3			temp1	not in vt8231
+	UCH1/Reading2	in0		temp2
+	UCH2		in1		temp4
+	UCH3		in2		temp5
+	UCH4		in3		temp6
+	UCH5		in4		temp7
+	3.3V		in5
+	-12V		in6			not in vt8231
+*/
+
+/* ins numbered 0-6 */
+#define VT8231_REG_IN_MAX(nr) ((nr)=0 ? 0x3d : 0x29 + ((nr) * 2))
+#define VT8231_REG_IN_MIN(nr) ((nr)=0 ? 0x3e : 0x2a + ((nr) * 2))
+#define VT8231_REG_IN(nr)	 (0x21 + (nr))
+
+/* fans numbered 1-2 */
+#define VT8231_REG_FAN_MIN(nr) (0x3a + (nr))
+#define VT8231_REG_FAN(nr)	 (0x28 + (nr))
+
+static const u8 regtemp[] = { 0x20, 0x21, 0x1f, 0x22, 0x23, 0x24, 0x25 };
+static const u8 regover[] = { 0x39, 0x3d, 0x1d, 0x2b, 0x2d, 0x2f, 0x31 };
+static const u8 reghyst[] = { 0x3a, 0x3e, 0x1e, 0x2c, 0x2e, 0x30, 0x32 };
+
+/* temps numbered 1-7 */
+#define VT8231_REG_TEMP(nr)		(regtemp[(nr) - 1])
+#define VT8231_REG_TEMP_OVER(nr)	(regover[(nr) - 1])
+#define VT8231_REG_TEMP_HYST(nr)	(reghyst[(nr) - 1])
+#define VT8231_REG_TEMP_LOW3	0x4b	/* bits 7-6 */
+#define VT8231_REG_TEMP_LOW2	0x49	/* bits 5-4 */
+#define VT8231_REG_TEMP_LOW47	0x4d
+
+#define VT8231_REG_CONFIG 0x40
+#define VT8231_REG_ALARM1 0x41
+#define VT8231_REG_ALARM2 0x42
+#define VT8231_REG_VID	0x45
+#define VT8231_REG_FANDIV 0x47
+#define VT8231_REG_UCH_CONFIG 0x4a
+#define VT8231_REG_TEMP1_CONFIG 0x4b
+#define VT8231_REG_TEMP2_CONFIG 0x4c
+
+/* temps 1-7; voltages 0-6 */
+#define ISTEMP(i, ch_config) ((i) = 1 ? 1 : \
+		  (i) = 3 ? 1 : \
+		  (i) = 2 ? ((ch_config) >> 1) & 0x01 : \
+					 ((ch_config) >> ((i)-1)) & 0x01)
+#define ISVOLT(i, ch_config) ((i) > 4 ? 1 : !(((ch_config) >> ((i)+2)) & 0x01))
+
+#define ALARMS_FROM_REG(val) (val)
+
+#define DIV_FROM_REG(val) (1 << (val))
+#define DIV_TO_REG(val) ((val)=8?3:(val)=4?2:(val)=1?0:1)
+#define PWM_FROM_REG(val) (val)
+#define PWM_TO_REG(val) SENSORS_LIMIT((val), 0, 255)
+
+#define TEMP_FROM_REG(val) (long)((val)*10)
+#define TEMP_FROM_REG10(val) (long)(((val)*10)/4)
+#define TEMP_TO_REG(val) (SENSORS_LIMIT(((val)<0?(((val)-5)/10):\
+			 ((val)+5)/10), 0, 255))
+#define IN_FROM_REG(val) ((long)(val))
+#define IN_TO_REG(val)  (SENSORS_LIMIT((((val) * 10 + 5)/10), 0, 255))
+
+
+/********* FAN RPM CONVERSIONS ********/
+/* But this chip saturates back at 0, not at 255 like all the other chips.
+   So, 0 means 0 RPM */
+static inline u8 FAN_TO_REG(long rpm, int div)
+{
+	if (rpm = 0)
+		return 0;						
+	rpm = SENSORS_LIMIT(rpm, 1, 1000000);   
+	return SENSORS_LIMIT((1310720 + rpm * div / 2) / (rpm * div), 1, 255);
+}
+
+#define MIN_TO_REG(a, b) FAN_TO_REG(a, b)
+#define FAN_FROM_REG(val, div) ((val)=0?0:(val)=255?0:1310720/((val)*(div)))
+
+struct vt8231_data {
+	struct i2c_client client;
+	struct semaphore update_lock;
+	struct class_device *class_dev;
+	char valid;		/* !=0 if following fields are valid */
+	unsigned long last_updated;	/* In jiffies */
+
+	u8 in[7];		/* Register value */
+	u8 in_max[7];		/* Register value */
+	u8 in_min[7];		/* Register value */
+	u16 temp[7];		/* Register value 10 bit */
+	u8 temp_over[7];	/* Register value */
+	u8 temp_hyst[7];	/* Register value */
+	u8 fan[2];		/* Register value */
+	u8 fan_min[2];		/* Register value */
+	u8 fan_div[2];		/* Register encoding, shifted right */
+	u16 alarms;		/* Register encoding */
+	u8 pwm[2];		/* Register value */
+	u8 pwm_ctl;		/* Register value */
+	u8 cpu0_vid;		/* Register encoding */
+	u8 vrm;
+	u8 uch_config;
+};
+
+static struct pci_dev *s_bridge;
+static int vt8231_detect(struct i2c_adapter *adapter);
+static int vt8231_detach_client(struct i2c_client *client);
+
+static inline int vt8231_read_value(struct i2c_client *client, u8 reg)
+{
+	return (inb_p(client->addr + reg));
+}
+
+static inline void vt8231_write_value(struct i2c_client *client, u8 reg, 
+					u8 value)
+{
+	outb_p(value, client->addr + reg);
+}
+
+static struct vt8231_data *vt8231_update_device(struct device *dev);
+static void vt8231_init_client(struct i2c_client *client);
+
+/* following are the sysfs callback functions */
+static ssize_t show_in(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", IN_FROM_REG(data->in[nr]));
+}
+
+static ssize_t show_in_min(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", IN_FROM_REG(data->in_min[nr]));
+}
+
+static ssize_t show_in_max(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", IN_FROM_REG(data->in_max[nr]));
+}
+
+static ssize_t set_in_min(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+	data->in_min[nr] = IN_TO_REG(val);
+	vt8231_write_value(client, VT8231_REG_IN_MIN(nr), 
+			data->in_min[nr]);
+	return count;
+}
+static ssize_t set_in_max(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+	data->in_max[nr] = IN_TO_REG(val);
+	vt8231_write_value(client, VT8231_REG_IN_MAX(nr), 
+			data->in_max[nr]);
+	return count;
+}
+
+#define show_in_offset(offset)					\
+static ssize_t show_in##offset (struct device *dev, 		\
+	struct device_attribute *attr, char *buf)		\
+{								\
+	return show_in(dev, buf, offset);			\
+}								\
+static ssize_t show_in##offset##_min (struct device *dev,	\
+		struct device_attribute *attr, char *buf)	\
+{								\
+	return show_in_min(dev, buf, offset);			\
+}								\
+static ssize_t show_in##offset##_max (struct device *dev,	\
+		struct device_attribute *attr, char *buf)	\
+{								\
+	return show_in_max(dev, buf, offset);			\
+}								\
+static ssize_t set_in##offset##_min (struct device *dev, 	\
+		struct device_attribute *attr,	  		\
+		const char *buf, size_t count)			\
+{								\
+	return set_in_min(dev, buf, count, offset);		\
+}								\
+static ssize_t set_in##offset##_max (struct device *dev, 	\
+			struct device_attribute *attr,	 	\
+			const char *buf, size_t count)		\
+{								\
+	return set_in_max(dev, buf, count, offset);		\
+}								\
+static DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in##offset, NULL);\
+static DEVICE_ATTR(in##offset##_min, S_IRUGO | S_IWUSR,	  	\
+		show_in##offset##_min, set_in##offset##_min);	\
+static DEVICE_ATTR(in##offset##_max, S_IRUGO | S_IWUSR,	  	\
+		show_in##offset##_max, set_in##offset##_max);
+
+show_in_offset(0);
+show_in_offset(1);
+show_in_offset(2);
+show_in_offset(3);
+show_in_offset(4);
+show_in_offset(5);
+
+/* 3 temperatures */
+static ssize_t show_temp(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", TEMP_FROM_REG10(data->temp[nr]));
+}
+static ssize_t show_temp_over(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", TEMP_FROM_REG(data->temp_over[nr]));
+}
+static ssize_t show_temp_hyst(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", TEMP_FROM_REG(data->temp_hyst[nr]));
+}
+static ssize_t set_temp_over(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int val = simple_strtol(buf, NULL, 10);
+	data->temp_over[nr] = TEMP_TO_REG(val);
+	vt8231_write_value(client, VT8231_REG_TEMP_OVER(nr), 
+			data->temp_over[nr]);
+	return count;
+}
+static ssize_t set_temp_hyst(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int val = simple_strtol(buf, NULL, 10);
+	data->temp_hyst[nr] = TEMP_TO_REG(val);
+	vt8231_write_value(client, VT8231_REG_TEMP_HYST(nr), 
+			data->temp_hyst[nr]);
+	return count;
+}
+#define show_temp_offset(offset)					\
+static ssize_t show_temp_##offset (struct device *dev, 			\
+			struct device_attribute *attr, char *buf)	\
+{									\
+	return show_temp(dev, buf, offset - 1);				\
+}									\
+static ssize_t show_temp_##offset##_over (struct device *dev, 		\
+			struct device_attribute *attr, char *buf)	\
+{									\
+	return show_temp_over(dev, buf, offset - 1);			\
+}									\
+static ssize_t show_temp_##offset##_hyst (struct device *dev, 		\
+			struct device_attribute *attr, char *buf)	\
+{									\
+	return show_temp_hyst(dev, buf, offset - 1);			\
+}									\
+static ssize_t set_temp_##offset##_over (struct device *dev, 		\
+			struct device_attribute *attr,		  	\
+			const char *buf, size_t count)			\
+{									\
+	return set_temp_over(dev, buf, count, offset - 1);		\
+}									\
+static ssize_t set_temp_##offset##_hyst (struct device *dev, 		\
+			struct device_attribute *attr,		  	\
+			const char *buf, size_t count)			\
+{									\
+	return set_temp_hyst(dev, buf, count, offset - 1);		\
+}									\
+static DEVICE_ATTR(temp##offset##_input, S_IRUGO, show_temp_##offset, NULL);\
+static DEVICE_ATTR(temp##offset##_max, S_IRUGO | S_IWUSR,		\
+		show_temp_##offset##_over, set_temp_##offset##_over);	\
+static DEVICE_ATTR(temp##offset##_max_hyst, S_IRUGO | S_IWUSR,		\
+		show_temp_##offset##_hyst, set_temp_##offset##_hyst);
+
+
+/* show_temp_offset(1); */
+show_temp_offset(2);
+show_temp_offset(3);
+show_temp_offset(4);
+show_temp_offset(5);
+show_temp_offset(6);
+show_temp_offset(7);
+
+/* 2 Fans */
+static ssize_t show_fan(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[nr], 
+				DIV_FROM_REG(data->fan_div[nr])));
+}
+static ssize_t show_fan_min(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%d\n", 
+		FAN_FROM_REG(data->fan_min[nr], 
+			DIV_FROM_REG(data->fan_div[nr])));
+}
+static ssize_t show_fan_div(struct device *dev, char *buf, int nr) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%d\n", DIV_FROM_REG(data->fan_div[nr]));
+}
+static ssize_t set_fan_min(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int val = simple_strtol(buf, NULL, 10);
+	data->fan_min[nr] = FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr]));
+	vt8231_write_value(client, VT8231_REG_FAN_MIN(nr+1), data->fan_min[nr]);
+	return count;
+}
+static ssize_t set_fan_div(struct device *dev, const char *buf, 
+		size_t count, int nr) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int val = simple_strtol(buf, NULL, 10);
+	int old = vt8231_read_value(client, VT8231_REG_FANDIV);
+	data->fan_div[nr] = DIV_TO_REG(val);
+	old = (old & 0x0f) | (data->fan_div[1] << 6) | (data->fan_div[0] << 4);
+	vt8231_write_value(client, VT8231_REG_FANDIV, old);
+	return count;
+}
+
+#define show_fan_offset(offset)						\
+static ssize_t show_fan_##offset (struct device *dev, 			\
+				struct device_attribute *attr, char *buf) \
+{									\
+	return show_fan(dev, buf, offset - 1);				\
+}									\
+static ssize_t show_fan_##offset##_min (struct device *dev, 		\
+				struct device_attribute *attr, char *buf) \
+{									\
+	return show_fan_min(dev, buf, offset - 1);			\
+}									\
+static ssize_t show_fan_##offset##_div (struct device *dev, 		\
+				struct device_attribute *attr, char *buf) \
+{									\
+	return show_fan_div(dev, buf, offset - 1);			\
+}									\
+static ssize_t set_fan_##offset##_min (struct device *dev, 		\
+				struct device_attribute *attr,		\
+				const char *buf, size_t count)		\
+{									\
+	return set_fan_min(dev, buf, count, offset - 1);		\
+}									\
+static ssize_t set_fan_##offset##_div (struct device *dev, 		\
+				struct device_attribute *attr,		\
+				const char *buf, size_t count)		\
+{									\
+	return set_fan_div(dev, buf, count, offset - 1);		\
+}									\
+static DEVICE_ATTR(fan##offset##_input, S_IRUGO, show_fan_##offset, NULL);\
+static DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR,		\
+		show_fan_##offset##_min, set_fan_##offset##_min);	\
+static DEVICE_ATTR(fan##offset##_div, S_IRUGO | S_IWUSR,		\
+		show_fan_##offset##_div, set_fan_##offset##_div);
+
+show_fan_offset(1);
+show_fan_offset(2);
+
+/* Alarms */
+static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, 
+			char *buf) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%d\n", ALARMS_FROM_REG(data->alarms));
+}
+static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+
+
+/**** VRM and VID *****/
+static ssize_t show_vid(struct device *dev, struct device_attribute *attr, 
+			char *buf) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)vid_from_reg(data->cpu0_vid, data->vrm)); 
+}
+static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid, NULL);
+
+static ssize_t show_vrm(struct device *dev, struct device_attribute *attr, 
+			char *buf) {
+	struct vt8231_data *data = vt8231_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)data->vrm);
+}
+static ssize_t set_vrm(struct device *dev, struct device_attribute *attr, 
+			const char *buf, size_t count) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int val = simple_strtol(buf, NULL, 10);
+	printk("Setting VRM to \"%s\" = %d\n", buf, val);
+	data->vrm = val;
+	return count;
+}
+static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm, set_vrm);
+
+static struct i2c_driver vt8231_driver = {
+	.owner		= THIS_MODULE, 
+	.name		= "vt8231", 
+	.id		= I2C_DRIVERID_VT8231, 
+	.flags		= I2C_DF_NOTIFY, 
+	.attach_adapter	= vt8231_detect, 
+	.detach_client	= vt8231_detach_client, 
+};
+
+int vt8231_detect(struct i2c_adapter *adapter)
+{
+	struct i2c_client *new_client;
+	struct vt8231_data *data;
+	int err = 0;
+	const char type_name[] = "vt8231";
+	u16 val;
+
+	/* 8231 requires multiple of 256 */
+	if (force_addr)
+		isa_address = force_addr & 0xFF00;
+
+	if (force_addr) {
+		dev_warn(&adapter->dev, "forcing ISA address 0x%04X\n", 
+				 isa_address);
+		if (PCIBIOS_SUCCESSFUL != pci_write_config_word(s_bridge, 
+						VT8231_BASE_REG, isa_address))
+			return -ENODEV;
+	}
+	if (PCIBIOS_SUCCESSFUL !+		pci_read_config_word(s_bridge, VT8231_ENABLE_REG, &val))
+		return -ENODEV;
+	if (!(val & 0x0001)) {
+		dev_warn(&adapter->dev, "enabling sensors\n");
+		if (PCIBIOS_SUCCESSFUL !+			pci_write_config_word(s_bridge, VT8231_ENABLE_REG, 
+							  val | 0x0001))
+			return -ENODEV;
+	}
+
+	/* Reserve the ISA region */
+	if (!request_region(isa_address, VT8231_EXTENT, type_name)) {
+		dev_err(&adapter->dev, "region 0x%x already in use!\n", 
+			   isa_address);
+		return -ENODEV;
+	}
+
+	if (!(data = kzalloc(sizeof(struct vt8231_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit_release;
+	}
+
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = isa_address;
+	new_client->adapter = adapter;
+	new_client->driver = &vt8231_driver;
+	new_client->flags = 0;
+	new_client->dev.parent = &adapter->dev;
+
+	/* Fill in the remaining client fields and put into the global list */
+	strlcpy(new_client->name, type_name, I2C_NAME_SIZE);
+
+	data->valid = 0;
+	init_MUTEX(&data->update_lock);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	vt8231_init_client(new_client);
+
+	/* Register sysfs hooks */
+	data->class_dev = hwmon_device_register(&new_client->dev);
+	if (IS_ERR(data->class_dev)) {
+		err = PTR_ERR(data->class_dev);
+		goto exit_detach;
+	}
+
+	device_create_file(&new_client->dev, &dev_attr_in0_input);
+	device_create_file(&new_client->dev, &dev_attr_in1_input);
+	device_create_file(&new_client->dev, &dev_attr_in2_input);
+	device_create_file(&new_client->dev, &dev_attr_in3_input);
+	device_create_file(&new_client->dev, &dev_attr_in4_input);
+	device_create_file(&new_client->dev, &dev_attr_in5_input);
+	device_create_file(&new_client->dev, &dev_attr_in0_min);
+	device_create_file(&new_client->dev, &dev_attr_in1_min);
+	device_create_file(&new_client->dev, &dev_attr_in2_min);
+	device_create_file(&new_client->dev, &dev_attr_in3_min);
+	device_create_file(&new_client->dev, &dev_attr_in4_min);
+	device_create_file(&new_client->dev, &dev_attr_in5_min);
+	device_create_file(&new_client->dev, &dev_attr_in0_max);
+	device_create_file(&new_client->dev, &dev_attr_in1_max);
+	device_create_file(&new_client->dev, &dev_attr_in2_max);
+	device_create_file(&new_client->dev, &dev_attr_in3_max);
+	device_create_file(&new_client->dev, &dev_attr_in4_max);
+	device_create_file(&new_client->dev, &dev_attr_in5_max);
+
+	device_create_file(&new_client->dev, &dev_attr_temp2_input);
+	device_create_file(&new_client->dev, &dev_attr_temp3_input);
+	device_create_file(&new_client->dev, &dev_attr_temp4_input);
+	device_create_file(&new_client->dev, &dev_attr_temp5_input);
+	device_create_file(&new_client->dev, &dev_attr_temp6_input);
+	device_create_file(&new_client->dev, &dev_attr_temp7_input);
+	device_create_file(&new_client->dev, &dev_attr_temp2_max);
+	device_create_file(&new_client->dev, &dev_attr_temp3_max);
+	device_create_file(&new_client->dev, &dev_attr_temp4_max);
+	device_create_file(&new_client->dev, &dev_attr_temp5_max);
+	device_create_file(&new_client->dev, &dev_attr_temp6_max);
+	device_create_file(&new_client->dev, &dev_attr_temp7_max);
+	device_create_file(&new_client->dev, &dev_attr_temp2_max_hyst);
+	device_create_file(&new_client->dev, &dev_attr_temp3_max_hyst);
+	device_create_file(&new_client->dev, &dev_attr_temp4_max_hyst);
+	device_create_file(&new_client->dev, &dev_attr_temp5_max_hyst);
+	device_create_file(&new_client->dev, &dev_attr_temp6_max_hyst);
+	device_create_file(&new_client->dev, &dev_attr_temp7_max_hyst);
+
+	device_create_file(&new_client->dev, &dev_attr_fan1_input);
+	device_create_file(&new_client->dev, &dev_attr_fan2_input);
+	device_create_file(&new_client->dev, &dev_attr_fan1_min);
+	device_create_file(&new_client->dev, &dev_attr_fan2_min);
+	device_create_file(&new_client->dev, &dev_attr_fan1_div);
+	device_create_file(&new_client->dev, &dev_attr_fan2_div);
+
+	device_create_file(&new_client->dev, &dev_attr_alarms);
+
+	device_create_file(&new_client->dev, &dev_attr_cpu0_vid);
+	device_create_file(&new_client->dev, &dev_attr_vrm);
+
+	return 0;
+
+exit_detach:
+	i2c_detach_client(new_client);
+exit_free:
+	kfree(data);
+exit_release:
+	release_region(isa_address, VT8231_EXTENT);
+	return err;
+}
+
+static int vt8231_detach_client(struct i2c_client *client)
+{
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int err;
+
+	hwmon_device_unregister(data->class_dev);
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, 
+		   "Client deregistration failed, client not detached.\n");
+		return err;
+	}
+
+	release_region(client->addr, VT8231_EXTENT);
+	kfree(i2c_get_clientdata(client));
+
+	return 0;
+}
+
+static void vt8231_init_client(struct i2c_client *client)
+{
+	struct vt8231_data *data = i2c_get_clientdata(client);
+
+	data->vrm = vid_which_vrm();
+
+	/* set "default" interrupt mode for alarms, which isn't the default */
+	vt8231_write_value(client, VT8231_REG_TEMP1_CONFIG, 0);
+	vt8231_write_value(client, VT8231_REG_TEMP2_CONFIG, 0);
+}
+
+static struct vt8231_data *vt8231_update_device(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct vt8231_data *data = i2c_get_clientdata(client);
+	int i, j;
+
+	down(&data->update_lock);
+
+	if ((jiffies - data->last_updated > HZ + HZ / 2) ||
+		(jiffies < data->last_updated) || !data->valid) {
+		data->uch_config = vt8231_read_value(client, 
+						VT8231_REG_UCH_CONFIG);
+		for (i = 0; i <= 5; i++) {
+			if(ISVOLT(i, data->uch_config)) {
+				data->in[i] = vt8231_read_value(client, 
+							VT8231_REG_IN(i));
+				data->in_min[i] = vt8231_read_value(client, 
+							VT8231_REG_IN_MIN(i));
+				data->in_max[i] = vt8231_read_value(client, 
+							VT8231_REG_IN_MAX(i));
+			} else {
+				data->in[i] = 0;
+				data->in_min[i] = 0;
+				data->in_max[i] = 0;
+			}
+		}
+		for (i = 1; i <= 2; i++) {
+			data->fan[i - 1] = vt8231_read_value(client, 
+							VT8231_REG_FAN(i));
+			data->fan_min[i - 1] = vt8231_read_value(client, 
+							VT8231_REG_FAN_MIN(i));
+		}
+		for (i = 2; i <= 7; i++) {
+			if(ISTEMP(i, data->uch_config)) {
+				data->temp[i - 1] = vt8231_read_value(client, 
+						       VT8231_REG_TEMP(i)) << 2;
+				switch(i) {
+					case 1:
+						/* ? */
+						j = 0;
+						break;
+					case 2:
+						j = (vt8231_read_value(client, 
+							VT8231_REG_TEMP_LOW2) &
+							0x30) >> 4;
+						break;
+					case 3:
+						j = (vt8231_read_value(client, 
+							VT8231_REG_TEMP_LOW3) &
+							0xc0) >> 6;
+						break;
+					case 4:
+					case 5:
+					case 6:
+					case 7:
+					default:
+						j = (vt8231_read_value(client, 
+						  	VT8231_REG_TEMP_LOW47)
+							 >> ((i-4)*2)) & 0x03;	
+						break;
+	
+				}
+				data->temp[i - 1] |= j;
+				data->temp_over[i - 1] = 
+					vt8231_read_value(client, 
+						VT8231_REG_TEMP_OVER(i));
+				data->temp_hyst[i - 1] = 
+					vt8231_read_value(client, 
+						VT8231_REG_TEMP_HYST(i));
+			} else {
+				data->temp[i - 1] = 0;
+				data->temp_over[i - 1] = 0;
+				data->temp_hyst[i - 1] = 0;
+			}
+		}
+
+		for (i = 1; i <= 2; i++) {
+			data->fan[i - 1] = vt8231_read_value(client, 
+						VT8231_REG_FAN(i));
+			data->fan_min[i - 1] = vt8231_read_value(client, 
+						VT8231_REG_FAN_MIN(i));
+			data->pwm[i - 1] = vt8231_read_value(client, 
+						VT8231_REG_PWM(i));
+		}
+
+		data->pwm_ctl = vt8231_read_value(client, VT8231_REG_PWM_CTL);
+		i = vt8231_read_value(client, VT8231_REG_FANDIV);
+		data->fan_div[0] = (i >> 4) & 0x03;
+		data->fan_div[1] = i >> 6;
+		data->alarms = vt8231_read_value(client, VT8231_REG_ALARM1) |
+			(vt8231_read_value(client, VT8231_REG_ALARM2) << 8);
+		data->cpu0_vid= vt8231_read_value(client, VT8231_REG_VID) 
+					& 0x1f;
+		data->last_updated = jiffies;
+		data->valid = 1;
+	}
+
+	up(&data->update_lock);
+
+	return data;
+}
+
+static struct pci_device_id vt8231_pci_ids[] = {
+	{
+		.vendor		 = PCI_VENDOR_ID_VIA, 
+		.device		 = PCI_DEVICE_ID_VIA_8231_4, 
+		.subvendor	= PCI_ANY_ID, 
+		.subdevice	= PCI_ANY_ID, 
+	}, 
+	{ 0, }
+};
+
+static int __devinit vt8231_pci_probe(struct pci_dev *dev, 
+				const struct pci_device_id *id)
+{
+	u16 val;
+	
+	if (PCIBIOS_SUCCESSFUL != pci_read_config_word(dev, VT8231_BASE_REG, 
+							&val))
+		return -ENODEV;
+	
+	isa_address = val & ~(VT8231_EXTENT - 1);
+	if (isa_address = 0 && force_addr = 0) {
+		dev_err(&dev->dev, "base address not set -\
+				 upgrade BIOS or use force_addr=0xaddr\n");
+		return -ENODEV;
+	}
+	if (force_addr)
+		isa_address = force_addr; /* so detect will get called */
+	
+	if (!isa_address) {
+		dev_err(&dev->dev, "No Via 8231 sensors found.\n");
+		return -ENODEV;
+	}
+	s_bridge = pci_dev_get(dev);
+	
+	if (i2c_isa_add_driver(&vt8231_driver)) {
+		pci_dev_put(s_bridge);
+		s_bridge = NULL;
+	}
+	
+	/* Always return failure here.  This is to allow other drivers to bind
+	* to this pci device.  We don't really want to have control over the
+	* pci device, we only wanted to read as few register values from it.
+	*/
+	return -ENODEV;
+}
+
+static struct pci_driver vt8231_pci_driver = {
+	.name		= "vt8231", 
+	.id_table	= vt8231_pci_ids, 
+	.probe		= vt8231_pci_probe, 
+};
+
+
+static int __init sm_vt8231_init(void)
+{
+	return pci_module_init(&vt8231_pci_driver);
+}
+
+static void __exit sm_vt8231_exit(void)
+{
+	pci_unregister_driver(&vt8231_pci_driver);
+	if (s_bridge != NULL) {
+		i2c_isa_del_driver(&vt8231_driver);
+		pci_dev_put(s_bridge);
+		s_bridge = NULL;
+	}
+}
+
+
+MODULE_AUTHOR("Mark D. Studebaker <mdsxyz123@yahoo.com>");
+MODULE_DESCRIPTION("VT8231 sensors");
+MODULE_LICENSE("GPL");
+
+module_init(sm_vt8231_init);
+module_exit(sm_vt8231_exit);

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

>>+**     in the 2.6.14 kernel source to create this code.
>>+**     VRM doesn't work on the Via Epia 5000 that I have as the kernel
> 
> call
> 
>>+**	   vid_which_vrm() returns -1.  Apparently the Via CPU is
> 
> unrecognised....
> 
> The vid_which_vrm issue is interesting. Rudolf, can you please look
> into it?
> 

Yes,

This is not yet in our detection table. Here is what we know:

TYPE  FAMILY  MODEL  STEPPING
0       6       7       0+   something special EDEN ESP samuel 2
0       6       7       8+   VRM85 C3 EZRA
0       6       8       0-7  VRM85 C3 EZRA T
0       6       9       0+   VRM85 NEMIAH
0       6       9       8+  max1718 C3-M

max1718 => Notebook CPU Step-Down Controller for Intel
 Mobile Voltage Positioning (IMVP-II)

http://www.egevo.se/dokument/max1718.pdf
http://www.maxim-ic.com/products/power_supplies/imvp.cfm

VID "something special"
1 1 1 1  1.30V
1 1 1 0  1.35V
0 0 0 0  2.05V


We need to define 2 new VIDs one is something called

IMVP-II (I dont know anything about -I and -III  => will call it 1718

and the that really strange samuel 2 VID codding. => will call it 670

Ideas? Questions?
If not I will develop the patch.

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Rudolf,

I'm not really familiar with this, so I'll trust it to you if you don't
mind. :-)

If it helps any, the output from LSHW for the CPU section is below:
     *-cpu
          description: CPU
          product: VIA Samuel 2
          vendor: CentaurHauls
          physical id: 4
          bus info: cpu@0
          version: VIA C3
          slot: Socket 370
          size: 533MHz
          capacity: 1GHz
          width: 32 bits
          clock: 133MHz
          capabilities: fpu fpu_exception wp de tsc msr cx8 mtrr pge mmx
3dnow
        *-cache:0
             description: L1 cache
             physical id: 8
             slot: Internal Cache
             size: 32KB
             capacity: 32KB
             capabilities: synchronous internal write-back
        *-cache:1
             description: L1 cache
             physical id: 9
             slot: Internal Cache
             size: 64KB
             capacity: 64KB
             capabilities: synchronous internal write-back


- Roger

-----Original Message-----
From: Rudolf Marek [mailto:r.marek@sh.cvut.cz] 
Sent: 01 November 2005 22:57
To: Roger Lucas
Cc: 'LM Sensors'; Jean Delvare
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

>>+**     in the 2.6.14 kernel source to create this code.
>>+**     VRM doesn't work on the Via Epia 5000 that I have as the kernel
> 
> call
> 
>>+**	   vid_which_vrm() returns -1.  Apparently the Via CPU is
> 
> unrecognised....
> 
> The vid_which_vrm issue is interesting. Rudolf, can you please look
> into it?
> 

Yes,

This is not yet in our detection table. Here is what we know:

TYPE  FAMILY  MODEL  STEPPING
0       6       7       0+   something special EDEN ESP samuel 2
0       6       7       8+   VRM85 C3 EZRA
0       6       8       0-7  VRM85 C3 EZRA T
0       6       9       0+   VRM85 NEMIAH
0       6       9       8+  max1718 C3-M

max1718 => Notebook CPU Step-Down Controller for Intel
 Mobile Voltage Positioning (IMVP-II)

http://www.egevo.se/dokument/max1718.pdf
http://www.maxim-ic.com/products/power_supplies/imvp.cfm

VID "something special"
1 1 1 1  1.30V
1 1 1 0  1.35V
0 0 0 0  2.05V


We need to define 2 new VIDs one is something called

IMVP-II (I dont know anything about -I and -III  => will call it 1718

and the that really strange samuel 2 VID codding. => will call it 670

Ideas? Questions?
If not I will develop the patch.

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

Roger Lucas wrote:
> Rudolf,
> 
> I'm not really familiar with this, so I'll trust it to you if you don't
> mind. :-)
> 
> If it helps any, the output from LSHW for the CPU section is below:
>      *-cpu
>           description: CPU
>           product: VIA Samuel 2

Can you confirm that this cpu has that strange 4bit VID?

> VID "something special"
> 1 1 1 1  1.30V
> 1 1 1 0  1.35V
> 0 0 0 0  2.05V

Thanks,
Regards

Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

> Can you confirm that this cpu has that strange 4bit VID?

Hmm if you know it has 1.2V so my table is not correct.
PLease can you give me output of CPUID utillity?

(if you have debian just apt-get install cpuid)

I need family/model/stepping

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Hi Rudolf,

I have done some digging on my motherboard (a Via Epia 5000).

http://www.viaembedded.com/product/epia_mini_itx_spec.jsp?motherboardId!

The processor is a Via EDEN ESP 5000.  This has a 533 MHz clock and 1.2v
supply.  You can find the datasheet for the chip on the link below (see
section 1.3 on page 7 for the speed/voltage model information).

http://www.viaembedded.com/product/6/14/Eden20v115.pdf

Fortunately, I am running Debian, so the "apt-get install cupid" worked
smoothly and returned the results below.  I have absolutely no idea what
they mean :-)

edensnake:~# cpuid
 eax in    eax      ebx      ecx      edx
00000000 00000001 746e6543 736c7561 48727561
00000001 00000673 00000000 00000000 00803135
80000000 80000006 00000000 00000000 00000000
80000001 00000673 00000000 00000000 80803135
80000002 20414956 756d6153 32206c65 00000000
80000003 00000000 00000000 00000000 00000000
80000004 00000000 00000000 00000000 00000000
80000005 00000000 08800880 40040120 40040120
80000006 00000000 00000000 40040120 00000000

Vendor ID: "CentaurHauls"; CPUID level 1

Unknown vendor
edensnake:~#

Thanks for you help, and let me know if you need anything else.

- Roger


-----Original Message-----
From: Rudolf Marek [mailto:r.marek@sh.cvut.cz] 
Sent: 02 November 2005 09:25
To: Roger Lucas
Cc: 'LM Sensors'
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

> Can you confirm that this cpu has that strange 4bit VID?

Hmm if you know it has 1.2V so my table is not correct.
PLease can you give me output of CPUID utillity?

(if you have debian just apt-get install cpuid)

I need family/model/stepping

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Rudolf,

On 2005-11-01, Rudolf Marek wrote:
> VID "something special"
> 1 1 1 1  1.30V
> 1 1 1 0  1.35V
> 0 0 0 0  2.05V

This looks like standard VRM 8.4 to me, nothing special.

> We need to define 2 new VIDs one is something called
>
> IMVP-II (I dont know anything about -I and -III  => will call it 1718
>
> and the that really strange samuel 2 VID codding. => will call it 670
>
> Ideas? Questions?
> If not I will develop the patch.

Sure, add new VRM-like values if needed. We might end up defining
constant for these so that the number do not look like magic.

While you're at it, I think we should do some changes to vid_from_reg: I
think VRM 8.2 was the default for historical reasons, but it's not
safe. We shouldn't have no default. 8.2 should be handled explicitely
as other values are. Any non-zero, non-handled value should trigger a
debug log message. Any non-handled value should result in 0 being
returned. OK?

Thanks,
--
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

Roger Lucas wrote:
> Hi Rudolf,
> 
> I have done some digging on my motherboard (a Via Epia 5000).
> 
> http://www.viaembedded.com/product/epia_mini_itx_spec.jsp?motherboardId!
> 
> The processor is a Via EDEN ESP 5000.  This has a 533 MHz clock and 1.2v
> supply.  You can find the datasheet for the chip on the link below (see
> section 1.3 on page 7 for the speed/voltage model information).
> 
> http://www.viaembedded.com/product/6/14/Eden20v115.pdf

Yes I have read that yesterday.

> Fortunately, I am running Debian, so the "apt-get install cupid" worked
> smoothly and returned the results below.  I have absolutely no idea what
> they mean :-)
> 
> edensnake:~# cpuid
>  eax in    eax      ebx      ecx      edx
> 00000000 00000001 746e6543 736c7561 48727561
> 00000001 00000673 00000000 00000000 00803135

Family is 6 model 7 stepping 3


0       6       7       0+   something special EDEN ESP samuel 2

This could be also this processor which seems to follow VRM 8.4 instead of VRM8.5. Dig around the datsheets I have and it seems some contradict
the other. I think correct would be VRM 8.5 for this case.

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

Jean Delvare wrote:
> Hi Rudolf,
> 
> On 2005-11-01, Rudolf Marek wrote:
> 
>>VID "something special"
>>1 1 1 1  1.30V
>>1 1 1 0  1.35V
>>0 0 0 0  2.05V
> 
> 
> This looks like standard VRM 8.4 to me, nothing special.

OK, it seems it is not used. Newer datasheets are not talking about this.

> Sure, add new VRM-like values if needed. We might end up defining
> constant for these so that the number do not look like magic.

Yes it would be better.

> 
> While you're at it, I think we should do some changes to vid_from_reg: I
> think VRM 8.2 was the default for historical reasons, but it's not
> safe. We shouldn't have no default. 8.2 should be handled explicitely
> as other values are. Any non-zero, non-handled value should trigger a
> debug log message. Any non-handled value should result in 0 being
> returned. OK?

Sounds good.
Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

Hello all,

Here is the first shot. I wrote it in the evening after two beers so dont expect to be bug free ;).
On the other hand comments wellcomed. (yes I know it can be written using ternary operator)

Roger, can you give a try if it detects your CPU and you will get back 1.2V?

TYPE  FAMILY  MODEL  STEPPING
0       6       7       0+   this should be 85? -  EDEN ESP samuel 2
0       6       7       8+   VRM85 C3 EZRA
0       6       8       0-7  VRM85 C3 EZRA T
0       6       9       0+   VRM85 NEMIAH
0       6       9       8+  max 1718 C3-M

Thanks,

Regards
Rudolf
-------------- next part --------------
diff -Naur a/drivers/hwmon/hwmon-vid.c b/drivers/hwmon/hwmon-vid.c
--- a/drivers/hwmon/hwmon-vid.c	2005-10-28 02:02:08.000000000 +0200
+++ b/drivers/hwmon/hwmon-vid.c	2005-11-02 22:59:19.577711744 +0100
@@ -60,9 +60,6 @@
 
 	switch(vrm) {
 
-	case  0:
-		return 0;
-
 	case 100:               /* VRD 10.0 */
 		if((val & 0x1f) = 0x1f)
 			return 0;
@@ -91,10 +88,16 @@
 	case 84:		/* VRM 8.4 */
 		val &= 0x0f;
 				/* fall through */
-	default:		/* VRM 8.2 */
+	case 82:		/* VRM 8.2 */
 		return(val = 0x1f ? 0 :
 		       val & 0x10  ? 5100 - (val) * 100 :
-		                     2050 - (val) * 50);
+			2050 - (val) * 50);
+	case 1718:		/* Intel IMVP-II */
+		if (val & 0x10) 
+			return 0975 - ((val & 0xF) * 25);
+		return 1750 - (val * 50);
+	default:		/* report 0 for unknown */
+		return 0;
 	}
 }
 
@@ -108,6 +111,7 @@
 	u8 vendor;
 	u8 eff_family;
 	u8 eff_model;
+	u8 eff_stepping;
 	int vrm_type;
 };
 
@@ -116,18 +120,22 @@
 #ifdef CONFIG_X86
 
 static struct vrm_model vrm_models[] = {
-	{X86_VENDOR_AMD, 0x6, ANY, 90},		/* Athlon Duron etc */
-	{X86_VENDOR_AMD, 0xF, ANY, 24},		/* Athlon 64, Opteron */
-	{X86_VENDOR_INTEL, 0x6, 0x9, 85},	/* 0.13um too */
-	{X86_VENDOR_INTEL, 0x6, 0xB, 85},	/* Tualatin */
-	{X86_VENDOR_INTEL, 0x6, ANY, 82},	/* any P6 */
-	{X86_VENDOR_INTEL, 0x7, ANY, 0},	/* Itanium */
-	{X86_VENDOR_INTEL, 0xF, 0x0, 90},	/* P4 */
-	{X86_VENDOR_INTEL, 0xF, 0x1, 90},	/* P4 Willamette */
-	{X86_VENDOR_INTEL, 0xF, 0x2, 90},	/* P4 Northwood */
-	{X86_VENDOR_INTEL, 0xF, 0x3, 100},	/* P4 Prescott */
-	{X86_VENDOR_INTEL, 0xF, 0x4, 100},	/* P4 Prescott */
-	{X86_VENDOR_INTEL, 0x10,ANY, 0},	/* Itanium 2 */
+	{X86_VENDOR_AMD, 0x6, ANY, ANY, 90},	/* Athlon Duron etc */
+	{X86_VENDOR_AMD, 0xF, ANY, ANY, 24},	/* Athlon 64, Opteron */
+	{X86_VENDOR_INTEL, 0x6, 0x9, ANY, 85},	/* 0.13um too */
+	{X86_VENDOR_INTEL, 0x6, 0xB, ANY, 85},	/* Tualatin */
+	{X86_VENDOR_INTEL, 0x6, ANY, ANY, 82},	/* any P6 */
+	{X86_VENDOR_INTEL, 0x7, ANY, ANY, 0},	/* Itanium */
+	{X86_VENDOR_INTEL, 0xF, 0x0, ANY, 90},	/* P4 */
+	{X86_VENDOR_INTEL, 0xF, 0x1, ANY, 90},	/* P4 Willamette */
+	{X86_VENDOR_INTEL, 0xF, 0x2, ANY, 90},	/* P4 Northwood */
+	{X86_VENDOR_INTEL, 0xF, 0x3, ANY, 100},	/* P4 Prescott */
+	{X86_VENDOR_INTEL, 0xF, 0x4, ANY, 100},	/* P4 Prescott */
+	{X86_VENDOR_INTEL, 0x10, ANY, ANY, 0},	/* Itanium 2 */
+	{X86_VENDOR_CENTAUR, 0x6, 0x7, ANY, 85},/* Eden ESP/Ezra */
+	{X86_VENDOR_CENTAUR, 0x6, 0x8, 0x7, 85},/* Ezra T */
+	{X86_VENDOR_CENTAUR, 0x6, 0x9, 0x7, 85},/* Nemiah */
+	{X86_VENDOR_CENTAUR, 0x6, 0x9, ANY, 1718},/* C3-M */
 	{X86_VENDOR_UNKNOWN, ANY, ANY, 0}	/* stop here */
 };
 
@@ -139,7 +147,8 @@
 		if (vrm_models[i].vendor=vendor)
 			if ((vrm_models[i].eff_family=eff_family)
 			 && ((vrm_models[i].eff_model=eff_model) ||
-			     (vrm_models[i].eff_model=ANY)))
+			     (vrm_models[i].eff_model=ANY)) &&
+			     (eff_stepping =< vrm_models[i].eff_stepping))
 				return vrm_models[i].vrm_type;
 		i++;
 	}
@@ -151,7 +160,7 @@
 {
 	struct cpuinfo_x86 *c = cpu_data;
 	u32 eax;
-	u8 eff_family, eff_model;
+	u8 eff_family, eff_model, eff_stepping;
 	int vrm_ret;
 
 	if (c->x86 < 6)		/* Any CPU with family lower than 6 */
@@ -160,6 +169,7 @@
 	eax = cpuid_eax(1);
 	eff_family = ((eax & 0x00000F00)>>8);
 	eff_model  = ((eax & 0x000000F0)>>4);
+	eff_stepping = eax & 0xF;
 	if (eff_family = 0xF) {	/* use extended model & family */
 		eff_family += ((eax & 0x00F00000)>>20);
 		eff_model += ((eax & 0x000F0000)>>16)<<4;

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Hi Rudolf,

Unfortunately, by this time of the evening I am in a similar state to you,
so I'll try this along when I do the changes from Jean's code review
(hopefully tomorrow).

- Roger

-----Original Message-----
From: Rudolf Marek [mailto:r.marek@sh.cvut.cz] 
Sent: 02 November 2005 22:09
To: lm-sensors@lm-sensors.org
Cc: Jean Delvare; roger@planbit.co.uk
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

Hello all,

Here is the first shot. I wrote it in the evening after two beers so dont
expect to be bug free ;).
On the other hand comments wellcomed. (yes I know it can be written using
ternary operator)

Roger, can you give a try if it detects your CPU and you will get back 1.2V?

TYPE  FAMILY  MODEL  STEPPING
0       6       7       0+   this should be 85? -  EDEN ESP samuel 2
0       6       7       8+   VRM85 C3 EZRA
0       6       8       0-7  VRM85 C3 EZRA T
0       6       9       0+   VRM85 NEMIAH
0       6       9       8+  max 1718 C3-M

Thanks,

Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Rudolf,

On 2005-11-02, Rudolf Marek wrote:
> TYPE  FAMILY  MODEL  STEPPING
> 0       6       7       0+   this should be 85? -  EDEN ESP samuel 2
> 0       6       7       8+   VRM85 C3 EZRA
> 0       6       8       0-7  VRM85 C3 EZRA T
> 0       6       9       0+   VRM85 NEMIAH
> 0       6       9       8+  max 1718 C3-M

This table doesn't make much sense to me. Having 0+ and 8+ for the same
model is a bit weird. Isn't if rather 0-7 and 8+?

Comments on the code:

> -		                     2050 - (val) * 50);
> +			2050 - (val) * 50);

Broken whitespace change.

> +	case 1718:		/* Intel IMVP-II */

If a specification is available, please point to it in the big comment on
top of the file. If not, explain the table like was done for the Opteron
in that same comment.

> +		if (val & 0x10)
> +			return 0975 - ((val & 0xF) * 25);

That "0975" will be taken as an invalid octal value by the compiler.

> +	default:		/* report 0 for unknown */
> +		return 0;

Debug message if vrm != 0?

>  	{X86_VENDOR_UNKNOWN, ANY, ANY, 0}	/* stop here */

This one misses one "ANY" now.

> +			     (vrm_models[i].eff_model=ANY)) &&
> +			     (eff_stepping =< vrm_models[i].eff_stepping))

=< should be <=.

Also, you forgot to pass eff_stepping as a parameter so it's undefined
here.

It also seems that you are not handling the case
vrm_models[i].eff_stepping = ANY. I understand that ANY happens to be a
large value so it will be silently handled by the comparison test, but
this is tricky programming. The value of ANY could change in the future.

It would be great if you could at least test that your patches compile
properly before sending them.

Thanks,
--
Jean Delvare

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Rudolf Marek]

Roger Lucas wrote:
> Hi Rudolf,
> 
> Unfortunately, by this time of the evening I am in a similar state to you,
> so I'll try this along when I do the changes from Jean's code review
> (hopefully tomorrow).

Hello,

Khali thanks for the comments. Here comes much better version that should work.

See attachment. Please test it.

Thanks,
Regards
Rudolf
-------------- next part --------------
diff -Naur a/drivers/hwmon/hwmon-vid.c b/drivers/hwmon/hwmon-vid.c
--- a/drivers/hwmon/hwmon-vid.c	2005-10-28 02:02:08.000000000 +0200
+++ b/drivers/hwmon/hwmon-vid.c	2005-11-05 19:35:58.306905250 +0100
@@ -49,6 +49,11 @@
         . . . .
        11110  =  0.800 V
        11111  =  0.000 V (off)
+
+    The 1718 specification is in fact Intel Mobile Voltage Positioning -
+    (IMVP-II). You can find more information in the datasheet of Max1718
+    http://www.maxim-ic.com/quick_view2.cfm/qv_pk/2452
+
 */
 
 /* vrm is the VRM/VRD document version multiplied by 10.
@@ -60,9 +65,6 @@
 
 	switch(vrm) {
 
-	case  0:
-		return 0;
-
 	case 100:               /* VRD 10.0 */
 		if((val & 0x1f) = 0x1f)
 			return 0;
@@ -91,10 +93,16 @@
 	case 84:		/* VRM 8.4 */
 		val &= 0x0f;
 				/* fall through */
-	default:		/* VRM 8.2 */
+	case 82:		/* VRM 8.2 */
 		return(val = 0x1f ? 0 :
 		       val & 0x10  ? 5100 - (val) * 100 :
 		                     2050 - (val) * 50);
+	case 1718:		/* Intel IMVP-II */
+		return(val & 0x10 ? 975 - (val & 0xF) * 25 :
+				    1750 - val * 50);
+	default:		/* report 0 for unknown */
+		printk(KERN_INFO "hwmon-vid: requested unknown VRM version\n");
+		return 0;
 	}
 }
 
@@ -108,6 +116,7 @@
 	u8 vendor;
 	u8 eff_family;
 	u8 eff_model;
+	u8 eff_stepping;
 	int vrm_type;
 };
 
@@ -115,23 +124,28 @@
 
 #ifdef CONFIG_X86
 
+/* the stepping parameter is highest acceptable stepping for current line */
+
 static struct vrm_model vrm_models[] = {
-	{X86_VENDOR_AMD, 0x6, ANY, 90},		/* Athlon Duron etc */
-	{X86_VENDOR_AMD, 0xF, ANY, 24},		/* Athlon 64, Opteron */
-	{X86_VENDOR_INTEL, 0x6, 0x9, 85},	/* 0.13um too */
-	{X86_VENDOR_INTEL, 0x6, 0xB, 85},	/* Tualatin */
-	{X86_VENDOR_INTEL, 0x6, ANY, 82},	/* any P6 */
-	{X86_VENDOR_INTEL, 0x7, ANY, 0},	/* Itanium */
-	{X86_VENDOR_INTEL, 0xF, 0x0, 90},	/* P4 */
-	{X86_VENDOR_INTEL, 0xF, 0x1, 90},	/* P4 Willamette */
-	{X86_VENDOR_INTEL, 0xF, 0x2, 90},	/* P4 Northwood */
-	{X86_VENDOR_INTEL, 0xF, 0x3, 100},	/* P4 Prescott */
-	{X86_VENDOR_INTEL, 0xF, 0x4, 100},	/* P4 Prescott */
-	{X86_VENDOR_INTEL, 0x10,ANY, 0},	/* Itanium 2 */
-	{X86_VENDOR_UNKNOWN, ANY, ANY, 0}	/* stop here */
+	{X86_VENDOR_AMD, 0x6, ANY, ANY, 90},		/* Athlon Duron etc */
+	{X86_VENDOR_AMD, 0xF, ANY, ANY, 24},		/* Athlon 64, Opteron and above VRM 24 */
+	{X86_VENDOR_INTEL, 0x6, 0x9, ANY, 85},		/* 0.13um too */
+	{X86_VENDOR_INTEL, 0x6, 0xB, ANY, 85},		/* Tualatin */
+	{X86_VENDOR_INTEL, 0x6, ANY, ANY, 82},		/* any P6 */
+	{X86_VENDOR_INTEL, 0x7, ANY, ANY, 0},		/* Itanium */
+	{X86_VENDOR_INTEL, 0xF, 0x0, ANY, 90},		/* P4 */
+	{X86_VENDOR_INTEL, 0xF, 0x1, ANY, 90},		/* P4 Willamette */
+	{X86_VENDOR_INTEL, 0xF, 0x2, ANY, 90},		/* P4 Northwood */
+	{X86_VENDOR_INTEL, 0xF, ANY, ANY, 100},		/* Prescott and above assume VRD 10 */
+	{X86_VENDOR_INTEL, 0x10, ANY, ANY, 0},		/* Itanium 2 */
+	{X86_VENDOR_CENTAUR, 0x6, 0x7, ANY, 85},	/* Eden ESP/Ezra */
+	{X86_VENDOR_CENTAUR, 0x6, 0x8, 0x7, 85},	/* Ezra T */
+	{X86_VENDOR_CENTAUR, 0x6, 0x9, 0x7, 85},	/* Nemiah */
+	{X86_VENDOR_CENTAUR, 0x6, 0x9, ANY, 1718},	/* C3-M */
+	{X86_VENDOR_UNKNOWN, ANY, ANY, ANY, 0}		/* stop here */
 };
 
-static int find_vrm(u8 eff_family, u8 eff_model, u8 vendor)
+static int find_vrm(u8 eff_family, u8 eff_model, u8 eff_stepping, u8 vendor)
 {
 	int i = 0;
 
@@ -139,7 +153,8 @@
 		if (vrm_models[i].vendor=vendor)
 			if ((vrm_models[i].eff_family=eff_family)
 			 && ((vrm_models[i].eff_model=eff_model) ||
-			     (vrm_models[i].eff_model=ANY)))
+			     (vrm_models[i].eff_model=ANY)) &&
+			     (eff_stepping <= vrm_models[i].eff_stepping))
 				return vrm_models[i].vrm_type;
 		i++;
 	}
@@ -151,7 +166,7 @@
 {
 	struct cpuinfo_x86 *c = cpu_data;
 	u32 eax;
-	u8 eff_family, eff_model;
+	u8 eff_family, eff_model, eff_stepping;
 	int vrm_ret;
 
 	if (c->x86 < 6)		/* Any CPU with family lower than 6 */
@@ -160,18 +175,19 @@
 	eax = cpuid_eax(1);
 	eff_family = ((eax & 0x00000F00)>>8);
 	eff_model  = ((eax & 0x000000F0)>>4);
+	eff_stepping = eax & 0xF;
 	if (eff_family = 0xF) {	/* use extended model & family */
 		eff_family += ((eax & 0x00F00000)>>20);
 		eff_model += ((eax & 0x000F0000)>>16)<<4;
 	}
-	vrm_ret = find_vrm(eff_family,eff_model,c->x86_vendor);
+	vrm_ret = find_vrm(eff_family, eff_model, eff_stepping, c->x86_vendor);
 	if (vrm_ret = 0)
 		printk(KERN_INFO "hwmon-vid: Unknown VRM version of your "
 		       "x86 CPU\n");
 	return vrm_ret;
 }
 
-/* and now something completely different for the non-x86 world */
+/* and now for something completely different for the non-x86 world */
 #else
 int vid_which_vrm(void)
 {

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Roger Lucas]

Hi Rudolf,

Thanks for the updated VRM patch.

This works correctly on my Via EDEN system - VRM is reported as version 8.5.

- Roger

-----Original Message-----
From: Rudolf Marek [mailto:r.marek@sh.cvut.cz] 
Sent: 05 November 2005 19:01
To: Roger Lucas
Cc: lm-sensors@lm-sensors.org; Jean Delvare
Subject: Re: [lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE
2.6

Roger Lucas wrote:
> Hi Rudolf,
> 
> Unfortunately, by this time of the evening I am in a similar state to you,
> so I'll try this along when I do the changes from Jean's code review
> (hopefully tomorrow).

Hello,

Khali thanks for the comments. Here comes much better version that should
work.

See attachment. Please test it.

Thanks,
Regards
Rudolf

[lm-sensors] Running lm-sensors on EPIA 5000 with Debian SARGE 2.6

[Jean Delvare]

Hi Rudolf, Roger, all,

> Thanks for the updated VRM patch.
>
> This works correctly on my Via EDEN system - VRM is reported as version
> 8.5.

If the patch is OK, please resend it with proper Signed-off-by line,
comment and title, so that I can add it to my stack.

Thanks,
--
Jean Delvare