Re: [lm-sensors] Proposal

Re: [lm-sensors] Proposal

[Hans-Jürgen Koch]

Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:

>
> The abituguru3 has a register which contains a motherboard ID. The current
> driver uses this ID, to look up info about the motherboard in a somewhat
> lenght table in the driver. 

Can you elaborate your design decision a bit? My first idea would be to have
a sysfs file that delivers that motherboard ID and then do the lookup in
user space.

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Hans-Jürgen Koch wrote:
> Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
> 
>> The abituguru3 has a register which contains a motherboard ID. The current
>> driver uses this ID, to look up info about the motherboard in a somewhat
>> lenght table in the driver. 
> 
> Can you elaborate your design decision a bit? My first idea would be to have
> a sysfs file that delivers that motherboard ID and then do the lookup in
> user space.
> 

As I don't want the abituguru3 driver to create entries in sysfs for sensors 
which aren't there, and as without the table in the driver I cannot be sure 
wether to create an in / temp / fan device for a given sensor address. Last but 
not least doing things this way allows me to always give a proper reading 
without userspace needing to "guess" any further nescesarry calculations to get 
from the reading to an actual measurement.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans-Jürgen Koch]

Am Sonntag 08 April 2007 19:36 schrieb Hans de Goede:
> Hans-Jürgen Koch wrote:
> > Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
> >> The abituguru3 has a register which contains a motherboard ID. The
> >> current driver uses this ID, to look up info about the motherboard in a
> >> somewhat lenght table in the driver.
> >
> > Can you elaborate your design decision a bit? My first idea would be to
> > have a sysfs file that delivers that motherboard ID and then do the
> > lookup in user space.
>
> As I don't want the abituguru3 driver to create entries in sysfs for
> sensors which aren't there, and as without the table in the driver I cannot
> be sure wether to create an in / temp / fan device for a given sensor
> address. 

Good point. Anyway, most sensor drivers have no way of knowing which of their
inputs are actually used. The LM93 driver I'm currently working on is a 
monster that creates about 160 (!) sysfs files. I don't find it easy to
judge what's worse, having a few unused sysfs files or a large table in
kernel space.

> Last but not least doing things this way allows me to always give 
> a proper reading without userspace needing to "guess" any further
> nescesarry calculations to get from the reading to an actual measurement.

I agree that this is convenient, especially when reading sysfs values e.g.
from a script (without a library that could do table lookups). This is
probably a strong argument in favor of your table solution.

Thanks,
Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Hans-Jürgen Koch wrote:
> Am Sonntag 08 April 2007 19:36 schrieb Hans de Goede:
>> Hans-Jürgen Koch wrote:
>>> Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
>>>> The abituguru3 has a register which contains a motherboard ID. The
>>>> current driver uses this ID, to look up info about the motherboard in a
>>>> somewhat lenght table in the driver.
>>> Can you elaborate your design decision a bit? My first idea would be to
>>> have a sysfs file that delivers that motherboard ID and then do the
>>> lookup in user space.
>> As I don't want the abituguru3 driver to create entries in sysfs for
>> sensors which aren't there, and as without the table in the driver I cannot
>> be sure wether to create an in / temp / fan device for a given sensor
>> address. 
> 
> Good point. Anyway, most sensor drivers have no way of knowing which of their
> inputs are actually used. The LM93 driver I'm currently working on is a 
> monster that creates about 160 (!) sysfs files. I don't find it easy to
> judge what's worse, having a few unused sysfs files or a large table in
> kernel space.
> 

Only 160? My abituguru 1/2 driver creates 177 for my motherboard and even more 
on some others (it can detect used versus unused inputs). And the abituguru3 
driver can create 230 with known motherboards, and if i wouldn't use the table 
approach it would create 384 entries. Sorry I'm bragging here I couldn't help 
myself, and hopefully this also makes it even clearer why I went with the table 
approach.

Also notice that the table (except for the used entry) could be made so that it 
is freed after module initialisation (currently it isn't).

>> Last but not least doing things this way allows me to always give 
>> a proper reading without userspace needing to "guess" any further
>> nescesarry calculations to get from the reading to an actual measurement.
> 
> I agree that this is convenient, especially when reading sysfs values e.g.
> from a script (without a library that could do table lookups). This is
> probably a strong argument in favor of your table solution.
> 

Yes its very convinient, in an ideal world all hwmon drivers would do this and 
libsensors would not be necessary. This is in no way meant to say that other 
drivers are written badly, I know the hardware makes it nearly impossible todo 
this for other drivers.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans-Jürgen Koch]

Am Sonntag 08 April 2007 20:56 schrieb Hans de Goede:

> >
> > Good point. Anyway, most sensor drivers have no way of knowing which of
> > their inputs are actually used. The LM93 driver I'm currently working on
> > is a monster that creates about 160 (!) sysfs files. I don't find it easy
> > to judge what's worse, having a few unused sysfs files or a large table
> > in kernel space.
>
> Only 160? My abituguru 1/2 driver creates 177 for my motherboard and even
> more on some others (it can detect used versus unused inputs). And the
> abituguru3 driver can create 230 with known motherboards, and if i wouldn't
> use the table approach it would create 384 entries. Sorry I'm bragging here
> I couldn't help myself, and hopefully this also makes it even clearer why I
> went with the table approach.
>
> Also notice that the table (except for the used entry) could be made so
> that it is freed after module initialisation (currently it isn't).
>

That's not bragging... I thank you for your explanations!

Cheers,
Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

On Sun, 08 Apr 2007 20:56:30 +0200, Hans de Goede wrote:
> Hans-Jürgen Koch wrote:
> > Am Sonntag 08 April 2007 19:36 schrieb Hans de Goede:
> >> Last but not least doing things this way allows me to always give 
> >> a proper reading without userspace needing to "guess" any further
> >> nescesarry calculations to get from the reading to an actual measurement.
> > 
> > I agree that this is convenient, especially when reading sysfs values e.g.
> > from a script (without a library that could do table lookups). This is
> > probably a strong argument in favor of your table solution.
> 
> Yes its very convinient, in an ideal world all hwmon drivers would do this and 
> libsensors would not be necessary.

Wrong. Conversions belong to user-space. We must only put in the kernel
as little as possible. This is a golden rule. The current design with a
user-space library is the right way to do. If you think it makes it
hard to write shell scripts that manipulate hardware monitoring data,
it means the library is not properly designed and/or we are missing a
few helper programs. This is an implementation problem, not a design
problem.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Hans,

On Sun, 08 Apr 2007 19:36:47 +0200, Hans de Goede wrote:
> Hans-Jürgen Koch wrote:
> > Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
> > 
> >> The abituguru3 has a register which contains a motherboard ID. The current
> >> driver uses this ID, to look up info about the motherboard in a somewhat
> >> lenght table in the driver. 
> > 
> > Can you elaborate your design decision a bit? My first idea would be to have
> > a sysfs file that delivers that motherboard ID and then do the lookup in
> > user space.

I guess that the motherboard ID can be retrieved in user-space using
dmidecode, can't it? So it might not even be needed to export it.

> As I don't want the abituguru3 driver to create entries in sysfs for sensors 
> which aren't there, and as without the table in the driver I cannot be sure 
> wether to create an in / temp / fan device for a given sensor address. Last but 
> not least doing things this way allows me to always give a proper reading 
> without userspace needing to "guess" any further nescesarry calculations to get 
> from the reading to an actual measurement.

This is absolutely not different from all other hardware monitoring
drivers. And all other drivers handle it in user-space, because that's
the right design. I see no valid reason why it would be different for
your abituguru3 driver. All you need is one configuration file per
motherboard.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Hans,

On Sun, 08 Apr 2007 15:51:02 +0200, Hans de Goede wrote:
> The abituguru3 driver, of which I've just submitted a second version for 
> review, has a new feature / sysfs file, which I would like to discuss, before 
> people think I'm trying to keep this new sysfs entry below the radar.
> 
> The abituguru3 has a register which contains a motherboard ID. The current 
> driver uses this ID, to look up info about the motherboard in a somewhat lenght 
> table in the driver. This is done this way because the uguru3 has 48 sensors, 
> and the only way to know which one are actually connected and wether they are 
> connected to voltage / temp / fan is by using this table. The added advantage 
> of this is that since we know exactly on which motherboard the driver is 
> running, additional stuff can be don. Like always correct values in mV / milli 
> degress celcius / RPM at the sysfs level without libsensors needing todo any 

As I already expressed further in the thread: this violates the design
decision all other hardware monitoring drivers are built upon.
Conversions, and motherboard support in general, belong to user-space.

> conversion. An other additional feature this enables, is telling userspace 
> which voltage / temp / fan is actually being monitored. So the abituguru3 
> driver has sysfs entries like:
> /sys/class/hwmon/hwmon0/in0_label
> 
> Catting this file will result (depependend on motherboard) in for example:
> CPU Core
> being returned
> 
> I personally like this, as with proper userspace support this makes the end 
> user experience pretty plug and play. so what do you think, good or bad?
> 
> Currently libsensors cannot handle this, but I don't think that is an argument 
> either way.

I am, however, fine with *_label entries in sysfs, as long as they are
used with care. The use of these entries should be limited to the case
where pin use can be probed by the driver from the hardware state.
BTW, the coretemp driver already creates these files, even though it is
not properly documented in Documentation/hwmon/sysfs-interface yet. And
I have plans to create some label files in the it87 driver too.

These sysfs files should not be seen as a replacement for sensors.conf's
label statements. They should only be created when the driver knows,
directly from the hardware, that a given input is used for a
well-defined function, and user-space couldn't know that.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 08 Apr 2007 19:36:47 +0200, Hans de Goede wrote:
>> Hans-Jürgen Koch wrote:
>>> Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
>>>
>>>> The abituguru3 has a register which contains a motherboard ID. The current
>>>> driver uses this ID, to look up info about the motherboard in a somewhat
>>>> lenght table in the driver. 
>>> Can you elaborate your design decision a bit? My first idea would be to have
>>> a sysfs file that delivers that motherboard ID and then do the lookup in
>>> user space.
> 
> I guess that the motherboard ID can be retrieved in user-space using
> dmidecode, can't it? So it might not even be needed to export it.
> 

Nope, the motherboard ID used by the chip is a 16 bit unsigned int, not some 
kinda string like dmidecode gives.

>> As I don't want the abituguru3 driver to create entries in sysfs for sensors 
>> which aren't there, and as without the table in the driver I cannot be sure 
>> wether to create an in / temp / fan device for a given sensor address. Last but 
>> not least doing things this way allows me to always give a proper reading 
>> without userspace needing to "guess" any further nescesarry calculations to get 
>> from the reading to an actual measurement.
> 
> This is absolutely not different from all other hardware monitoring
> drivers. And all other drivers handle it in user-space, because that's
> the right design. I see no valid reason why it would be different for
> your abituguru3 driver. All you need is one configuration file per
> motherboard.
> 

Actually it is _very_ different. The uguru is not a chip, its more a piece of 
software (looking from the driver POV) then a chip, thus sensors which aren't 
there really aren't there, they are not just unconnected pins, they _really_ 
aren't there.

And afaik some drivers have magic and or module options to not generate 
generate certain sysfs entries in certain cases, this is no different.

Also this is they way how Abit handles this. The windows software comes with an 
ini-files, each named with the hex value of the ID, and that is used to 
determine how to talk to the uguru (which addresses to use) and what to ask.

Last but not least, this way (with libsensors-support and/or dyn chip support), 
the experience is truely plug and play, isn't that what we want in the end?

The same argument could be made for PCI ID tables in the kernel for hardware 
not essential to booting (or through ramdisk, even for boot essential hardware) 
why have the PCI-id's in the kernel? Why not have them in userspace and always 
use the new_id sysfs files?

Again the same argument could be made for blacklists / whitelists for many 
things in the kernel. There always is a trade-off, between putting this info in 
the kernel or in userspace. You yourself agreed, that given the limited amount 
of motherboards featuring the uguru, it might even be an idea to add an export 
a dmi-info-table with the supported motherboards to the driver, how is this 
different?

Regards,

Hans







_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 08 Apr 2007 19:36:47 +0200, Hans de Goede wrote:
>> Hans-Jürgen Koch wrote:
>>> Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
>>>
>>>> The abituguru3 has a register which contains a motherboard ID. The current
>>>> driver uses this ID, to look up info about the motherboard in a somewhat
>>>> lenght table in the driver. 
>>> Can you elaborate your design decision a bit? My first idea would be to have
>>> a sysfs file that delivers that motherboard ID and then do the lookup in
>>> user space.
> 
> I guess that the motherboard ID can be retrieved in user-space using
> dmidecode, can't it? So it might not even be needed to export it.
> 
>> As I don't want the abituguru3 driver to create entries in sysfs for sensors 
>> which aren't there, and as without the table in the driver I cannot be sure 
>> wether to create an in / temp / fan device for a given sensor address. Last but 
>> not least doing things this way allows me to always give a proper reading 
>> without userspace needing to "guess" any further nescesarry calculations to get 
>> from the reading to an actual measurement.
> 
> This is absolutely not different from all other hardware monitoring
> drivers. And all other drivers handle it in user-space, because that's
> the right design. I see no valid reason why it would be different for
> your abituguru3 driver. All you need is one configuration file per
> motherboard.
> 

Oh and another thing I just came up with, last time I checked, it isn't 
possible from userspace to change an inX_input to a tempX_input, or are you 
suggesting that I export all 48 sensors, 3 times, once as in, once as temp and 
once as fan and them make user space ignore temp1 and fan1, and use in0, as 
sensor 0/1 actually is an in sensor?

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 08 Apr 2007 19:36:47 +0200, Hans de Goede wrote:
>> Hans-Jürgen Koch wrote:
>>> Am Sonntag 08 April 2007 15:51 schrieb Hans de Goede:
>>>
>>>> The abituguru3 has a register which contains a motherboard ID. The current
>>>> driver uses this ID, to look up info about the motherboard in a somewhat
>>>> lenght table in the driver. 
>>> Can you elaborate your design decision a bit? My first idea would be to have
>>> a sysfs file that delivers that motherboard ID and then do the lookup in
>>> user space.
> 
> I guess that the motherboard ID can be retrieved in user-space using
> dmidecode, can't it? So it might not even be needed to export it.
> 
>> As I don't want the abituguru3 driver to create entries in sysfs for sensors 
>> which aren't there, and as without the table in the driver I cannot be sure 
>> wether to create an in / temp / fan device for a given sensor address. Last but 
>> not least doing things this way allows me to always give a proper reading 
>> without userspace needing to "guess" any further nescesarry calculations to get 
>> from the reading to an actual measurement.
> 
> This is absolutely not different from all other hardware monitoring
> drivers. And all other drivers handle it in user-space, because that's
> the right design. I see no valid reason why it would be different for
> your abituguru3 driver. All you need is one configuration file per
> motherboard.
> 

Oh and yet another thing, since when should all conversions be done in 
userspace? I did exactly that with the abituguru 1/2 driver and there I had to 
change it to give the return the actual pin values in Volts and not register 
values, iow do the conversion in the driver. You are contradicting yourself!

Sorry, but I'm pissed about this you're bashing a driver at which you haven't 
even looked, even though it has been submitted for review in Januari!

Then I asked if it was a good idea if I would exchange reviews with Juerg and 
his DME1737 driver, if that was ok with you. I _explictly_ asked this, and then 
after the review and a new version was posted by Juerg, you said that you would 
still have to review it yourself. What was the fricking point then in me 
reviewing it in the first place? Why did you think I asked if it was ok for 
others to review drivers?? I asked because I already was afraid that you would 
not accept reviews done by others, and tada you don't!

This attitude is (needlessly) slowing down lmsensors development, and might 
very well be one of the reasons why there hardly is a development community 
surrounding lmsensors.

Regards,

Hans





_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hans,

On Mon, 09 Apr 2007 14:26:06 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > I guess that the motherboard ID can be retrieved in user-space using
> > dmidecode, can't it? So it might not even be needed to export it.
> 
> Nope, the motherboard ID used by the chip is a 16 bit unsigned int, not some 
> kinda string like dmidecode gives.

I didn't mean the 16-bit number itself, just one way to uniquely
identify the motherboard model.

> > This is absolutely not different from all other hardware monitoring
> > drivers. And all other drivers handle it in user-space, because that's
> > the right design. I see no valid reason why it would be different for
> > your abituguru3 driver. All you need is one configuration file per
> > motherboard.
> 
> Actually it is _very_ different. The uguru is not a chip, its more a piece of 
> software (looking from the driver POV) then a chip, thus sensors which aren't 
> there really aren't there, they are not just unconnected pins, they _really_ 
> aren't there.

You don't have to convince me that Abit "chips" are crap, I know that
already ;)

More seriously, I'm sorry but I just can't see the difference. Other
hardware monitoring chips can have pins used for different functions.
Their drivers have to create the sysfs files which match the hardware
reality, and they do it in a motherboard-independent way. Your
abituguru3 driver must do the same. If you can't get the wiring
information from the hardware (or embedded software), this a major
design flaw from Abit, or the price to pay for writing a driver for an
undocumented device.

Remember how I told you months ago that I did not want us to support
undocumented chips because I knew it would be painful? Here you are.

> And afaik some drivers have magic and or module options to not generate 
> generate certain sysfs entries in certain cases, this is no different.

What kind of "magic" are you thinking of? Many drivers are reading
their configuration registers to find out wiring information, but
there's nothing magic about that.

The only module options we have that change which sysfs entries are
created are:
* uch_config in the vt1211 driver, which I didn't like, and I don't
think anyone is actually using it.
* extra_sensor_type in the gl520sm driver, which I didn't like either,
and I don't think anyone is actually using it.
In both cases, the parameter is there just in case the BIOS failed to
properly initialize the device, and should otherwise never be needed.

Oh, and of course the abituguru driver has lots of module parameters to
control a whole lot of things. But you know about that one already ;)

Anyway, the main point here is that none of our drivers include
motherboard-specific information. Except hdaps, granted, and it is a
pain to maintain, which is why I don't want other drivers to do the
same.

> Also this is they way how Abit handles this. The windows software comes with an 
> ini-files, each named with the hex value of the ID, and that is used to 
> determine how to talk to the uguru (which addresses to use) and what to ask.

So they have a user-space tool which reprograms the driver depending on
the ID. This is quite different from what you proposed, isn't it?

> Last but not least, this way (with libsensors-support and/or dyn chip support), 
> the experience is truely plug and play, isn't that what we want in the end?

What we want is not to have to update the kernel drivers each time a
new motherboard is released. Of course, the less effort is needed from
the end user, the better, but this doesn't mean everything is in the
driver. This might as well mean a better infrastructure in user-space,
which is exactly what some of your students have been, and are, working
on.

> The same argument could be made for PCI ID tables in the kernel for hardware 
> not essential to booting (or through ramdisk, even for boot essential hardware) 
> why have the PCI-id's in the kernel? Why not have them in userspace and always 
> use the new_id sysfs files?

This is an interesting point. I see two main differences though.
Firstly, as you wrote, the PCI ID tables are needed at boot time for a
number of drivers. Once the infrastructure is in place, the overhead to
extend its usage to all PCI devices is thin, so it would be stupid not
to do it. Secondly, PCI ID tables are listing devices, not motherboards.

That being said, new PCI IDs can already be quite painful. If you look
at the patches that we've been taking for the popular SMBus master
drivers (i2c-i801, i2c-viapro, i2c-nforce2, i2c-piix4) over the past
few years, you'll see that most of them are simply adding new PCI IDs.
If the new_id sysfs file was more broadly used, it wouldn't take a new
kernel to support these new devices. OTOH, the real problem here is that
manufacturers should stop giving new PCI IDs to 100% compatible
devices, it's useless :(

> Again the same argument could be made for blacklists / whitelists for many 
> things in the kernel. There always is a trade-off, between putting this info in 
> the kernel or in userspace. You yourself agreed, that given the limited amount 
> of motherboards featuring the uguru, it might even be an idea to add an export 
> a dmi-info-table with the supported motherboards to the driver, how is this 
> different?

I seem to remember that I agreed that the abituguru driver could check
that the motherboard maker was Abit using the DMI data. I don't
remember what my opinion was on a complete DMI table as the hdaps
driver has. Either way, my point was to make sure that the driver
wouldn't cause trouble is loaded on a system without an uGuru chip.
There's a major difference between identifying the motherboard and
embedding motherboard-specific configuration information inside the
driver. With the approach you are advocating, I fear that pretty much
every problem we might encounter (bug in the feature defintions, new
motherboard model, new revision of a supported motherboard model,
etc...) will take a driver update to solve. This is what I would like
to avoid.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Hans,

On Mon, 09 Apr 2007 14:34:35 +0200, Hans de Goede wrote:
> Oh and another thing I just came up with, last time I checked, it isn't 
> possible from userspace to change an inX_input to a tempX_input, or are you 
> suggesting that I export all 48 sensors, 3 times, once as in, once as temp and 
> once as fan and them make user space ignore temp1 and fan1, and use in0, as 
> sensor 0/1 actually is an in sensor?

You're right, this isn't possible, because it's not needed. The drivers
are usually able to read the device configuration from its registers,
and create the right sysfs entries based on that information. The only
exceptions being the vt1211 and gl520sm drivers, which I mentioned in
my previous reply.

So I seem to understand that, for the abituguru3, you are not able to
read the configuration from the device? Again I must blame Abit for
their very poor design and/or lack of documentation :(

Didn't you have the same problem originally in the first abiturugu
driver? If so, how did you solve it?

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

On Mon, 09 Apr 2007 14:50:03 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > On Sun, 08 Apr 2007 19:36:47 +0200, Hans de Goede wrote:
> >> (...) Last but 
> >> not least doing things this way allows me to always give a proper reading 
> >> without userspace needing to "guess" any further nescesarry calculations to get 
> >> from the reading to an actual measurement.
> > 
> > This is absolutely not different from all other hardware monitoring
> > drivers. And all other drivers handle it in user-space, because that's
> > the right design. I see no valid reason why it would be different for
> > your abituguru3 driver. All you need is one configuration file per
> > motherboard.
> 
> Oh and yet another thing, since when should all conversions be done in 
> userspace? I did exactly that with the abituguru 1/2 driver and there I had to 
> change it to give the return the actual pin values in Volts and not register 
> values, iow do the conversion in the driver. You are contradicting yourself!

What I really meant was that motherboard-specific conversions, such as
the ones accounting for voltage scaling resistors, belong to
user-space. The conversions from register values to standard units (mV,
RPM etc...) still belong to kernel-space in our model.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hans,
> 
> On Mon, 09 Apr 2007 14:26:06 +0200, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> This is absolutely not different from all other hardware monitoring
>>> drivers. And all other drivers handle it in user-space, because that's
>>> the right design. I see no valid reason why it would be different for
>>> your abituguru3 driver. All you need is one configuration file per
>>> motherboard.
>> Actually it is _very_ different. The uguru is not a chip, its more a piece of 
>> software (looking from the driver POV) then a chip, thus sensors which aren't 
>> there really aren't there, they are not just unconnected pins, they _really_ 
>> aren't there.
> 
> You don't have to convince me that Abit "chips" are crap, I know that
> already ;)
> 

Actually they are quite nice, but unfortunately not documented. Being able to 
correctly provide all info to userspace without needing any further 
configuration / conversion is a feature. Also they tend to give very stable / 
non erratic readings.

> More seriously, I'm sorry but I just can't see the difference. Other
> hardware monitoring chips can have pins used for different functions.
> Their drivers have to create the sysfs files which match the hardware
> reality, and they do it in a motherboard-independent way. Your
> abituguru3 driver must do the same. If you can't get the wiring
> information from the hardware (or embedded software), this a major
> design flaw from Abit, or the price to pay for writing a driver for an
> undocumented device.
> 

I can get the wiring info from the chip and I do, thats why the motherboard ID 
register is there, in essence the abituguru 3 is a familiy of many different 
chips, with the motherboard ID register telling you the exact model of the 
chip. Think of it as a model/stepping/chip identification register instead of a 
motherboard id register if you must.

Once I know which model of the abituguru3 family the driver us running on, then 
I look at my table describing all the models of the abituguru3 chip family and 
in that way find out what features this model has and thus which sysfs entries 
to create.

> Remember how I told you months ago that I did not want us to support
> undocumented chips because I knew it would be painful? Here you are.
> 

Actually writing the driver has been pretty painless, defending the (in my mind 
completely sane) table approach OTOH indeed is painful.

>> And afaik some drivers have magic and or module options to not generate 
>> generate certain sysfs entries in certain cases, this is no different.
> 
> What kind of "magic" are you thinking of? Many drivers are reading
> their configuration registers to find out wiring information, but
> there's nothing magic about that.
> 

So is this one, its reading the configuration register which tells the driver 
which model/stepping we are running on.

> Oh, and of course the abituguru driver has lots of module parameters to
> control a whole lot of things. But you know about that one already ;)
> 

Yes the abituguru (rev 1/2) driver is a pain as the autodetect code doesn't 
work on revision 1 motherboards. I've been thinking about identifying the 2 
revision 1 motherboards using dmi and then those options will no longer be needed.

> Anyway, the main point here is that none of our drivers include
> motherboard-specific information. Except hdaps, granted, and it is a
> pain to maintain, which is why I don't want other drivers to do the
> same.
> 

I don't know about the hdaps driver, but I can promise you for the abituguru3 
this won't be a pain to maintain:
1) new motherboard gets released
2) download updated windows software
3) read ini file for new motherboard and add additional entry to table
    in abituguru3.c
4) done

>> Also this is they way how Abit handles this. The windows software comes with an 
>> ini-files, each named with the hex value of the ID, and that is used to 
>> determine how to talk to the uguru (which addresses to use) and what to ask.
> 
> So they have a user-space tool which reprograms the driver depending on
> the ID. This is quite different from what you proposed, isn't it?
> 

No they have a "driver" allowing port banging and do everything else in userspace.

>> Last but not least, this way (with libsensors-support and/or dyn chip support), 
>> the experience is truely plug and play, isn't that what we want in the end?
> 
> What we want is not to have to update the kernel drivers each time a
> new motherboard is released. Of course, the less effort is needed from
> the end user, the better, but this doesn't mean everything is in the
> driver. This might as well mean a better infrastructure in user-space,
> which is exactly what some of your students have been, and are, working
> on.
> 

Yes, and my previous students have been working on dyn chipsupport, which is 
meant to dynamicly build a list of features. What is the point in:
1) creating sysfs entries for features which aren't there
2) dynamicly creating a list of features for this chip in libsensors based on
    sysfs entries which then will contain features which aren't there
3) having to create / find / install through dmi magic a /etc/sensors.conf
    telling libsensors to ignore the features which aren't there?

Really I see no point in this. Actually when I use the generic printing 
routines with the k8temp driver and a stock /etc/sensors.conf I get 3 errors 
because my chip only has 1 temp sensor. Now if I comment the k8temp entry in 
lib/chips.c so that it uses the dyn chip code, then it will work fine out of 
the box with the generic print code. The k8temp specific print code works 
around this be silently ignoring get_feature errors.

This for example also happens with gkrellm compiled with libsensors support + 
k8temp + stock /etc/sensors.conf. With the static entry in lib/chips.c I get 4 
k8temp entries in gkrellm of which 3 show an error. With the dyn chip support 
it works as advertised. As the k8temp driver does the right thing and doesn't 
create sysfs entries for sensors which aren't there. Because in my vision, 
sysfs should represent the actual wiring of a chip as accurate as possible

When I wrote the abituguru3 driver I've thought about where to put this 
information for a couple of days before deciding todo things this way, here is 
my chain of thoughts as far as I remember it:

1) sysfs should represent the actual wiring of the chips as accurate as
    possible
2) for this I need a table with model/stepping/chip id's and per model which
    sensors are at which "addresses" and what type they have
3) since I need that table anyways I might as well at the conversion and
    label info, so that all this information is in one place

I think we agree on 1), you yourself said you're fine with this as long as this 
is done using chip configuration registers. So then the discussion becomes is 
the abituguru3 ID register a configuration register or not?

I say it is, I guess I should maybe change the text / kprints to make this 
clear. Actually I can proof that this register does not identify the 
motherboard but rather the sensor config / layout as both the "Abit AT8" and 
the "Abit AT8 32x" contain 0x0011 in this register (and have identical sensors) 
but they are 2 completely different motherboards, 1 uses the ati RD480 chipset 
and the other the ati RD580, see:
http://www.abit-usa.com/products/mb/techspec.php?categories=1&model09
http://www.abit-usa.com/products/mb/techspec.php?categories=1&model11

> With the approach you are advocating, I fear that pretty much
> every problem we might encounter (bug in the feature defintions, new
> motherboard model, new revision of a supported motherboard model,
> etc...) will take a driver update to solve. This is what I would like
> to avoid.
> 

I understand, but this is not a black and white issue. As said I believe that 
it is important that sysfs represents the actual wiring of the chips as 
accurate as possible. To me this is more important then having to update the 
driver for new motherboards now and then (its not like Abit does a new 
motherboard every month). Also the driver will simply refuse to work with newer 
motherboards atm, however it could be changed to just export all sensors for 
unknown motherboards as you want it todo for all motherboards, although I don't 
know if this is a food idea.

Regards,

Hans




_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Mark M. Hoffman]

Hi everyone:

Finally, back to the thread that resulted in change of maintainers, but not
yet any merged code. ;)

* Hans de Goede <j.w.r.degoede@hhs.nl> [2007-04-11 15:49:08 +0200]:
> Jean Delvare wrote:
> > Hans,
> > 
> > On Mon, 09 Apr 2007 14:26:06 +0200, Hans de Goede wrote:
> >> Jean Delvare wrote:
> >>> This is absolutely not different from all other hardware monitoring
> >>> drivers. And all other drivers handle it in user-space, because that's
> >>> the right design. I see no valid reason why it would be different for
> >>> your abituguru3 driver. All you need is one configuration file per
> >>> motherboard.
> >> Actually it is _very_ different. The uguru is not a chip, its more a piece of 
> >> software (looking from the driver POV) then a chip, thus sensors which aren't 
> >> there really aren't there, they are not just unconnected pins, they _really_ 
> >> aren't there.

There is another way to look at it: the uguru3 is not a single chip, it is a
*family* of chips.  With that model, we would end up with a compromise between
your positions that has precedent in other drivers.

I.e. Append the motherboard ID to the chip name, and also use that to create
the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
because you can put a section in sensors.conf for each variant of uguru3.

It's not that I'm against the label files per se.  But IMHO this suggestion
is the closest fit to the existing model of operation.

> > You don't have to convince me that Abit "chips" are crap, I know that
> > already ;)
> > 
> 
> Actually they are quite nice, but unfortunately not documented. Being able to 
> correctly provide all info to userspace without needing any further 
> configuration / conversion is a feature. Also they tend to give very stable / 
> non erratic readings.
> 
> > More seriously, I'm sorry but I just can't see the difference. Other
> > hardware monitoring chips can have pins used for different functions.
> > Their drivers have to create the sysfs files which match the hardware
> > reality, and they do it in a motherboard-independent way. Your
> > abituguru3 driver must do the same. If you can't get the wiring
> > information from the hardware (or embedded software), this a major
> > design flaw from Abit, or the price to pay for writing a driver for an
> > undocumented device.
> > 
> 
> I can get the wiring info from the chip and I do, thats why the motherboard ID 
> register is there, in essence the abituguru 3 is a familiy of many different 
> chips, with the motherboard ID register telling you the exact model of the 
> chip. Think of it as a model/stepping/chip identification register instead of a 
> motherboard id register if you must.
> 
> Once I know which model of the abituguru3 family the driver us running on, then 
> I look at my table describing all the models of the abituguru3 chip family and 
> in that way find out what features this model has and thus which sysfs entries 
> to create.
> 
> > Remember how I told you months ago that I did not want us to support
> > undocumented chips because I knew it would be painful? Here you are.
> > 
> 
> Actually writing the driver has been pretty painless, defending the (in my mind 
> completely sane) table approach OTOH indeed is painful.
> 
> >> And afaik some drivers have magic and or module options to not generate 
> >> generate certain sysfs entries in certain cases, this is no different.
> > 
> > What kind of "magic" are you thinking of? Many drivers are reading
> > their configuration registers to find out wiring information, but
> > there's nothing magic about that.
> > 
> 
> So is this one, its reading the configuration register which tells the driver 
> which model/stepping we are running on.

Yep, this leads directly to my suggestion above.  Guys, is that acceptable?
(/me is out, but leaving the rest uncut since it's such an old thread)

> > Oh, and of course the abituguru driver has lots of module parameters to
> > control a whole lot of things. But you know about that one already ;)
> > 
> 
> Yes the abituguru (rev 1/2) driver is a pain as the autodetect code doesn't 
> work on revision 1 motherboards. I've been thinking about identifying the 2 
> revision 1 motherboards using dmi and then those options will no longer be needed.
> 
> > Anyway, the main point here is that none of our drivers include
> > motherboard-specific information. Except hdaps, granted, and it is a
> > pain to maintain, which is why I don't want other drivers to do the
> > same.
> > 
> 
> I don't know about the hdaps driver, but I can promise you for the abituguru3 
> this won't be a pain to maintain:
> 1) new motherboard gets released
> 2) download updated windows software
> 3) read ini file for new motherboard and add additional entry to table
>     in abituguru3.c
> 4) done
> 
> >> Also this is they way how Abit handles this. The windows software comes with an 
> >> ini-files, each named with the hex value of the ID, and that is used to 
> >> determine how to talk to the uguru (which addresses to use) and what to ask.
> > 
> > So they have a user-space tool which reprograms the driver depending on
> > the ID. This is quite different from what you proposed, isn't it?
> > 
> 
> No they have a "driver" allowing port banging and do everything else in userspace.
> 
> >> Last but not least, this way (with libsensors-support and/or dyn chip support), 
> >> the experience is truely plug and play, isn't that what we want in the end?
> > 
> > What we want is not to have to update the kernel drivers each time a
> > new motherboard is released. Of course, the less effort is needed from
> > the end user, the better, but this doesn't mean everything is in the
> > driver. This might as well mean a better infrastructure in user-space,
> > which is exactly what some of your students have been, and are, working
> > on.
> > 
> 
> Yes, and my previous students have been working on dyn chipsupport, which is 
> meant to dynamicly build a list of features. What is the point in:
> 1) creating sysfs entries for features which aren't there
> 2) dynamicly creating a list of features for this chip in libsensors based on
>     sysfs entries which then will contain features which aren't there
> 3) having to create / find / install through dmi magic a /etc/sensors.conf
>     telling libsensors to ignore the features which aren't there?
> 
> Really I see no point in this. Actually when I use the generic printing 
> routines with the k8temp driver and a stock /etc/sensors.conf I get 3 errors 
> because my chip only has 1 temp sensor. Now if I comment the k8temp entry in 
> lib/chips.c so that it uses the dyn chip code, then it will work fine out of 
> the box with the generic print code. The k8temp specific print code works 
> around this be silently ignoring get_feature errors.
> 
> This for example also happens with gkrellm compiled with libsensors support + 
> k8temp + stock /etc/sensors.conf. With the static entry in lib/chips.c I get 4 
> k8temp entries in gkrellm of which 3 show an error. With the dyn chip support 
> it works as advertised. As the k8temp driver does the right thing and doesn't 
> create sysfs entries for sensors which aren't there. Because in my vision, 
> sysfs should represent the actual wiring of a chip as accurate as possible
> 
> When I wrote the abituguru3 driver I've thought about where to put this 
> information for a couple of days before deciding todo things this way, here is 
> my chain of thoughts as far as I remember it:
> 
> 1) sysfs should represent the actual wiring of the chips as accurate as
>     possible
> 2) for this I need a table with model/stepping/chip id's and per model which
>     sensors are at which "addresses" and what type they have
> 3) since I need that table anyways I might as well at the conversion and
>     label info, so that all this information is in one place
> 
> I think we agree on 1), you yourself said you're fine with this as long as this 
> is done using chip configuration registers. So then the discussion becomes is 
> the abituguru3 ID register a configuration register or not?
> 
> I say it is, I guess I should maybe change the text / kprints to make this 
> clear. Actually I can proof that this register does not identify the 
> motherboard but rather the sensor config / layout as both the "Abit AT8" and 
> the "Abit AT8 32x" contain 0x0011 in this register (and have identical sensors) 
> but they are 2 completely different motherboards, 1 uses the ati RD480 chipset 
> and the other the ati RD580, see:
> http://www.abit-usa.com/products/mb/techspec.php?categories=1&model09
> http://www.abit-usa.com/products/mb/techspec.php?categories=1&model11
> 
> > With the approach you are advocating, I fear that pretty much
> > every problem we might encounter (bug in the feature defintions, new
> > motherboard model, new revision of a supported motherboard model,
> > etc...) will take a driver update to solve. This is what I would like
> > to avoid.
> > 
> 
> I understand, but this is not a black and white issue. As said I believe that 
> it is important that sysfs represents the actual wiring of the chips as 
> accurate as possible. To me this is more important then having to update the 
> driver for new motherboards now and then (its not like Abit does a new 
> motherboard every month). Also the driver will simply refuse to work with newer 
> motherboards atm, however it could be changed to just export all sensors for 
> unknown motherboards as you want it todo for all motherboards, although I don't 
> know if this is a food idea.
> 
> Regards,
> 
> Hans

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Mark M. Hoffman]

Hi all:

* Jean Delvare <khali@linux-fr.org> [2007-04-11 13:49:01 +0200]:
> Hi Hans,
> 
> On Mon, 09 Apr 2007 14:34:35 +0200, Hans de Goede wrote:
> > Oh and another thing I just came up with, last time I checked, it isn't 
> > possible from userspace to change an inX_input to a tempX_input, or are you 
> > suggesting that I export all 48 sensors, 3 times, once as in, once as temp and 
> > once as fan and them make user space ignore temp1 and fan1, and use in0, as 
> > sensor 0/1 actually is an in sensor?
> 
> You're right, this isn't possible, because it's not needed. The drivers
> are usually able to read the device configuration from its registers,
> and create the right sysfs entries based on that information. The only
> exceptions being the vt1211 and gl520sm drivers, which I mentioned in
> my previous reply.

In this case, there is no practical difference between reading that info from a
register on the chip and inferring it from a register on the motherboard.

They are a matched pair, indivisible.  You can't just buy a tube of uguru3s
from digikey and wire them up to random projects like you can w/ e.g. lm93.

> So I seem to understand that, for the abituguru3, you are not able to
> read the configuration from the device? Again I must blame Abit for
> their very poor design and/or lack of documentation :(

See above.  Lack of docs is bad, of course; but I don't see any problem with
that design.  It's just different.  

> Didn't you have the same problem originally in the first abiturugu
> driver? If so, how did you solve it?

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Mark M. Hoffman]

Hi Hans:

> Jean Delvare wrote:
> > This is absolutely not different from all other hardware monitoring
> > drivers. And all other drivers handle it in user-space, because that's
> > the right design. I see no valid reason why it would be different for
> > your abituguru3 driver. All you need is one configuration file per
> > motherboard.

* Hans de Goede <j.w.r.degoede@hhs.nl> [2007-04-09 14:50:03 +0200]:
> Oh and yet another thing, since when should all conversions be done in 
> userspace? I did exactly that with the abituguru 1/2 driver and there I had to 
> change it to give the return the actual pin values in Volts and not register 
> values, iow do the conversion in the driver.
> [...]

The kernel-to-userspace interface for hwmon is the *measured* value at the
*pin*, in natural units.

If the pin on a chip can measure e.g. 0.0-5.0 volts, then the sysfs file will
report that voltage.

If that same pin is wired into a circuit which is used for measuring a 12V
power supply (and thus needs further conversion) that is *irrelevant* to the
driver.  The hwmon driver for a well-documented and general purpose sensor chip
need not concern itself with external circuitry.

(It cannot, short of moving all of libsensors or its equivalent into the kernel,
which would be horrible.  Even if we could track all mainboard types and
somehow update the kernel with all that info, you still need a way for embedded
people to reconfigure it for their custom boards or whatever.  *That* is why
we keep libsensors out of the kernel.  Not because of "if it can be done in
userspace, it should" or any nonsense like that.)

A chip like uguru breaks this model, because we have *no* *idea* what it might
be measuring at the *pin*.  But it turns out, it doesn't matter.  It's so
tightly coupled to the mainboard that we can treat the whole thing as a "chip".
The embedded case goes away (like I mentioned in another thread) because nobody
is designing the uguru into custom boards anyway.

I hope this rationale for the kernel/userspace API is clear now.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Mark M. Hoffman wrote:
> Hi all:
> 
> * Jean Delvare <khali@linux-fr.org> [2007-04-11 13:49:01 +0200]:
>> Hi Hans,
>>
>> On Mon, 09 Apr 2007 14:34:35 +0200, Hans de Goede wrote:
>>> Oh and another thing I just came up with, last time I checked, it isn't 
>>> possible from userspace to change an inX_input to a tempX_input, or are you 
>>> suggesting that I export all 48 sensors, 3 times, once as in, once as temp and 
>>> once as fan and them make user space ignore temp1 and fan1, and use in0, as 
>>> sensor 0/1 actually is an in sensor?
>> You're right, this isn't possible, because it's not needed. The drivers
>> are usually able to read the device configuration from its registers,
>> and create the right sysfs entries based on that information. The only
>> exceptions being the vt1211 and gl520sm drivers, which I mentioned in
>> my previous reply.
> 
> In this case, there is no practical difference between reading that info from a
> register on the chip and inferring it from a register on the motherboard.
>

To be precise, the info actually is read from a register on the chip, not a 
register on the motherboard (the chip is on the motherboard ofcourse).

So there really is no difference here, a register gets read and based on its 
contents it gets determined what inputs are available and what their types are.

As an added bonus, one can also determine the scaling factors and labels for 
the inputs too.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Mark M. Hoffman wrote:
> Hi Hans:
> 
>> Jean Delvare wrote:
>>> This is absolutely not different from all other hardware monitoring
>>> drivers. And all other drivers handle it in user-space, because that's
>>> the right design. I see no valid reason why it would be different for
>>> your abituguru3 driver. All you need is one configuration file per
>>> motherboard.
> 
> * Hans de Goede <j.w.r.degoede@hhs.nl> [2007-04-09 14:50:03 +0200]:
>> Oh and yet another thing, since when should all conversions be done in 
>> userspace? I did exactly that with the abituguru 1/2 driver and there I had to 
>> change it to give the return the actual pin values in Volts and not register 
>> values, iow do the conversion in the driver.
>> [...]
> 
> The kernel-to-userspace interface for hwmon is the *measured* value at the
> *pin*, in natural units.
> 
> If the pin on a chip can measure e.g. 0.0-5.0 volts, then the sysfs file will
> report that voltage.
> 
> If that same pin is wired into a circuit which is used for measuring a 12V
> power supply (and thus needs further conversion) that is *irrelevant* to the
> driver.  The hwmon driver for a well-documented and general purpose sensor chip
> need not concern itself with external circuitry.
> 
> (It cannot, short of moving all of libsensors or its equivalent into the kernel,
> which would be horrible.  Even if we could track all mainboard types and
> somehow update the kernel with all that info, you still need a way for embedded
> people to reconfigure it for their custom boards or whatever.  *That* is why
> we keep libsensors out of the kernel.  Not because of "if it can be done in
> userspace, it should" or any nonsense like that.)
> 
> A chip like uguru breaks this model, because we have *no* *idea* what it might
> be measuring at the *pin*.  But it turns out, it doesn't matter.  It's so
> tightly coupled to the mainboard that we can treat the whole thing as a "chip".
> The embedded case goes away (like I mentioned in another thread) because nobody
> is designing the uguru into custom boards anyway.
> 
> I hope this rationale for the kernel/userspace API is clear now.
> 

It is and always was, but as you say the uguru is a bit of a special case here.

I've got some good news about the fintek f77882fg / epox ep1308 chipb btw, my 
student (Hans Edgington) has returned the motherboard and last friday I build a 
system with it. I also compiled his driver and I've added / fixed reading of 
the voltages, There I'm doing exactly what you describe above, return a sysfs 
reading between 0 and 2.048 volts, and then let libsensors scale it depending 
on the attached divider circuit.

I still need to merge the patch for true fintek / msi board support and then 
I'll post an initial version.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Mark M. Hoffman wrote:
> Hi everyone:
> 
> Finally, back to the thread that resulted in change of maintainers, but not
> yet any merged code. ;)
> 
> * Hans de Goede <j.w.r.degoede@hhs.nl> [2007-04-11 15:49:08 +0200]:
>> Jean Delvare wrote:
>>> Hans,
>>>
>>> On Mon, 09 Apr 2007 14:26:06 +0200, Hans de Goede wrote:
>>>> Jean Delvare wrote:
>>>>> This is absolutely not different from all other hardware monitoring
>>>>> drivers. And all other drivers handle it in user-space, because that's
>>>>> the right design. I see no valid reason why it would be different for
>>>>> your abituguru3 driver. All you need is one configuration file per
>>>>> motherboard.
>>>> Actually it is _very_ different. The uguru is not a chip, its more a piece of 
>>>> software (looking from the driver POV) then a chip, thus sensors which aren't 
>>>> there really aren't there, they are not just unconnected pins, they _really_ 
>>>> aren't there.
> 
> There is another way to look at it: the uguru3 is not a single chip, it is a
> *family* of chips.  With that model, we would end up with a compromise between
> your positions that has precedent in other drivers.
> 
> I.e. Append the motherboard ID to the chip name, and also use that to create
> the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
> because you can put a section in sensors.conf for each variant of uguru3.
> 
> It's not that I'm against the label files per se.  But IMHO this suggestion
> is the closest fit to the existing model of operation.
> 

I could do that, actually making the ID available in some way to userspace is a 
good idea, however I'm still in favor of the fooX_label approach because:

1) The driver will need the table it currently has no matter what, to determine
    which inputs the model / this specific member of the family has, and at
    which addresses these inputs reside and what type they have.

    Since the table is already there, I might as well at the labels there and
    have all info in a single place = less maintainance / no sync problems

2) With dyn chip support in place (soon hopefully) a newer kernel with support
    for new motherboards added to the table, will just work without requiring
    userspace updates. When the labels are in sensors.conf userspace will need
    updating too, and if sensors.conf has been edited, it will require manual
    intervention / editing  even.

    More in general if other drivers, for example CPU / chipset internal
    sensor temp drivers get added, they can have foo#_label sysfs entries too,
    and the new dyn chip support libsensors will do the right thing.

    Atleast Fedora, and probably other distro's update the kernel way more often
    then userspace tools.

>> So is this one, its reading the configuration register which tells the driver 
>> which model/stepping we are running on.
> 
> Yep, this leads directly to my suggestion above.  Guys, is that acceptable?
> (/me is out, but leaving the rest uncut since it's such an old thread)
> 

If the above solution is what it takes to gets the driver integrated, then 
maybe I can agree, but I see big pain (mainly for me) in having to maintain the 
needed table partly in the driver and partly in sensors.conf, since the driver 
will need a table anyways, why not put all the info in one place?

---

I've feeling we're arguing about 2 different things at once here, so here is a 
try to split them:

1) do we want foo#_label sysfs entries for devices were we can give a sensible
    label to userspace, like cpu temp drivers
2) if we agree that we want / will allow 1), is it ok for the uguru3 driver to
    generate foo#_label sysfs entries using the contents of an uguru3 register
    which addresses a lookup table with the actual info.


Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Mark M. Hoffman]

Hi Hans:

* Hans de Goede <j.w.r.degoede@hhs.nl> [2007-06-16 21:50:20 +0200]:
> Mark M. Hoffman wrote:
> >Hi everyone:
> >
> >Finally, back to the thread that resulted in change of maintainers, but not
> >yet any merged code. ;)
> >
> >* Hans de Goede <j.w.r.degoede@hhs.nl> [2007-04-11 15:49:08 +0200]:
> >>Jean Delvare wrote:
> >>>Hans,
> >>>
> >>>On Mon, 09 Apr 2007 14:26:06 +0200, Hans de Goede wrote:
> >>>>Jean Delvare wrote:
> >>>>>This is absolutely not different from all other hardware monitoring
> >>>>>drivers. And all other drivers handle it in user-space, because that's
> >>>>>the right design. I see no valid reason why it would be different for
> >>>>>your abituguru3 driver. All you need is one configuration file per
> >>>>>motherboard.
> >>>>Actually it is _very_ different. The uguru is not a chip, its more a 
> >>>>piece of software (looking from the driver POV) then a chip, thus 
> >>>>sensors which aren't there really aren't there, they are not just 
> >>>>unconnected pins, they _really_ aren't there.
> >
> >There is another way to look at it: the uguru3 is not a single chip, it is 
> >a
> >*family* of chips.  With that model, we would end up with a compromise 
> >between
> >your positions that has precedent in other drivers.
> >
> >I.e. Append the motherboard ID to the chip name, and also use that to 
> >create
> >the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
> >because you can put a section in sensors.conf for each variant of uguru3.
> >
> >It's not that I'm against the label files per se.  But IMHO this suggestion
> >is the closest fit to the existing model of operation.
> >
> 
> I could do that, actually making the ID available in some way to userspace 
> is a good idea, however I'm still in favor of the fooX_label approach 
> because:
> 
> 1) The driver will need the table it currently has no matter what, to 
> determine
>    which inputs the model / this specific member of the family has, and at
>    which addresses these inputs reside and what type they have.
> 
>    Since the table is already there, I might as well at the labels there and
>    have all info in a single place = less maintainance / no sync problems

I anticipated that argument, and I can't disagree.  I only wanted to propose
something that was perhaps less controversial.

> 2) With dyn chip support in place (soon hopefully) a newer kernel with 
> support
>    for new motherboards added to the table, will just work without requiring
>    userspace updates. When the labels are in sensors.conf userspace will 
>    need
>    updating too, and if sensors.conf has been edited, it will require manual
>    intervention / editing  even.
> 
>    More in general if other drivers, for example CPU / chipset internal
>    sensor temp drivers get added, they can have foo#_label sysfs entries 
>    too,
>    and the new dyn chip support libsensors will do the right thing.
> 
>    Atleast Fedora, and probably other distro's update the kernel way more 
>    often
>    then userspace tools.
> 
> >>So is this one, its reading the configuration register which tells the 
> >>driver which model/stepping we are running on.
> >
> >Yep, this leads directly to my suggestion above.  Guys, is that acceptable?
> >(/me is out, but leaving the rest uncut since it's such an old thread)
> >
> 
> If the above solution is what it takes to gets the driver integrated, then 
> maybe I can agree, but I see big pain (mainly for me) in having to maintain 
> the needed table partly in the driver and partly in sensors.conf, since the 
> driver will need a table anyways, why not put all the info in one place?
> 
> ---
> 
> I've feeling we're arguing about 2 different things at once here, so here 
> is a try to split them:
> 
> 1) do we want foo#_label sysfs entries for devices were we can give a 
> sensible
>    label to userspace, like cpu temp drivers
> 2) if we agree that we want / will allow 1), is it ok for the uguru3 driver 
> to
>    generate foo#_label sysfs entries using the contents of an uguru3 
>    register
>    which addresses a lookup table with the actual info.

Without allowing #2, I would say #1 is pointless...

But I've heard enough: IMHO the answers are yes and yes.  Would you please
respin your patch against v2.6.22-rc4 and I'll look at it as soon as I can.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Mark,

On Sat, 16 Jun 2007 11:14:36 -0400, Mark M. Hoffman wrote:
> Finally, back to the thread that resulted in change of maintainers, but not
> yet any merged code. ;)

Naah, it was a _coincidence_.

> There is another way to look at it: the uguru3 is not a single chip, it is a
> *family* of chips.  With that model, we would end up with a compromise between
> your positions that has precedent in other drivers.

I can't really think of any precedent in other drivers.

> I.e. Append the motherboard ID to the chip name, and also use that to create
> the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
> because you can put a section in sensors.conf for each variant of uguru3.
> 
> It's not that I'm against the label files per se.  But IMHO this suggestion
> is the closest fit to the existing model of operation.

You're speaking as if the motherboard config file database and its
sensors-detect integration are never going to happen. I certainly hope
they will, and this makes your point pretty moot IMHO. Ultimately, we
want a configuration file for every motherboard, and we want it to be
selected automatically based on DMI information. With this in place,
appending the ID to the chip name doesn't buy us anything.

The idea of a sensors.conf.eg file with all chip configurations in it
comes from 1999 or so, we've all seen how broken it is nowadays (and
this has nothing to do with the abituguru3 driver.) Designing drivers
today based on a poor design decision taken 8 years ago doesn't sound
particularly smart.

So, no, I don't think that appending the ID to the device name makes
much sense, sorry.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hi Mark,
> 
> You're speaking as if the motherboard config file database and its
> sensors-detect integration are never going to happen. I certainly hope
> they will, and this makes your point pretty moot IMHO. Ultimately, we
> want a configuration file for every motherboard, and we want it to be
> selected automatically based on DMI information. With this in place,
> appending the ID to the chip name doesn't buy us anything.
> 

Actually my students have posted a working version of this already. What we 
really need right now is for someone to take a good look at that!

Especially at the sensors-detect modifications. I'm afraid I'm not much use 
there as I don't speak much perl.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Hans,

On Sat, 16 Jun 2007 21:50:20 +0200, Hans de Goede wrote:
> Mark M. Hoffman wrote:
> > There is another way to look at it: the uguru3 is not a single chip, it is a
> > *family* of chips.  With that model, we would end up with a compromise between
> > your positions that has precedent in other drivers.
> > 
> > I.e. Append the motherboard ID to the chip name, and also use that to create
> > the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
> > because you can put a section in sensors.conf for each variant of uguru3.
> > 
> > It's not that I'm against the label files per se.  But IMHO this suggestion
> > is the closest fit to the existing model of operation.
> 
> I could do that, actually making the ID available in some way to userspace is a 
> good idea, however I'm still in favor of the fooX_label approach because:
> 
> 1) The driver will need the table it currently has no matter what, to determine
>     which inputs the model / this specific member of the family has, and at
>     which addresses these inputs reside and what type they have.

Agreed to some extent, but see below.

>     Since the table is already there, I might as well at the labels there and
>     have all info in a single place = less maintainance / no sync problems

Not agreed. A few posts before in this thread, you were writing: "its
not like Abit does a new motherboard every month", to explain that it
wasn't a problem that adding support required patching the kernel. And
now you explain that putting everything in the kernel (rather than
having a part in user-space) is better for exactly the opposite reason.
That's not fair.

Quite frankly, once the user has gone through the pain of upgrading
his/her kernel, I doubt that changing a user-space configuration file
will frighten him/her much.

> 2) With dyn chip support in place (soon hopefully) a newer kernel with support
>     for new motherboards added to the table, will just work without requiring
>     userspace updates. When the labels are in sensors.conf userspace will need
>     updating too, and if sensors.conf has been edited, it will require manual
>     intervention / editing  even.
> 
>     More in general if other drivers, for example CPU / chipset internal
>     sensor temp drivers get added, they can have foo#_label sysfs entries too,
>     and the new dyn chip support libsensors will do the right thing.
> 
>     Atleast Fedora, and probably other distro's update the kernel way more often
>     then userspace tools.

Even if the labels and scaling are done in user-space, the user-space
tools will not need to be upgraded to support a new motherboard! All
that will be needed is a configuration file for that new motherboard -
as is the case for every other motherboard out there. In the future,
this will be automated, but even today, downloading a configuration file
from the lm-sensors.org wiki is a trivial task (again, especially
compared to patching and recompiling your kernel.)

So I just can't see any major problem with having the labels and
scaling in configuration files. As far as I can see, these
configurations file could even be generated automatically from
abit's .ini file, so the maintenance burden would be next to zero.

> If the above solution is what it takes to gets the driver integrated, then 
> maybe I can agree, 

Mark's proposal (append ID to device name) isn't needed if we have
everything in the driver, and it's not needed if we have a part in the
driver and the rest in user-space. So it's not going to help us decide
ourselves either way.

>                     but I see big pain (mainly for me) in having to maintain the 
> needed table partly in the driver and partly in sensors.conf, since the driver 
> will need a table anyways, why not put all the info in one place?

First of all, the current version of libsensors doesn't support
driver-provided labels yet. So your proposal implies that we implement
this first, otherwise your driver will not work properly.

One reason (not to put everything in the driver) is to keep the driver
small. Currently, the description table in your abituguru3 driver
accounts for more than 55% of the total driver size [2]. Most of the
size is for the labels and scaling factors as far as I can see.

Also, sysfs files have a cost. I find it funny that you were arguing
against individual alarm files a few months ago because you didn't want
too many sysfs files to be created, and now you want to create a label
file for every input.

Lastly, note that users will need a configuration file anyway, if they
want to set limits, or ignore some input, or change labels because the
driver-provided ones don't please them.

> I've feeling we're arguing about 2 different things at once here, so here is a 
> try to split them:
> 
> 1) do we want foo#_label sysfs entries for devices were we can give a sensible
>     label to userspace, like cpu temp drivers

Note that my original intention with foo#_label sysfs entries was
slightly different. My plan was to use them only when the driver can
give a sensible label _and_ user-space can't. In the case of the k8temp
driver, for example, we decided _not_ to create label files, because
these labels would have been always the same, so user-space can do it
just as well.

The use case I had in mind was the IT8716F chip, where two of the input
voltage channels can be routed to either regular Vin pins, or to the
chip's own power supply pins. In the latter case, the driver knows the
labels, but user-space doesn't. Right now the information is printed in
the logs, but I thought that label files (with libsensors support)
would make it more convenient for the users. But I'm not even sure any
longer, after all we still need a configuration file for each
motherboard anyway, so we could just pick such extra information from
the logs when we write the configuration file.

> 2) if we agree that we want / will allow 1), is it ok for the uguru3 driver to
>     generate foo#_label sysfs entries using the contents of an uguru3 register
>     which addresses a lookup table with the actual info.

At this point I decided to take a look at the actual data table in your
driver. I noticed the following things (please correct me if I
overlooked something):
* Ports 0 to 15, when used, are always used for voltages.
* Ports 16 to 23 are never used.
* Ports 24 to 31, when used, are always used for temperatures.
* Ports 32 to 39, when used, are always used for fans.
* Scaling factors for temperatures are always "1, 1, 0", i.e. no
  scaling.
* Scaling factors for fans are always "60, 1, 0", i.e. scaling from
  per-second to per-minute.

This means that the only valuable information in your table is:
* Which ports are in use.
* The labels.
* The scaling factors for voltages (and even then I see that the offset
  value is always 0).

If we admit that the labels and voltage scaling factors could live in
user-space as well, this leaves us with a single information for each
motherboard: the ports in use, which is basically a 40 bit field. Add
16 bits for the device ID, round up to 8 bytes due to possible alignment
constraints, multiply by the 15 motherboards supported by your driver,
this means that the whole table would fit in 120 bytes. Compare this to
the 13 kB or so that your current table is taking, that's a huge
difference.

Another advantage if a motherboard record is just a 16-bit ID and a
couple bit fields, is that these can be easily provided as module
parameters. This means that a user has an alternative to kernel
recompilation if his/her brand new Abit motherboard is not yet
supported by the abituguru3 driver in his/her kernel. I believe that
this is a significant benefit compared to the current implementation,
as far as user experience is concerned.

So, as a summary: I don't think that moving the labels and the voltage
scaling factors to user-space is a problem for maintenance, and in fact
I believe that it would make your driver more simple, and and more
user-friendly.

Now, I voluntarily put myself in a situation where I can't prevent you
from doing things the way you like. This means that you can just ignore
what I said if you think I'm wrong. And I'm not going to insist either
- it's not like I care that much. I am probably not going to ever use
your driver myself anyway, as I don't have Abit motherboards.


[1] Measured on x86_64. I compiled your driver and ran size:
$ size drivers/hwmon/abituguru3.o
   text    data     bss     dec     hex filename
  22400     414      16   22830    592e drivers/hwmon/abituguru3.o
Then I commented out all the entries but one (to prevent the compiler
from optimizing things away):
$ size drivers/hwmon/abituguru3.o
   text    data     bss     dec     hex filename
   9695     414      16   10125    278d drivers/hwmon/abituguru3.o

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Mark,

On Sat, 16 Jun 2007 11:42:13 -0400, Mark M. Hoffman wrote:
> A chip like uguru breaks this model, because we have *no* *idea* what it might
> be measuring at the *pin*.  But it turns out, it doesn't matter.

I don't think I agree with this statement. There are not dozens of ways
to measure a voltage. The chip must have an ADC. The table in Hans'
driver (derived from the .ini file provided by Abit) suggests a 10 mV
LSB. Assuming an 8-bit ADC, this means a range from 0 to 2.55V, and the
other scaling factors seem to confirm this (*3 for 5V, *6 for 12V.) If
we know which chip Abit is using (still a W83L950D as for the previous
µGurus?) we might even be able to verify this.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sat, 16 Jun 2007 21:50:20 +0200, Hans de Goede wrote:
>> Mark M. Hoffman wrote:
>>> There is another way to look at it: the uguru3 is not a single chip, it is a
>>> *family* of chips.  With that model, we would end up with a compromise between
>>> your positions that has precedent in other drivers.
>>>
>>> I.e. Append the motherboard ID to the chip name, and also use that to create
>>> the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
>>> because you can put a section in sensors.conf for each variant of uguru3.
>>>
>>> It's not that I'm against the label files per se.  But IMHO this suggestion
>>> is the closest fit to the existing model of operation.
>> I could do that, actually making the ID available in some way to userspace is a 
>> good idea, however I'm still in favor of the fooX_label approach because:
>>
>> 1) The driver will need the table it currently has no matter what, to determine
>>     which inputs the model / this specific member of the family has, and at
>>     which addresses these inputs reside and what type they have.
> 
> Agreed to some extent, but see below.
> 
>>     Since the table is already there, I might as well at the labels there and
>>     have all info in a single place = less maintainance / no sync problems
> 
> Not agreed. A few posts before in this thread, you were writing: "its
> not like Abit does a new motherboard every month", to explain that it
> wasn't a problem that adding support required patching the kernel. And
> now you explain that putting everything in the kernel (rather than
> having a part in user-space) is better for exactly the opposite reason.
> That's not fair.
> 
> Quite frankly, once the user has gone through the pain of upgrading
> his/her kernel, I doubt that changing a user-space configuration file
> will frighten him/her much.
>

However most user don't go through this pain themselves, the just get a kernel 
update from the distro, atleast one distro I know of (Fedora) syncs its kernel 
to upstream quite regulary, even for the stable release.

This whole argument is about making things "just work" (tm), with the table in 
the driver, dynchip support + generic chip printing, when a user gets a new 
kernel (through yum update) which comes with support for this motherboard, then 
things will "just work" and yes I plan to add support for foo#_label to the 
libsensors 3.0.0 code.

You agree below that the table is needed, but can be made smaller. However if 
the table is needed, then things automatically will not work for a newer 
motherboard without a new kernel. Thus lets make things so that only a newer 
kernel is needed and nothing else. Also it is very important to me to keep all 
info in one place as this is way easier to maintain. So if we need a table lets 
put everything in there.

>>                     but I see big pain (mainly for me) in having to maintain the 
>> needed table partly in the driver and partly in sensors.conf, since the driver 
>> will need a table anyways, why not put all the info in one place?
> 
> First of all, the current version of libsensors doesn't support
> driver-provided labels yet. So your proposal implies that we implement
> this first, otherwise your driver will not work properly.
>

Yes I plan on implementing this in the 3.x tree.

> One reason (not to put everything in the driver) is to keep the driver
> small. Currently, the description table in your abituguru3 driver
> accounts for more than 55% of the total driver size [2]. Most of the
> size is for the labels and scaling factors as far as I can see.
> 

1) Abit motherboards are not used in embedded setups so 13K of code is
    neglectible.
2) If this really is seen as an issue, all of the table, except for the entry
    of the detected motherboard can be freed after init.

> Also, sysfs files have a cost. I find it funny that you were arguing
> against individual alarm files a few months ago because you didn't want
> too many sysfs files to be created, and now you want to create a label
> file for every input.
> 

Well you yourself have argued / shown that the price of sysfs files is small, 
so I say this argument is mute.

> Lastly, note that users will need a configuration file anyway, if they
> want to set limits, or ignore some input, or change labels because the
> driver-provided ones don't please them.
> 

Yes, power users may need one, more power to them, in the mean time lets try to 
make things easier / "just work" for normal users.

>> I've feeling we're arguing about 2 different things at once here, so here is a 
>> try to split them:
>>
>> 1) do we want foo#_label sysfs entries for devices were we can give a sensible
>>     label to userspace, like cpu temp drivers
> 
> Note that my original intention with foo#_label sysfs entries was
> slightly different. My plan was to use them only when the driver can
> give a sensible label _and_ user-space can't. In the case of the k8temp
> driver, for example, we decided _not_ to create label files, because
> these labels would have been always the same, so user-space can do it
> just as well.
>

Actually I think the k8temp driver should provide tempX_label,
what if a dual core processor with only one tempsensor per core shows up?

Does the sensor in the second core then gets called temp3, thus temp2 gets 
skipped to match the labels currently in sensors.conf? What about quad core's?

Howto differentitatie between multiple core's in one die, and multiple sockets?

Also the current k8temp setup is broken, as with 2.10.4 with the default 
sensors.conf it will print 3 missing temp errors in gkrellm as libsensors says 
there are 4 temp sensors, thus one needs to add ignore lines for 3. With 
dynchip support this problem will no longer exist, and when combined with 
foo#_label files and PCI-id based k8temp loading (already happening) this will 
make the end user experience truely plug and play, which is exactly what we want.

---

Yet another problem, is that I can currently add support for new abituguru3 
motherboards as soon as abit releases a new windows uguru software supporting 
them. With your approach, to get plug and play support dmi based config will 
have to be integrated (planned) and then I need to wait for a user to contact 
me with the dmi strings for this new motherboard, only then can I get a new 
mobo truely supported. So the chain changes from:

-New mobo release
-Download windows software, add info to kernel driver table
-As soon as user gets new kernel through distro everything will work

to:

-New mobo release
-Download windows software, add info to kernel driver table
-Wait for user to start complaining that things don't work
-Ask dmi strings
-Add dmi strings + hand written config file to dmi - configfile database
-User must get both new kernel and new config-file database through distro,
  before things start to work.

Notice how the second scenario contains all the steps of the first + many more 
steps.

---

To be honest I'm getting somewhat sick about this discussion. Having all the 
info in the driver is just easier for both the end-user and the driver maintainer.

Your only argument OTOH is it doesn't fit with our current practices well I say 
<beep> our current practices. Our current practices are actually pretty broken 
in many aspects. For example the whole libsensors needing to know about chips 
instead of querying the driver for what the chip can do was a serious design 
mistake from day one.

You're acting as if I'm breaking the kernel/userspace ABI or something as holy 
as that, which I'm not even close too. All I'm trying is to make things just 
work for the end user, and really the easiest way todo that is to have all the 
needed info in one place. Fragmenting this info can only lead to problems where 
  sensors.conf and the driver get out of sync. For example what happens when 
the in kernel driver table skips the temp sensor with address 25 because that 
seemed right and later that turns out to be a bug? Then all tempX with X > 1 
will become temp(X+1), with all the info in the driver, things will still work 
fine, with things spit, all the labels but for temp1 will be wrong.

Scattering all this info around over many files is just wrong, just as wrong as 
  libsensors not using a query the chip for what it can do model from day one.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Mark M. Hoffman]

Hi Jean, Hans:

* Jean Delvare <khali@linux-fr.org> [2007-06-18 20:31:33 +0200]:
> Hi Mark,
> 
> On Sat, 16 Jun 2007 11:14:36 -0400, Mark M. Hoffman wrote:
> > Finally, back to the thread that resulted in change of maintainers, but not
> > yet any merged code. ;)
> 
> Naah, it was a _coincidence_.
> 
> > There is another way to look at it: the uguru3 is not a single chip, it is a
> > *family* of chips.  With that model, we would end up with a compromise between
> > your positions that has precedent in other drivers.
> 
> I can't really think of any precedent in other drivers.

lm85.c:

  40 I2C_CLIENT_INSMOD_6(lm85b, lm85c, adm1027, adt7463, emc6d100, emc6d102);

(...)

1251         if (!(kind = adt7463 && (data->vid & 0x80)))
1252                 if ((err = device_create_file(&new_client->dev,
1253                                         &dev_attr_in4_input))

> > I.e. Append the motherboard ID to the chip name, and also use that to create
> > the needed sysfs files.  Voila, you don't need "label" sysfs files anymore
> > because you can put a section in sensors.conf for each variant of uguru3.
> > 
> > It's not that I'm against the label files per se.  But IMHO this suggestion
> > is the closest fit to the existing model of operation.
> 
> You're speaking as if the motherboard config file database and its
> sensors-detect integration are never going to happen. I certainly hope
> they will, and this makes your point pretty moot IMHO. Ultimately, we
> want a configuration file for every motherboard, and we want it to be
> selected automatically based on DMI information. With this in place,
> appending the ID to the chip name doesn't buy us anything.
> 
> The idea of a sensors.conf.eg file with all chip configurations in it
> comes from 1999 or so, we've all seen how broken it is nowadays (and
> this has nothing to do with the abituguru3 driver.) Designing drivers
> today based on a poor design decision taken 8 years ago doesn't sound
> particularly smart.

Look at that strawman burn!  *Nobody* here is denying that libsensors needs an
upgrade.  It'd be nice if both of you stopped bitching about it.

> So, no, I don't think that appending the ID to the device name makes
> much sense, sorry.

I withdraw the proposal: what doesn't make sense is to maintain the info in
two different places.  That would result in the worst of both worlds.

Elsewhere in this thread... I agree with Jean that pulling a new config file
for a new board will get support to people w/ new boards more quickly.  Not
all distros update as frequently as Fedora.  This would require the driver
to go ahead and create all possible sysfs files in case it didn't recognzize
the board, and let userspace sort it out.  And then it is logical to wonder:
as long as we're already doing *that*, why put any tables in the driver at
all?

Nonetheless, I'm still OK w/ Hans' approach here, because ultimately he is
going to be the one maintaining it.  And really, kernel code is not written
on stone.  If it becomes clear that this approach sucks, we'll change it.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Mark,

On Tue, 19 Jun 2007 08:27:32 -0400, Mark M. Hoffman wrote:
> Elsewhere in this thread... I agree with Jean that pulling a new config file
> for a new board will get support to people w/ new boards more quickly.  Not
> all distros update as frequently as Fedora.  This would require the driver
> to go ahead and create all possible sysfs files in case it didn't recognzize
> the board, and let userspace sort it out.  (...)

Err, no, even me wouldn't like it. If we were to handle unknown
versions, my proposal was module parameters letting the user decide
which channels should be created. I never proposed to create all the
sysfs files.

> Nonetheless, I'm still OK w/ Hans' approach here, because ultimately he is
> going to be the one maintaining it.  And really, kernel code is not written
> on stone.  If it becomes clear that this approach sucks, we'll change it.

Fine with me.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] Proposal

[Jean Delvare]

Hi Hans,

Just answering a couple questions:

On Tue, 19 Jun 2007 10:31:01 +0200, Hans de Goede wrote:
> 2) If this really is seen as an issue, all of the table, except for the entry
>     of the detected motherboard can be freed after init.

Yes, I'd definitely like this to be done. Shouldn't be difficult.

> Actually I think the k8temp driver should provide tempX_label,
> what if a dual core processor with only one tempsensor per core shows up?
> 
> Does the sensor in the second core then gets called temp3, thus temp2 gets 
> skipped to match the labels currently in sensors.conf?

Yes. Note that all hardware monitoring drivers do this. Missing inputs
are just not in sysfs and don't cause other inputs to be renumbered.
This makes the code much cleaner. That's something I don't like in your
abituguru3 driver.

> What about quad core's?

I don't think we've seen any, and I don't think the k8temp driver
currently supports them.

> Howto differentitatie between multiple core's in one die, and multiple sockets?

My understanding is that there's one PCI device per socket, so you have
one k8temp device in the former case and two in the latter.

> Also the current k8temp setup is broken, as with 2.10.4 with the default 
> sensors.conf it will print 3 missing temp errors in gkrellm
> there are 4 temp sensors, thus one needs to add ignore lines for 3.

The k8temp driver does the right thing. gkrellm is broken, it shouldn't
print errors for missing inputs with 2.6 kernels. The problem is not
specific to the k8temp driver at all, BTW. The core problem is that
libsensors was designed for the old procfs interface, where drivers
couldn't omit an input even if they knew the chip didn't have it. The
kernel evolved and user-space is lagging behind.

> With dynchip support this problem will no longer exist,

Which is exactly why we implemented it...

> and when combined with 
> foo#_label files and PCI-id based k8temp loading (already happening) this will 
> make the end user experience truely plug and play, which is exactly what we want.

This will be the case even without foo#_label.

> Yet another problem, is that I can currently add support for new abituguru3 
> motherboards as soon as abit releases a new windows uguru software supporting 
> them. With your approach, to get plug and play support dmi based config will 
> have to be integrated (planned) and then I need to wait for a user to contact 
> me with the dmi strings for this new motherboard, only then can I get a new 
> mobo truely supported. So the chain changes from:
> 
> -New mobo release
> -Download windows software, add info to kernel driver table
> -As soon as user gets new kernel through distro everything will work
> 
> to:
> 
> -New mobo release
> -Download windows software, add info to kernel driver table
> -Wait for user to start complaining that things don't work
> -Ask dmi strings
> -Add dmi strings + hand written config file to dmi - configfile database
> -User must get both new kernel and new config-file database through distro,
>   before things start to work.
> 
> Notice how the second scenario contains all the steps of the first + many more 
> steps.

Good point. I thought that the configuration file from Abit also
included the motherboard names, so that it would be easy to write and
publish the new sensors.conf files yourself. If you need to wait for
user reports to find out which motherboards have the new IDs, then
indeed it's more work that I hoped it would be.

>                                                 For example what happens when 
> the in kernel driver table skips the temp sensor with address 25 because that 
> seemed right and later that turns out to be a bug? Then all tempX with X > 1 
> will become temp(X+1), with all the info in the driver, things will still work 
> fine, with things spit, all the labels but for temp1 will be wrong.

You driver shouldn't renumber inputs to start with.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors