[lm-sensors] vt1211 questions

[lm-sensors] vt1211 questions

[Juerg Haefliger]

Folks,

I'm currently modifying the vt1211 driver to add PWM fan control and
for (yet another) attempt to submit it to the 2.6.x kernel tree. While
going over the code (Lars Ekman's port of Mark D. Studebaker's
original driver) I noticed 2 things:

1) The mapping of the temperature reading to the actual sysfs node
name is convoluted. I.e. Reading1 maps to temp3 and Reading3 maps to
temp1. Comments in the code mention that this is to minimize changes
from the via686a driver. Since I'm currently cleaning up and extending
the code there's probably not much commonality between the two
anymore. So should I change that convoluted mapping to Reading1->temp1
and Reading3->temp3? It's much less confusing... :-)

2) 'Sensors' doesn't read temp1 because there's another note in the
code saying that temp1 (Reading3) is not implemented in vt1211. That's
not true, in fact Reading3 is the vt1211 internal thermal diode. So
'sensors' (and the libs) needs to be patched as well..

With that I'm proposing the following changes to the vt1211 driver
(both 2.4.x and 2.6.x) and the 'sensors' application (and all involved
parties/libraries as well of course):

Old:
Reading1 -> temp3 (read by 'sensors')
Reading3 -> temp1 (ignored by 'sensors')

New:
Reading1 -> temp1 (read by 'sensors')
Reading3 -> temp3 (read by 'sensors')

And of course sensors.conf has to be patched as well.

Thanks
...juerg

[lm-sensors] vt1211 questions

[Rudolf Marek]

Hello all,

Juerg are you still working on the port?

Seems Tomislav wants to test it...

> I'm currently modifying the vt1211 driver to add PWM fan control and
> for (yet another) attempt to submit it to the 2.6.x kernel tree. While
> going over the code (Lars Ekman's port of Mark D. Studebaker's
> original driver) I noticed 2 things:
> 
> 1) The mapping of the temperature reading to the actual sysfs node
> name is convoluted. I.e. Reading1 maps to temp3 and Reading3 maps to
> temp1. Comments in the code mention that this is to minimize changes
> from the via686a driver. Since I'm currently cleaning up and extending
> the code there's probably not much commonality between the two
> anymore. So should I change that convoluted mapping to Reading1->temp1
> and Reading3->temp3? It's much less confusing... :-)

Well this must be done in 2.4 tree too. If you want to change it then why not...

> 2) 'Sensors' doesn't read temp1 because there's another note in the
> code saying that temp1 (Reading3) is not implemented in vt1211. That's
> not true, in fact Reading3 is the vt1211 internal thermal diode. So
> 'sensors' (and the libs) needs to be patched as well..

This was already fixed by you. It was removed by mds with the in6 two years ago.
I found a datasheet here:
http://ftp.21ic.com.cn/%E6%8A%80%E6%9C%AF%E8%B5%84%E6%96%99/VIA%20CHIPSET/VT1211.PDF

> With that I'm proposing the following changes to the vt1211 driver
> (both 2.4.x and 2.6.x) and the 'sensors' application (and all involved
> parties/libraries as well of course):
> 
> Old:
> Reading1 -> temp3 (read by 'sensors')
> Reading3 -> temp1 (ignored by 'sensors')
> 
> New:
> Reading1 -> temp1 (read by 'sensors')
> Reading3 -> temp3 (read by 'sensors')
> 
> And of course sensors.conf has to be patched as well.
> 

Yes I agree. Why not. We will relase soon new lm-sensors version with old mapping. Maybe
if you want to release your driver soon just do as it is now, so people can use this version and not
the CVS one...

Please let us know if you already have something for 2.6 Maybe some base patch without PWM first?

regards
Rudolf

[lm-sensors] vt1211 questions

[Juerg Haefliger]

Hi all,

> Hello all,
>
> Juerg are you still working on the port?

Yes

> Seems Tomislav wants to test it...

Good. The more testing the better.

[snip]

> > I'm currently modifying the vt1211 driver to add PWM fan control and
> > for (yet another) attempt to submit it to the 2.6.x kernel
> > tree. While
> > going over the code (Lars Ekman's port of Mark D. Studebaker's
> > original driver) I noticed 2 things:
> >
> > 1) The mapping of the temperature reading to the actual sysfs node
> > name is convoluted. I.e. Reading1 maps to temp3 and Reading3 maps to
> > temp1. Comments in the code mention that this is to minimize changes
> > from the via686a driver. Since I'm currently cleaning up
> > and extending
> > the code there's probably not much commonality between the two
> > anymore. So should I change that convoluted mapping to
> > Reading1->temp1
> > and Reading3->temp3? It's much less confusing... :-)
>
> Well this must be done in 2.4 tree too. If you want to change
> it then why not...

At this point, I leave the mapping as it is (in 2.4) and make sure
that the 2.6 version of the driver will have the same mapping. Maybe I
will fix it in both 2.4 and 2.6 at a later time. It's not a high
priority, just cosmetics.

> > 2) 'Sensors' doesn't read temp1 because there's another note in the
> > code saying that temp1 (Reading3) is not implemented in
> vt1211. That's
> > not true, in fact Reading3 is the vt1211 internal thermal diode. So
> > 'sensors' (and the libs) needs to be patched as well..
>
> This was already fixed by you. It was removed by mds with the
> in6 two years ago.
> I found a datasheet here:
> http://ftp.21ic.com.cn/%E6%8A%80%E6%9C%AF%E8%B5%84%E6%96%99/VI
> A%20CHIPSET/VT1211.PDF

The datasheet states that temp1 is reserved for the internal
temperature. That doesn't mean it cant be read. And yes, it's fixed in
the current CVS version..

> With that I'm proposing the following changes to the vt1211 driver
> (both 2.4.x and 2.6.x) and the 'sensors' application (and all involved
> parties/libraries as well of course):
>
> Old:
> Reading1 -> temp3 (read by 'sensors')
> Reading3 -> temp1 (ignored by 'sensors')
>
> New:
> Reading1 -> temp1 (read by 'sensors')
> Reading3 -> temp3 (read by 'sensors')
>
> And of course sensors.conf has to be patched as well.
>
>
> Yes I agree. Why not. We will relase soon new lm-sensors version with old mapping. Maybe
> if you want to release your driver soon just do as it is now, so people can use this version and > not the CVS one...

As mentioned earlier, I'll make sure the 2.6 driver has the same
mapping as the current 2.4 driver.

> Please let us know if you already have something for 2.6 Maybe some base patch without PWM > first?

I do have a working (I believe) 2.6 version that does have PWM
support. Unfortunately I can't fully test it, because VIA decided to
*not* use the PWM outputs on the EPIA M10000 board (how stupid is
that?). The fans are just connected to 12V and are always running full
speed. I'm thinking of maybe reworking my board... :-)

I can certainly generate a patch and make it available for people to
test it. Speaking of making kernel patches: I'm not too familiar with
that. Am I supposed to generate a patch against the latest (devel)
kernel or is 2.6.14 good enough?

I'll try to do that over the next couple of days. I also need to
supply some basic information on how to use the PWM feature and it
also requires a new sensors.conf since the 2.6 driver doesn't do weird
scaling of temperature and voltage numbers anymore. It just returns
the true register values and any scaling is performed via the compute
lines in sensors.conf.

Is it possible to host the patch on the lm-sensors webpage and maybe
issue a call to VT1211 users? Just like you did it with the VT8231.

Thanks and regards
...juerg



> regards
> Rudolf

[lm-sensors] vt1211 questions

[Rudolf Marek]

Juerg Haefliger wrote:
> Hi all,
> 
> 
>>Hello all,
>>
>>Juerg are you still working on the port?
> 
> Yes

Good.

> 
> The datasheet states that temp1 is reserved for the internal
> temperature. That doesn't mean it cant be read. And yes, it's fixed in
> the current CVS version..

Yep.

>>Yes I agree. Why not. We will relase soon new lm-sensors version with old mapping. Maybe
>>if you want to release your driver soon just do as it is now, so people can use this version and > not the CVS one...
> 
> 
> As mentioned earlier, I'll make sure the 2.6 driver has the same
> mapping as the current 2.4 driver.

Good.

> 
> 
>>Please let us know if you already have something for 2.6 Maybe some base patch without PWM > first?
> 
> 
> I do have a working (I believe) 2.6 version that does have PWM
> support. Unfortunately I can't fully test it, because VIA decided to
> *not* use the PWM outputs on the EPIA M10000 board (how stupid is
> that?). The fans are just connected to 12V and are always running full
> speed. I'm thinking of maybe reworking my board... :-)

Heh should not be so difficult.
1) make sure the pin for PWM is not for something else
2) tap/solder the pin on sio
3) put the transitor plus RC filter there
4) plug the fan ;)

maybe #3 is somewhere described is sample schematics.


> I can certainly generate a patch and make it available for people to
> test it. Speaking of making kernel patches: I'm not too familiar with
> that. Am I supposed to generate a patch against the latest (devel)
> kernel or is 2.6.14 good enough?

Well for real inclusion in should be against latestest RC version or git
For some test make sure it works with 2.6.15.

You can read the Documenation/CodingStyle and SubmittingPatches files to get
clue how to do that. Dont worry we will help you ;)

You can send patches in text attachment to this list. (no inline is neccessary)

> I'll try to do that over the next couple of days. I also need to
> supply some basic information on how to use the PWM feature and it
> also requires a new sensors.conf since the 2.6 driver doesn't do weird
> scaling of temperature and voltage numbers anymore. It just returns
> the true register values and any scaling is performed via the compute
> lines in sensors.conf.

We have a interface standard in the kernel Documenation/hwmon directory, just read the sysfs-interface.
If you need further help on that please write here.

> Is it possible to host the patch on the lm-sensors webpage and maybe
> issue a call to VT1211 users? Just like you did it with the VT8231.

Sure. We can do that on lm-sensors website or mine or Jean's ;)

Regards
Rudolf

[lm-sensors] vt1211 questions

[Jean Delvare]

Hi Juerg,

> At this point, I leave the mapping as it is (in 2.4) and make sure
> that the 2.6 version of the driver will have the same mapping. Maybe I
> will fix it in both 2.4 and 2.6 at a later time. It's not a high
> priority, just cosmetics.

We will most certainly need to do the same changes in 2.4 we had to do
for the vt8231 driver. But obviously not just before the freshly
announced 2.10.0 release. And you're right, the 2.4 and 2.6 drivers will
have to be in sync. This means not only the input numbers, but also, in
part, the values. Magnitude differences are OK, libsensors can deal
with them, but that's about it.

> I can certainly generate a patch and make it available for people to
> test it. Speaking of making kernel patches: I'm not too familiar with
> that. Am I supposed to generate a patch against the latest (devel)
> kernel or is 2.6.14 good enough?

I can only accept patches against a recent tree. Ideally, the latest
-mm tree. At the very least, Linus' latest (-git).

> I'll try to do that over the next couple of days. I also need to
> supply some basic information on how to use the PWM feature and it
> also requires a new sensors.conf since the 2.6 driver doesn't do weird
> scaling of temperature and voltage numbers anymore. It just returns
> the true register values and any scaling is performed via the compute
> lines in sensors.conf.

This might not be acceptable with regards to the standard interface as
defined in Documentation/hwmon/sysfs-interface. As much as possible,
the values you pass through sysfs must make some physical sense, even
if they are not direct readings. See how we did for the VT8231, it has
many common points with the VT1211 so you should be able to do the same.

> Is it possible to host the patch on the lm-sensors webpage and maybe
> issue a call to VT1211 users? Just like you did it with the VT8231.

We can't really host temporary files on the lm-sensors webpage, but we
can make annoucements and point to any external URI you'd have set up.
If you can't host the patch yourself, just send it to me and I'll make
it available on my server (if so, I'll also update it after that as
needed.)

Thanks,
-- 
Jean Delvare

[lm-sensors] vt1211 questions

[Juerg Haefliger]

Hi Jean,

Thanks for the reply.

>> I can certainly generate a patch and make it available for people to
>> test it. Speaking of making kernel patches: I'm not too familiar with
>> that. Am I supposed to generate a patch against the latest (devel)
>> kernel or is 2.6.14 good enough?
>
> I can only accept patches against a recent tree. Ideally, the latest
> -mm tree. At the very least, Linus' latest (-git).

OK, will do that.

>
>> I'll try to do that over the next couple of days. I also need to
>> supply some basic information on how to use the PWM feature and it
>> also requires a new sensors.conf since the 2.6 driver doesn't do weird
>> scaling of temperature and voltage numbers anymore. It just returns
>> the true register values and any scaling is performed via the compute
>> lines in sensors.conf.
>
> This might not be acceptable with regards to the standard interface as
> defined in Documentation/hwmon/sysfs-interface. As much as possible,
> the values you pass through sysfs must make some physical sense, even
> if they are not direct readings. See how we did for the VT8231, it has
> many common points with the VT1211 so you should be able to do the same.

I did look at the documentation and it states that the return values
are supposed to be in millivolts or millicelcius. But since the
conversion from register values to physically meaningful values is
highly dependent on the board implementation I believe the whole
conversion needs to happen in userspace. I also looked at the vt8231
and the driver doesn't scale the voltage register values at all but
scales the temperature readings. It multiplies 8bit register values by
10 and 10bit register values by 2.5. I don't understand why there is
prescaling in the driver. Can't that all be done by libsensors (except
for the truncation to 8bits)?
>
>> Is it possible to host the patch on the lm-sensors webpage and maybe
>> issue a call to VT1211 users? Just like you did it with the VT8231.
>
> We can't really host temporary files on the lm-sensors webpage, but we
> can make annoucements and point to any external URI you'd have set up.
> If you can't host the patch yourself, just send it to me and I'll make
> it available on my server (if so, I'll also update it after that as
> needed.)

OK great, thanks!
...juerg


> Thanks,
> --
> Jean Delvare
>