lm_sensors2/prog/detect sensors-detect

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Add ACPI method for detecting IBM systems

Testing is wanted on this. Many cases can happen and I could only test a
few myself.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Added nForce2 detection, which was missing, although we have
>         nForce2 supported since 16 Feb 2003.

Note that I do not have a nForce2 at hand, so this is untested. I don't
know exactly what I was supposed to fill in the procid and match fields,
I hope it'll work as expected.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Update of /home/cvs/lm_sensors2/prog/detect
> In directory mordac.netroedge.com:/tmp/cvs-serv11020
> 
> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Now supports dmidecode 2.0 and later.

Notice that I didn't updated the included version of dmidecode. It is
still 1.8 and I plan to stick to 1.x. I simply made sure sensors-detect
would work with dmidecode 2.x so that people already having dmidecode on
their system (i.e., not from lm-sensors) have no trouble.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

I tried the latest CVS update of sensors detect.

With LANG=en_US.UTF8
It fails to detect the ITE IT8705F / IT8712F / SiS 950, but I don't get the warnings anymore.

with LANG=en_US
it does detect the IT8705F


Next adapter: SMBus Via Pro adapter at e800 (Non-I2C SMBus adapter)
Do you want to scan it? (YES/no/selectively):
Client found at address 0x37
Client found at address 0x52
Probing for `Serial EEPROM (SDRAM DIMM)'... Success!
    (confidence 8, driver `eeprom')
Client found at address 0x69

 Some chips are also accessible through the ISA bus. ISA probes are
 typically a bit more dangerous, as we have to write to I/O ports to do
 this.  Do you want to scan the ISA bus? (YES/no):
Probing for `National Semiconductor LM78'
  Trying address 0x0290... Failed!
Probing for `National Semiconductor LM78-J'
  Trying address 0x0290... Failed!
Probing for `National Semiconductor LM79'
  Trying address 0x0290... Failed!
Probing for `Winbond W83781D'
  Trying address 0x0290... Failed!
Probing for `Winbond W83782D'
  Trying address 0x0290... Failed!
Probing for `Winbond W83627HF'
  Trying address 0x0290... Failed!
Probing for `Winbond W83697HF'
  Trying address 0x0290... Failed!
Probing for `Silicon Integrated Systems SIS5595'
  Trying general detect... Failed!
Probing for `VIA Technologies VT82C686 Integrated Sensors'
  Trying general detect... Failed!
Probing for `VIA Technologies VT8231 Integrated Sensors'
  Trying general detect... Failed!
Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290... Failed!
Probing for `IPMI BMC KCS'
  Trying address 0x0ca0... Failed!
Probing for `IPMI BMC SMIC'
  Trying address 0x0ca8... Failed!



This is on Redhhat 9 perl with a patch.. as shown below...

Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.4.20-2.48smp, archname=i386-linux-thread-multi
    uname='linux str'
    config_args='-des -Doptimize=-O2 -march=i386 -mcpu=i686 -g 
-Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red 
Hat, Inc. -Dinstallprefix=/usr -Dprefix=/usr -Darchname=i386-linux 
-Dvendorprefix=/usr -Dsiteprefix=/usr 
-Dotherlibdirs=/usr/lib/perl5/5.8.0 -Duseshrplib -Dusethreads 
-Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db 
-Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio 
-Dinstallusrbinperl -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less 
-isr'
    hint=recommended, useposix=true, d_sigactionfifine
    usethreadsfifine use5005threads=undef'
 useithreadsfifine usemultiplicity    useperlio= d_sfio=undef uselargefilesfifine usesocks=undef
    use64bitint=undef use64bitall=un uselongdouble    usemymalloc=, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS 
-DDEBUGGING -fno-strict-aliasing -I/usr/local/include 
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITSd -I/usr/include/gdbm',
    optimize='',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING 
-fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='3.2.2 20030213 (Red Hat Linux 8.0 
3.2.2-1)', gccosandvers=''
gccversion='3.2.2 200302'
    intsize=e, longsize= , ptrsize=p, doublesize=8, byteorder\x1234
    d_longlongfifine, longlongsize=8, d_longdblfifine, longdblsize\x12
    ivtype='long'
k', ivsize=4'
ivtype='long'
known_ext, nvtype='double'
o_nonbl', nvsize=, Off_t='', lseeksize=8
    alignbytes=4, prototypefifine
  Linker and Libraries:
    ld='gcc'
l', ldflags =' -L/usr/local/lib'
ldf'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lpthread -lc -lcrypt -lutil
    perllibs    libc=/lib/libc-2.3.1.so, so=so, useshrplib=true, libperl=libper
    gnulibc_version='2.3.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so', d_dlsymun=undef, ccdlflags='-rdynamic 
-Wl,-rpath,/usr/lib/perl5/5.8.0/i386-linux-thread-multi/CORE'
    cccdlflags='-fPIC'
ccdlflags='-rdynamic -Wl,-rpath,/usr/lib/perl5', lddlflags='s 
Unicode/Normalize XS/A'


Characteristics of this binary (from libperl):
  Compile-time options: DEBUGGING MULTIPLICITY USE_ITHREADS 
USE_LARGE_FILES PERL_IMPLICIT_CONTEXT
  Locally applied patches:
        MAINT18379
  Built under linux
  Compiled at Feb 18 2003 22:19:53
  @INC:
    /usr/lib/perl5/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/5.8.0
    /usr/lib/perl5/site_perl/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/site_perl/5.8.0
    /usr/lib/perl5/site_perl
    /usr/lib/perl5/vendor_perl/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.8.0
    /usr/lib/perl5/vendor_perl
    /usr/lib/perl5/5.8.0/i386-linux-thread-multi
    /usr/lib/perl5/5.8.0
    .






On Wed, 11 Jun 2003 10:33:28 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> 
khali> > Modified Files:
khali> > 	sensors-detect 
khali> > Log Message:
khali> > (Khali) Undid outb() fix by Jim Morris.
khali> >         Fixed faulty outb() calls.
khali> >         Armored outb() against future faulty calls.
khali> 
khali> Please note that I in no way pretend the issue is solved. It is just my
khali> first step in trying to solve it.
khali> 
khali> Jim, could you please checkout the new sensors-detect and give it a try?
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

(quoting myself)

> OK, the next step will be to prepare a specially modified
> sensors-detect script with a lot of debugging info for your particular
> chipset. Then, I'll ask you to run it both with and without the UTF8
> locale, and we'll see where the hell the bug is lurking.

Get it there:
http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k1

Jim, please try with both non-UTF and UTF locales, and see if the
outputs differ.

PS: Yes, I know, I used the barbarian way of debugging. But I'm quite
confident it will work ;)

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

khali> > Probing for `ITE IT8705F / IT8712F / SiS 950'
khali> >   Trying address 0x0290... Failed!
khali> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I guess it's there?

Yes exactly, that says success and everything else proceeds as expected.

khali> What's more, your first
khali> proposal was to mask with 0x7f, not 0xff, which was not very logical.

No but it worked, and was the first attempt I made to get it to work.
Maybe the values being passed are wrong, or we are getting sign extension?

khali> Do we have any chance to know what this patch does? Not sure it as an
khali> interest though.

I believe it is a UTF patch, I googled for perl and UTF8 and found a few
references to this bug in other scripts, and there is reference to
Redhat using an unreleased patch.

On Thu, 12 Jun 2003 10:32:22 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> 
khali> > With LANG=en_US.UTF8
khali> > It fails to detect the ITE IT8705F / IT8712F / SiS 950, but I don't
khali> > get the warnings anymore.
khali> > 
khali> > with LANG=en_US
khali> > it does detect the IT8705F
khali> 
khali> That's almost what I had expected. I think that your issue and the
khali> warnings were mostly unrelated. I was able to reproduce the warning on a
khali> non-UTF system, and a friend of mine, who is using a Red Hat 9 system in
khali> "UTF mode", could run this version (well, Red Hat's one but the fix is
khali> almost the same) and had his chipset detected. What's more, your first
khali> proposal was to mask with 0x7f, not 0xff, which was not very logical. So
khali> I think there is a *second* bug hiding in there. Maybe it's only
khali> relative to your particular chipset, maybe not.
khali> 
khali> Where does the output differ when the chipset is successfully found?
khali> 
khali> > (...)
khali> > Probing for `VIA Technologies VT8231 Integrated Sensors'
khali> >   Trying general detect... Failed!
khali> > Probing for `ITE IT8705F / IT8712F / SiS 950'
khali> >   Trying address 0x0290... Failed!
khali> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I guess it's there?
khali> > Probing for `IPMI BMC KCS'
khali> >   Trying address 0x0ca0... Failed!
khali> > Probing for `IPMI BMC SMIC'
khali> >   Trying address 0x0ca8... Failed!
khali> 
khali> > This is on Redhhat 9 perl with a patch.. as shown below...
khali> 
khali> Do we have any chance to know what this patch does? Not sure it as an
khali> interest though.
khali> 
khali> OK, the next step will be to prepare a specially modified sensors-detect
khali> script with a lot of debugging info for your particular chipset. Then,
khali> I'll ask you to run it both with and without the UTF8 locale, and we'll
khali> see where the hell the bug is lurking.
khali> 
khali> The process could be drastically accelerated if you have a possibility
khali> to meet me on IRC (Khali@freenode).
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> With LANG=en_US.UTF8
> It fails to detect the ITE IT8705F / IT8712F / SiS 950, but I don't
> get the warnings anymore.
> 
> with LANG=en_US
> it does detect the IT8705F

That's almost what I had expected. I think that your issue and the
warnings were mostly unrelated. I was able to reproduce the warning on a
non-UTF system, and a friend of mine, who is using a Red Hat 9 system in
"UTF mode", could run this version (well, Red Hat's one but the fix is
almost the same) and had his chipset detected. What's more, your first
proposal was to mask with 0x7f, not 0xff, which was not very logical. So
I think there is a *second* bug hiding in there. Maybe it's only
relative to your particular chipset, maybe not.

Where does the output differ when the chipset is successfully found?

> (...)
> Probing for `VIA Technologies VT8231 Integrated Sensors'
>   Trying general detect... Failed!
> Probing for `ITE IT8705F / IT8712F / SiS 950'
>   Trying address 0x0290... Failed!
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I guess it's there?
> Probing for `IPMI BMC KCS'
>   Trying address 0x0ca0... Failed!
> Probing for `IPMI BMC SMIC'
>   Trying address 0x0ca8... Failed!

> This is on Redhhat 9 perl with a patch.. as shown below...

Do we have any chance to know what this patch does? Not sure it as an
interest though.

OK, the next step will be to prepare a specially modified sensors-detect
script with a lot of debugging info for your particular chipset. Then,
I'll ask you to run it both with and without the UTF8 locale, and we'll
see where the hell the bug is lurking.

The process could be drastically accelerated if you have a possibility
to meet me on IRC (Khali@freenode).

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Undid outb() fix by Jim Morris.
>         Fixed faulty outb() calls.
>         Armored outb() against future faulty calls.

Please note that I in no way pretend the issue is solved. It is just my
first step in trying to solve it.

Jim, could you please checkout the new sensors-detect and give it a try?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> No but it worked, and was the first attempt I made to get it to work.
> Maybe the values being passed are wrong, or we are getting sign
> extension?

I suspect the value is wrong, and masking reverted back to the good
value. Now we have to understand where the value goes wrong, possibly
why, and find the better fix we can.

> I believe it is a UTF patch, I googled for perl and UTF8 and found a
> few references to this bug in other scripts, and there is reference to
> Redhat using an unreleased patch.

Patch 18379 is not related to UTF. BTW, there are much much more than
this only patch in Red Hat's Perl. There are at least 50, three quarters
of them being taken from Perl's CVS (so one quarter is Red Hat's own set
of patches). I really wonder why there is only one patch showing in your
output. There are 28 patches containing the "UTF" string, so it may be
hard finding which one is causing the problem. And we are not even sure
any patch is faulty.

Some questions now:

1* I'd like to make sure you and my friend are using the same version of
Perl. He is using perl(2:5.8.0-88).i386 (Red Hat 9). Which version are
you using?

2* How did you get the output you attached in your last mail?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

I'll get this done over the weekend... sorry for the delay.

On Thu, 12 Jun 2003 13:33:03 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> (quoting myself)
khali> 
khali> > OK, the next step will be to prepare a specially modified
khali> > sensors-detect script with a lot of debugging info for your particular
khali> > chipset. Then, I'll ask you to run it both with and without the UTF8
khali> > locale, and we'll see where the hell the bug is lurking.
khali> 
khali> Get it there:
khali> http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k1
khali> 
khali> Jim, please try with both non-UTF and UTF locales, and see if the
khali> outputs differ.
khali> 
khali> PS: Yes, I know, I used the barbarian way of debugging. But I'm quite
khali> confident it will work ;)
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

Sorry for the delay, I got busy with some other work...

Ok they are different I attach both files... but here are the juicy bits...

With UTF on..
Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
ite_isa_detect called with chip = 0 and addr = 656
val = 144
val = 88
calling outb with addr = 661 and val = 167
In the if block
Failed!

With UTF off...
Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
ite_isa_detect called with chip = 0 and addr = 656
val = 255
val = 79
calling outb with addr = 661 and val = 176
after return #1
after return #2
Success!
    (confidence 8, driver `it87')

On Thu, 12 Jun 2003 13:33:03 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> (quoting myself)
khali> 
khali> > OK, the next step will be to prepare a specially modified
khali> > sensors-detect script with a lot of debugging info for your particular
khali> > chipset. Then, I'll ask you to run it both with and without the UTF8
khali> > locale, and we'll see where the hell the bug is lurking.
khali> 
khali> Get it there:
khali> http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k1
khali> 
khali> Jim, please try with both non-UTF and UTF locales, and see if the
khali> outputs differ.
khali> 
khali> PS: Yes, I know, I used the barbarian way of debugging. But I'm quite
khali> confident it will work ;)
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: try2.utf
Type: application/octet-stream
Size: 4644 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030622/488d9d10/try2.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: try1.nonutf
Type: application/octet-stream
Size: 4817 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030622/488d9d10/try1.obj

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> > Could be... But there is one last thing I would like to try. My
> > changes(-k3) don't seem to have had any effect, so I'm trying
> > something similar but different. Please get
> >   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k4
> > and tell me, again, it it behaves differently with and without an
> > UTF-8 locale.
> 
> That seemed to fix it!!!

"Ah ah ah (evil laugh) you nasty little bug, you thought you could lurk
forever? Diiiiiiiie!"

Ahhhh I feel better :) For the ones interested, the solution was to call
binmode(IOPORTS, ':raw') after opening '/dev/ports'. The ':raw' keyword,
among other things, prevents Perl from using the UTF-8 layer on file I/O
(where available). I did not find this first because I don't have Perl
5.8.0 on my development machine, and this layers thing is new in 5.8.0.
I was told on IRC to have a look at the binmode perldoc page on Perl
5.8.0, which I did, and it led me to the solution.

Ok Jim, thanks a lot for your help again. I'll commit a clean fix to
sensors-detect to CVS today.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

Ok here are the two results snipped for ITE detection....

On Wed, 25 Jun 2003 14:45:38 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> 
khali> > Sorry for the delay, I got busy with some other work...
khali> 
khali> Same here as you can see.
khali> 
khali> > Ok they are different I attach both files... but here are the juicy
khali> > bits...
khali> > 
khali> > With UTF on..
khali> > Probing for `ITE IT8705F / IT8712F / SiS 950'
khali> >   Trying address 0x0290...
khali> > ite_isa_detect called with chip = 0 and addr = 656
khali> > val = 144
khali> > val = 88
khali> > calling outb with addr = 661 and val = 167
khali> > In the if block
khali> > Failed!
khali> > 
khali> > With UTF off...
khali> > Probing for `ITE IT8705F / IT8712F / SiS 950'
khali> >   Trying address 0x0290...
khali> > ite_isa_detect called with chip = 0 and addr = 656
khali> > val = 255
khali> > val = 79
khali> > calling outb with addr = 661 and val = 176
khali> > after return #1
khali> > after return #2
khali> > Success!
khali> >     (confidence 8, driver `it87')
khali> 
khali> All right. I think that everyone will now agree that the problem is
khali> *not* in outb but in *inb*.
khali> 
khali> Time to try another set of prints. Please grab the new sensors-detect
khali> at:
khali>   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k2
khali> and give it a try. As usual, all the interest is to compare the output
khali> between a UTF and a non-UTF locale.
khali> 
khali> It will probably generate more noise than the previous one did. I expect
khali> to see that on your system, ord($X) and unpack("C", $X) give different
khali> results (while I think both functions should return the same value in
khali> all cases, on all systems).
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: probe2.utf
Type: application/octet-stream
Size: 1241 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030626/546eee79/probe2.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: probe2.noutf
Type: application/octet-stream
Size: 967 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030626/546eee79/probe2-0001.obj

lm_sensors2/prog/detect sensors-detect

[Mark D. Studebaker]

With your fix and perl 5.005.02, at the beginning I get:

Useless use of a constant in void context at prog/detect/sensors-detect line 1345.

which doesn't s top the script, but later on (just after saying YES to ISA bus scan)

Your vendor has not defined Fcntl macro O_BINARY, used at prog/detect/sensors-detect line 1343.

and the script stops. So how do we get this to work on older perls?


Jean Delvare wrote:
>>>Could be... But there is one last thing I would like to try. My
>>>changes(-k3) don't seem to have had any effect, so I'm trying
>>>something similar but different. Please get
>>>  http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k4
>>>and tell me, again, it it behaves differently with and without an
>>>UTF-8 locale.
>>
>>That seemed to fix it!!!
> 
> 
> "Ah ah ah (evil laugh) you nasty little bug, you thought you could lurk
> forever? Diiiiiiiie!"
> 
> Ahhhh I feel better :) For the ones interested, the solution was to call
> binmode(IOPORTS, ':raw') after opening '/dev/ports'. The ':raw' keyword,
> among other things, prevents Perl from using the UTF-8 layer on file I/O
> (where available). I did not find this first because I don't have Perl
> 5.8.0 on my development machine, and this layers thing is new in 5.8.0.
> I was told on IRC to have a look at the binmode perldoc page on Perl
> 5.8.0, which I did, and it led me to the solution.
> 
> Ok Jim, thanks a lot for your help again. I'll commit a clean fix to
> sensors-detect to CVS today.
> 

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

Yes, I'll run more tests. I'll get this one done today.

On Fri, 27 Jun 2003 09:40:21 +0200
Jean Delvare <khali@linux-fr.org> wrote:

khali> 
khali> > Ok here are the two results snipped for ITE detection....
khali> 
khali> I guess there were similar errors (Malformed UTF-8 character) for some
khali> other chipsets?
khali> 
khali> Anyway, I'm stumped. I was expecting your system to return UTF-8 chars
khali> from /dev/ports, and it doesn't seem so (or why would perl say they are
khali> malformed?) And if it returns regular chars, why are the values
khali> different from the ones obtained with a non-UTF-8 locale?  I just don't
khali> understand.
khali> 
khali> So from here I only see two possibilities:
khali> 
khali> 1* Opening /dev/ports in binary mode solves the problem (which I doubt,
khali> since traditionally, Unix systems don't differenciate text and binary
khali> modes). Please grab the modified script that does this and tell me if it
khali> works:
khali>   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k3
khali> 
khali> 2* I just don't know what we can do. In this case, the only thing I can
khali> propose is checking for the LANG environment variable, and generate a
khali> warning at start is if matches *.UTF8, stating that the user should
khali> better restart the script with a non-UTF-8 locale. We can even change
khali> the LANG environment variable in the script, but I don't know if that
khali> would work (I don't know if Perl "reads" it at the beginning, once for
khali> all, or each time its value matters). Jim, would you accept doing some
khali> more tests for us?
khali> 
khali> Thanks.
khali> 
khali> -- 
khali> Jean Delvare
khali> http://www.ensicaen.ismra.fr/~delvare/
khali> 

--
Jim Morris morris@wolfman.com

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Sorry for the delay, I got busy with some other work...

Same here as you can see.

> Ok they are different I attach both files... but here are the juicy
> bits...
> 
> With UTF on..
> Probing for `ITE IT8705F / IT8712F / SiS 950'
>   Trying address 0x0290...
> ite_isa_detect called with chip = 0 and addr = 656
> val = 144
> val = 88
> calling outb with addr = 661 and val = 167
> In the if block
> Failed!
> 
> With UTF off...
> Probing for `ITE IT8705F / IT8712F / SiS 950'
>   Trying address 0x0290...
> ite_isa_detect called with chip = 0 and addr = 656
> val = 255
> val = 79
> calling outb with addr = 661 and val = 176
> after return #1
> after return #2
> Success!
>     (confidence 8, driver `it87')

All right. I think that everyone will now agree that the problem is
*not* in outb but in *inb*.

Time to try another set of prints. Please grab the new sensors-detect
at:
  http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k2
and give it a try. As usual, all the interest is to compare the output
between a UTF and a non-UTF locale.

It will probably generate more noise than the previous one did. I expect
to see that on your system, ord($X) and unpack("C", $X) give different
results (while I think both functions should return the same value in
all cases, on all systems).

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

On Friday 27 June 2003 02:22 pm, you wrote:
>
> Could be... But there is one last thing I would like to try. My changes
> (-k3) don't seem to have had any effect, so I'm trying something similar
> but different. Please get
>   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k4
> and tell me, again, it it behaves differently with and without an UTF-8
> locale.

That seemed to fix it!!!

without UTF8......

Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
Called inb with porte7
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte8
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte9
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf3
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ordy
Returning resy

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ord\x176
Returning res\x176

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x17
Returning res\x17

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x144
Returning res\x144
Success!
    (confidence 8, driver `it87')
Probing for `IPMI BMC KCS'
  Trying address 0x0ca0...
Called inb with port235
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5
Failed!

========================with UTF8
Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
Called inb with porte7
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte8
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte9
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf3
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ordy
Returning resy

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ord\x176
Returning res\x176

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x17
Returning res\x17

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x144
Returning res\x144
Success!
    (confidence 8, driver `it87')
Probing for `IPMI BMC KCS'
  Trying address 0x0ca0...
Called inb with port235
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5
Failed!

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> > Anyway, I'm stumped. I was expecting your system to return UTF-8
> > chars from /dev/ports, and it doesn't seem so (or why would perl say
> > they are malformed?) And if it returns regular chars, why are the
> > values different from the ones obtained with a non-UTF-8 locale?  I
> > just don't understand.
> 
> This could be a bug in Perl and their partial UTF8 hack

Could be... But there is one last thing I would like to try. My changes
(-k3) don't seem to have had any effect, so I'm trying something similar
but different. Please get:
  http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k4
and tell me, again, it it behaves differently with and without an UTF-8
locale.

Thanks a lot for your precious help.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> With your fix and perl 5.005.02, at the beginning I get:
> 
> Useless use of a constant in void context at
> prog/detect/sensors-detect line 1345.
> 
> which doesn't s top the script, but later on (just after saying YES to
> ISA bus scan)
> 
> Your vendor has not defined Fcntl macro O_BINARY, used at
> prog/detect/sensors-detect line 1343.
> 
> and the script stops. So how do we get this to work on older perls?

We can simply remove the O_BINARY. I let it there just in case, but the
real fix is binmode() two lines below. Binmode does more than O_BINARY
does, so it seems safe to remove the later. I was once told that
O_BINARY was not portable, now I know why ;)

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Ok here are the two results snipped for ITE detection....

I guess there were similar errors (Malformed UTF-8 character) for some
other chipsets?

Anyway, I'm stumped. I was expecting your system to return UTF-8 chars
from /dev/ports, and it doesn't seem so (or why would perl say they are
malformed?) And if it returns regular chars, why are the values
different from the ones obtained with a non-UTF-8 locale?  I just don't
understand.

So from here I only see two possibilities:

1* Opening /dev/ports in binary mode solves the problem (which I doubt,
since traditionally, Unix systems don't differenciate text and binary
modes). Please grab the modified script that does this and tell me if it
works:
  http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k3

2* I just don't know what we can do. In this case, the only thing I can
propose is checking for the LANG environment variable, and generate a
warning at start is if matches *.UTF8, stating that the user should
better restart the script with a non-UTF-8 locale. We can even change
the LANG environment variable in the script, but I don't know if that
would work (I don't know if Perl "reads" it at the beginning, once for
all, or each time its value matters). Jim, would you accept doing some
more tests for us?

Thanks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jim Morris]

On Friday 27 June 2003 12:40 am, you wrote:
>
> I guess there were similar errors (Malformed UTF-8 character) for some
> other chipsets?
>

yes for instance...

Probing for `IPMI BMC KCS'
  Trying address 0x0ca0...
Called inb with port235
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected non-continuation byte 0xff, immediately 
after start byte 0xff) in ord at sensors-detect2.pl line 1358.
Read value with ord=0
Returning res%5
Failed!

> Anyway, I'm stumped. I was expecting your system to return UTF-8 chars
> from /dev/ports, and it doesn't seem so (or why would perl say they are
> malformed?) And if it returns regular chars, why are the values
> different from the ones obtained with a non-UTF-8 locale?  I just don't
> understand.

This could be a bug in Perl and their partial UTF8 hack

>
> So from here I only see two possibilities:
>
> 1* Opening /dev/ports in binary mode solves the problem (which I doubt,
> since traditionally, Unix systems don't differenciate text and binary
> modes). Please grab the modified script that does this and tell me if it
> works:
>   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k3

With UTF8......
Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
Called inb with porte7
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected continuation byte 0x90, with no 
preceding start byte) in ord at sensors-detect-k3 line 1358.
Read value with ord=0
Returning res\x144

Called inb with porte8
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected continuation byte 0x90, with no 
preceding start byte) in ord at sensors-detect-k3 line 1358.
Read value with ord=0
Returning res\x144

Called inb with porte9
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected continuation byte 0x90, with no 
preceding start byte) in ord at sensors-detect-k3 line 1358.
Read value with ord=0
Returning res\x144

Called inb with portf3
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected continuation byte 0x90, with no 
preceding start byte) in ord at sensors-detect-k3 line 1358.
Read value with ord=0
Returning res\x144

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ord\x1552
Returning res!6

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ordf
Returning resf
Failed!
Probing for `IPMI BMC KCS'
  Trying address 0x0ca0...
Called inb with port235
After sysseek
After sysread, nrchars=1
Malformed UTF-8 character (unexpected non-continuation byte 0xff, immediately 
after start byte 0xff) in ord at sensors-detect-k3 line 1358.
Read value with ord=0
Returning res%5
Failed!

================================
Without UTF8

Probing for `ITE IT8705F / IT8712F / SiS 950'
  Trying address 0x0290...
Called inb with porte7
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte8
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with porte9
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf3
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ordy
Returning resy

Called inb with portf1
After sysseek
After sysread, nrchars=1
Read value with ord\x176
Returning res\x176

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x17
Returning res\x17

Called inb with portf2
After sysseek
After sysread, nrchars=1
Read value with ord\x144
Returning res\x144
Success!
    (confidence 8, driver `it87')
Probing for `IPMI BMC KCS'
  Trying address 0x0ca0...
Called inb with port235
After sysseek
After sysread, nrchars=1
Read value with ord%5
Returning res%5
Failed!


-- 
Jim Morris

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> > > Could be... But there is one last thing I would like to try. My
> > > changes(-k3) don't seem to have had any effect, so I'm trying
> > > something similar but different. Please get
> > >   http://www.ensicaen.ismra.fr/~delvare/sensors-detect-k4
> > > and tell me, again, it it behaves differently with and without an
> > > UTF-8 locale.
> > 
> > That seemed to fix it!!!
> 
> "Ah ah ah (evil laugh) you nasty little bug, you thought you could
> lurk forever? Diiiiiiiie!"

Jim, still around? Although the UTF-8 fix finally worked, it turned out
to be incompatible with older versions of Perl MDS is still using. I
think I know what to do but I'd like to make sure it doesn't make the
UTF-8 trouble come back again. This is very very unlikely to happen, but
who knows...

Please edit your local copy of sensors-detect, search for the "sub
initialize_ioports" and make that function looks like this:

sub initialize_ioports
{
  sysopen (IOPORTS, "/dev/port", O_RDWR)
    or die "/dev/port: $!\n";
  binmode (IOPORTS);
}

(If you still have the -k4 script, it will probably mean removing
O_BINARY and :raw. These are the things older Perl don't know about.)

Then let us know if the UTF-8 test still works, or not. Thanks!

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Detect which of modules.conf or conf.modules should be used.
>         Detect which of /dev/i2c-*, /dev/i2c/* or /dev/i2c*  should be
>         used.
>         Stop if no i2c device files exist.

I agree that one could run sensors-detect for the ISA and Super I/O
chips only. However, I think that forcing the user to create the devices
if they do not exist will be helpful in 99.99% of the cases. If someone
ever complains about that, we'll change it.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Add VPD method to detect IBM systems. In the long term, the
> other methods (DMI and ACPI) are expected to become unnecessary.

I implemented this according to the document Pam Huntley pointed out
several months ago:
  http://www.pc.ibm.com/qtechinfo/MIGR-45120.html

I should have done that times ago but never found the time to do so.
Some weeks ago I implemented it in C for integration to the dmidecode
project, and I converted the code to Perl today.

This detection method is expected to supersede the two other ones we had
until today (DMI and ACPI). Three significant advantages:
1* Detection will be more simple with a single method.
2* We won't rely on dmidecode anymore.
3* VPD is more precise and efficient since it's aimed at IBM systems.
   We'll be able to start a blacklist and/or a whitelist.

I'd appreciate some of you give a try to sensors-detect now. If the VPD
detection method if confirmed to work, I'll drop the other two methods
soon.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

cool.
you still don't know which systems have 24RF08's in them, though, do
you?

tested new sensors-detect on 4 machines.
2 w/ ACPI+DMI worked fine.
One with neither (iopener) gave the standard Thinkpad warning at the
beginning.
On my HP ia64 machine it didn't give any ACPI or DMI info but didn't
give
the Thinkpad warning either... what's up with that?

Jean Delvare wrote:
> 
> > Modified Files:
> >       sensors-detect
> > Log Message:
> > (Khali) Add VPD method to detect IBM systems. In the long term, the
> > other methods (DMI and ACPI) are expected to become unnecessary.
> 
> I implemented this according to the document Pam Huntley pointed out
> several months ago:
>   http://www.pc.ibm.com/qtechinfo/MIGR-45120.html
> 
> I should have done that times ago but never found the time to do so.
> Some weeks ago I implemented it in C for integration to the dmidecode
> project, and I converted the code to Perl today.
> 
> This detection method is expected to supersede the two other ones we had
> until today (DMI and ACPI). Three significant advantages:
> 1* Detection will be more simple with a single method.
> 2* We won't rely on dmidecode anymore.
> 3* VPD is more precise and efficient since it's aimed at IBM systems.
>    We'll be able to start a blacklist and/or a whitelist.
> 
> I'd appreciate some of you give a try to sensors-detect now. If the VPD
> detection method if confirmed to work, I'll drop the other two methods
> soon.
> 
> --
> Jean Delvare
> http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> tested new sensors-detect on 4 machines.
> 2 w/ ACPI+DMI worked fine.

Typical configuration. The output doesn't even mention VPD, right?

> One with neither (iopener) gave the standard Thinkpad warning at the
> beginning.

Should not happen. This could happen if /dev/mem is unreadable for some
reason. Could you download dmidecode CVS and give a try to biosdecode?
Then edit the Makefile, remove -DUSE_MMAP, compile again and retry.
Might crash. If it does, this means I should use mmap() to access
/dev/mem in sensors-detect - although I don't even know how to do that.

Anything special about that iopener? You once sent me a copy of the BIOS
area and there's nothing strange at first sight, except that it actually
doesn't support any of ACPI, SMBIOS nor VPD.

If biosdecode works OK on it (both with and without -DUSE_MMAP), please
try to track down why system_safeness_by_vpd in sensors-detect returns
0. There are three reasons it could (you'll see in the code). The third
one is the more likely to cause problems (I think).

> On my HP ia64 machine it didn't give any ACPI or DMI info but didn't
> give the Thinkpad warning either... what's up with that?

That's the expected behavior. This is why VPD supersedes the other
methods. If ACPI or DMI isn't present, you can't conclude, while VPD
being not there *does* mean that it's not an IBM system (with the
exception of one very specific Thinkpad model).

Thanks a lot for the feedback.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> never mind.
> I think, on the iopener, when sensors-detect gave the IBM warning,
> I was running as non-root.
> It doesn't give the warning when run as root.
> Sorry for the false alarm.

I prefer it that way, since I had some difficulties imagining what would
have cause such a failure in the detection code.

> But anyway, would it be helpful to put out a message in sensors-detect
> when it can't determine the system type, even as root?

Auxiliary question, should we even accept running as non-root? We can't
detect an IBM system if we are not root, still the user can scan any
already-loaded bus driver, providing i2c-dev is loaded too. Isn't it
dangerous? OTOH i2c-dev shouldn't be loaded on such a system, and
i2c-piix4 shouldn't possibly load anyway.

(BTW, should I replace DMI with VPD in i2c-piix4/dmi_scan? Or is it
considered a feature to use different protection systems? Not all
Thinkpads have a DMI table. OTOH, DMI scan is already handled by the
kernel so idealy we could get rid of dmi_scan, as it was done in
Linux 2.6. And after all, the eeprom module is supposed to be safe WRT
faulty eeproms now, so dmi/vpd detection at this level is not really
required anymore, right?)

To answer your question, failing to detect the system type now (with
VPD) can only occur if we can't read /dev/mem (because we are not root,
or because a read error occured, which could happen on some non-i386
architectures). Since all Thinkpads are i386 systems AFAIK, the second
case isn't really a problem.

And we already output a message if the system type couldn't be detected.
I'll just leave it in place.

> And yes, on the 2 "typical" machines, it doesn't mention VPD.

Which basically means "the system is safe", left apart the two cases
mentioned above.

> nice job on the dmidecode package and website BTW.

Thanks :) Do you really like the website. I'm not sure how to organize
it. For now, there's a single page with anything on it. I wonder if I
should split it into many pages, but actually there isn't that much to
write about it, so my conclusion so far was that it wasn't necessary.

As for the design, see how clean and easy it is to use CSS and XHTML to
get an easy-to-maintain site. Admittedly, I kept the look very simple,
half because I like simple and fast-loading pages as a user, half
because I am not particularily talented when it comes to complex design.

I'd love to see the lm_sensors website with a single CSS and clean HTML
pages too, unfortunately I don't really have the time to work on this -
and after all the current website does its job rather well, methinks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

never mind.
I think, on the iopener, when sensors-detect gave the IBM warning,
I was running as non-root.
It doesn't give the warning when run as root.
Sorry for the false alarm.

But anyway, would it be helpful to put out a message in sensors-detect
when it can't determine the system type, even as root?

And yes, on the 2 "typical" machines, it doesn't mention VPD.

nice job on the dmidecode package and website BTW.

Jean Delvare wrote:
> 
> > tested new sensors-detect on 4 machines.
> > 2 w/ ACPI+DMI worked fine.
> 
> Typical configuration. The output doesn't even mention VPD, right?
> 
> > One with neither (iopener) gave the standard Thinkpad warning at the
> > beginning.
> 
> Should not happen. This could happen if /dev/mem is unreadable for some
> reason. Could you download dmidecode CVS and give a try to biosdecode?
> Then edit the Makefile, remove -DUSE_MMAP, compile again and retry.
> Might crash. If it does, this means I should use mmap() to access
> /dev/mem in sensors-detect - although I don't even know how to do that.
> 
> Anything special about that iopener? You once sent me a copy of the BIOS
> area and there's nothing strange at first sight, except that it actually
> doesn't support any of ACPI, SMBIOS nor VPD.
> 
> If biosdecode works OK on it (both with and without -DUSE_MMAP), please
> try to track down why system_safeness_by_vpd in sensors-detect returns
> 0. There are three reasons it could (you'll see in the code). The third
> one is the more likely to cause problems (I think).
> 
> > On my HP ia64 machine it didn't give any ACPI or DMI info but didn't
> > give the Thinkpad warning either... what's up with that?
> 
> That's the expected behavior. This is why VPD supersedes the other
> methods. If ACPI or DMI isn't present, you can't conclude, while VPD
> being not there *does* mean that it's not an IBM system (with the
> exception of one very specific Thinkpad model).
> 
> Thanks a lot for the feedback.
> 
> --
> Jean Delvare
> http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Get rid of DMI and ACPI detection methods. VPD should be
>         sufficent.

This is the last step to a definitive (hopefully) solution to the
Thinkpad problem.

What's missing now is the list of faulty Thinkpads (those with the
24RF08) to blacklist, or the list of non-faulty Thinkpads to white-list.
The problem is that I doubt we'll ever obtain such lists in an official
way. Even if we would, this still means we would have to maintain these
lists. Maintaining a blacklist in real-time is almost impossible without
an active support from IBM, which I guess we'll never get. Maintaining a
white-list is safer, but still requires that someone spends time to
track newly released Thinkpad models.

So I have been thinking of another way to handle the problem. Here we
go.

I assume the following is true:
1* We have a reliable way to detect IBM systems in sensors-detect (VPD).
2* Some of these systems have a dangerous 24RF08 chip on-board. Such
chips use a fixed range of addresses: 0x54-0x57.
3* These addresses are only used by two drivers in our package: eeprom
and ddcmon. These drivers now have a workaround that is supposed to
prevent the 24RF08 corruption. The sensors-detect script itself has such
a workaround
4* The i2c-piix4, which is the bus driver needed on all faulty systems
seen so far, won't load on IBM systems (using DMI, not VPD).
Let me know if I'm wrong somewhere.

3 and 4 together make it not even necessary to prevent IBM users to run
sensors-detect. Nothing bad should happen anymore. Even 4 is
somewhat redundant with 3 (and not very reliable still it could miss
some IBM systems). Still I understand that we want to play it safe. But
we don't have to bail out from sensors-detect if an IBM system is
detected. Instead, what about simply excluding 0x54-0x57 from the scan
range and/or exclude the eeprom and ddcmon drivers?

Since regular eeproms are rarely seen within 0x54-0x57 (except on Sony
laptops, but they are not IBM laptops, of course), excluding this range
from the scan is not really a problem. And excluding addresses from the
scan should be really easy.

Excluding drivers would probably be a bit more tricky, and I don't think
it's necessary. Exclusing addresses should be sufficent, and even safer.

I first feared that the ddcmon would cause trouble because it declares
0x51-0x57 as subaddresses, but reading the driver code, it seems that it
doesn't use them (and doesn't even "register" them).

One additional note. In sensors-detect we scan all addresses from 0x00
to 0x7F. I don't think it's correct. According to what I know, 0x00,
0x02-0x03, 0x0C and 0x7C-0x7F should not be scanned, being respectively
the general call address, a reserved range, the alert response address
and another reserved range. I propose to exclude these addresses from
the scan. Then, all we need to do is add four addresses to the exclusion
list if an IBM system is detected. This also opens a path for more
addresses exclusions if this is needed later.

Comments welcome.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (mds) add sysfs support to sensors-detect by using 'i2cdetect -l'
>       instead of 'cat /proc/bus/i2c'

Is this considered a definitive change, or am I still supposed to code
something there?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

that should do it.
It got fairly complex, so I realized that rather than recode it all in
perl
it would be better to just tweak i2cdetect to output exactly what
sensors-detect
needed (with the new -l option) than call i2cdetect.
So sensors-detect now needs i2cdetect but I think that's OK.


Jean Delvare wrote:
> 
> > Modified Files:
> >       sensors-detect
> > Log Message:
> > (mds) add sysfs support to sensors-detect by using 'i2cdetect -l'
> >       instead of 'cat /proc/bus/i2c'
> 
> Is this considered a definitive change, or am I still supposed to code
> something there?
> 
> --
> Jean Delvare
> http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> > One additional note. In sensors-detect we scan all addresses from
> > 0x00 to 0x7F. I don't think it's correct. According to what I know,
> > 0x00, 0x02-0x03, 0x0C and 0x7C-0x7F should not be scanned, being
> > respectively the general call address, a reserved range, the alert
> > response address and another reserved range. I propose to exclude
> > these addresses from the scan. Then, all we need to do is add four
> > addresses to the exclusion list if an IBM system is detected. This
> > also opens a path for more addresses exclusions if this is needed
> > later.
> 
> agreed. this used to be on my to-do list.
> i2c-core scans all addressess too which is even worse
> than having userspace do it.

Does i2c-core scan anything? I thought the chip drivers were (and not
all addresses of course, only those specified in the driver).

> Actually it's not that bad to scan all the adresses, 
> it's just bad to probe them all with each of the driver tests.

What distinction do you make here between scan and probe? Not sure I get
you.

> the worst one is 0x00. The others shouldn't really be a problem, I
> don't think.

Agreed, but I don't see the point in scanning addresses that cannot be
used by valid chips.

My fear with 0x00 is that, if it really works as a broadcast address, a
quick write on it could reach a 24RF08 chip, and break it (since the
workaround is only enabled on addresses 0x54-0x57).

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
> 
> > Modified Files:
> >       sensors-detect
> > Log Message:
> > (Khali) Get rid of DMI and ACPI detection methods. VPD should be
> >         sufficent.
> 
> This is the last step to a definitive (hopefully) solution to the
> Thinkpad problem.
> 
> What's missing now is the list of faulty Thinkpads (those with the
> 24RF08) to blacklist, or the list of non-faulty Thinkpads to white-list.
> The problem is that I doubt we'll ever obtain such lists in an official
> way. Even if we would, this still means we would have to maintain these
> lists. Maintaining a blacklist in real-time is almost impossible without
> an active support from IBM, which I guess we'll never get. Maintaining a
> white-list is safer, but still requires that someone spends time to
> track newly released Thinkpad models.
> 
> So I have been thinking of another way to handle the problem. Here we
> go.
> 
> I assume the following is true:
> 1* We have a reliable way to detect IBM systems in sensors-detect (VPD).
> 2* Some of these systems have a dangerous 24RF08 chip on-board. Such
> chips use a fixed range of addresses: 0x54-0x57.
> 3* These addresses are only used by two drivers in our package: eeprom
> and ddcmon. These drivers now have a workaround that is supposed to
> prevent the 24RF08 corruption. The sensors-detect script itself has such
> a workaround
> 4* The i2c-piix4, which is the bus driver needed on all faulty systems
> seen so far, won't load on IBM systems (using DMI, not VPD).
> Let me know if I'm wrong somewhere.
> 
> 3 and 4 together make it not even necessary to prevent IBM users to run
> sensors-detect. Nothing bad should happen anymore. Even 4 is
> somewhat redundant with 3 (and not very reliable still it could miss
> some IBM systems). Still I understand that we want to play it safe. But
> we don't have to bail out from sensors-detect if an IBM system is
> detected. Instead, what about simply excluding 0x54-0x57 from the scan
> range and/or exclude the eeprom and ddcmon drivers?
> 
> Since regular eeproms are rarely seen within 0x54-0x57 (except on Sony
> laptops, but they are not IBM laptops, of course), excluding this range
> from the scan is not really a problem. And excluding addresses from the
> scan should be really easy.
> 
> Excluding drivers would probably be a bit more tricky, and I don't think
> it's necessary. Exclusing addresses should be sufficent, and even safer.
> 

Agreed that we are being exceptionally conservative.
I recommend that we proceed in baby steps, a little bit more in each
release,
so that we get some testing and confidence.

Some way of separating Thinkpads from servers would be a good first
step.
We could blacklist all Thinkpads but go a little farther with the
servers.

> I first feared that the ddcmon would cause trouble because it declares
> 0x51-0x57 as subaddresses, but reading the driver code, it seems that it
> doesn't use them (and doesn't even "register" them).

This is because some monitors use the exceptionally stupid 24C00 or
24C01 (NOT 24C01A)
eeproms that respond to ALL addresses 0x50-57 with the same data.
The subaddress declaration keeps sensors-detect from recommending the
driver getting loaded 8 times, or ddcmon once and eeprom 7 times.

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
> 
> > > One additional note. In sensors-detect we scan all addresses from
> > > 0x00 to 0x7F. I don't think it's correct. According to what I know,
> > > 0x00, 0x02-0x03, 0x0C and 0x7C-0x7F should not be scanned, being
> > > respectively the general call address, a reserved range, the alert
> > > response address and another reserved range. I propose to exclude
> > > these addresses from the scan. Then, all we need to do is add four
> > > addresses to the exclusion list if an IBM system is detected. This
> > > also opens a path for more addresses exclusions if this is needed
> > > later.
> >
> > agreed. this used to be on my to-do list.
> > i2c-core scans all addressess too which is even worse
> > than having userspace do it.
> 
> Does i2c-core scan anything? I thought the chip drivers were (and not
> all addresses of course, only those specified in the driver).
> 

well, i2c-core has i2c-probe(), but nobody calls it but lm92.
interesting.
i2c-proc has i2c-detect(), which all the drivers call
(including lm92... hmm...).

But the clients control the address range for both calls,
so that's OK. Although maybe i2c-core and i2c-proc shouldn't allow
scans of 0x00?


> > Actually it's not that bad to scan all the adresses,
> > it's just bad to probe them all with each of the driver tests.
> 
> What distinction do you make here between scan and probe? Not sure I get
> you.
> 

By 'scan' I meant a presence detect using write quick.
By 'probe' I meant reading registers in the device to look for device
IDs, etc.
That may not be the correct terminology.

> > the worst one is 0x00. The others shouldn't really be a problem, I
> > don't think.
> 
> Agreed, but I don't see the point in scanning addresses that cannot be
> used by valid chips.
> 
> My fear with 0x00 is that, if it really works as a broadcast address, a
> quick write on it could reach a 24RF08 chip, and break it (since the
> workaround is only enabled on addresses 0x54-0x57).
> 
> --
> Jean Delvare
> http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Agreed that we are being exceptionally conservative.
> I recommend that we proceed in baby steps, a little bit more in each
> release, so that we get some testing and confidence.

Agreed. I'd even propose not to change anything before the next release.
The detection method change is enough.

> Some way of separating Thinkpads from servers would be a good first
> step. We could blacklist all Thinkpads but go a little farther with
> the servers.

That's not necessarily easy. This means adding the list of known
Thinkpad IDs to sensors-detect. As I explained, this is a list we would
then have to maitain, with or without the help of IBM. You can imagine
that someone with a brand new Thinkpad, not listed yet, would give a try
to lm_sensors and have his/her laptop seen as a desktop IBM system.

And, since we don't have a list of desktop IDs, we can't build a
white-list either.

This is how I came to my simple "IBM detected, block 24RF08 addresses"
strategy. It is both safe and almost-non-intrusive, as far as I can see.

> > I first feared that the ddcmon would cause trouble because it
> > declares 0x51-0x57 as subaddresses, but reading the driver code, it
> > seems that it doesn't use them (and doesn't even "register" them).
> 
> This is because some monitors use the exceptionally stupid 24C00 or
> 24C01 (NOT 24C01A) eeproms that respond to ALL addresses 0x50-57 with
> the same data. The subaddress declaration keeps sensors-detect from
> recommending the driver getting loaded 8 times, or ddcmon once and
> eeprom 7 times.

I think all my monitors do that. But since I read in the DDC specs that
there could be more than one 256-byte block of data, I thought it was
just normal. I clearly remember my monitors answering on all 8
addresses, but I don't remember if all addresses were returning the same
data. I'll take a look next time.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
> 
> > Agreed that we are being exceptionally conservative.
> > I recommend that we proceed in baby steps, a little bit more in each
> > release, so that we get some testing and confidence.
> 
> Agreed. I'd even propose not to change anything before the next release.
> The detection method change is enough.
> 
> > Some way of separating Thinkpads from servers would be a good first
> > step. We could blacklist all Thinkpads but go a little farther with
> > the servers.
> 
> That's not necessarily easy. This means adding the list of known
> Thinkpad IDs to sensors-detect. As I explained, this is a list we would
> then have to maitain, with or without the help of IBM. You can imagine
> that someone with a brand new Thinkpad, not listed yet, would give a try
> to lm_sensors and have his/her laptop seen as a desktop IBM system.
> 
> And, since we don't have a list of desktop IDs, we can't build a
> white-list either.
> 

I was hoping the DMI would have a string that said 'laptop' or
'thinkpad',
or something would have a model number that was consistent with
laptop model numbers.

> This is how I came to my simple "IBM detected, block 24RF08 addresses"
> strategy. It is both safe and almost-non-intrusive, as far as I can see.
> 
> > > I first feared that the ddcmon would cause trouble because it
> > > declares 0x51-0x57 as subaddresses, but reading the driver code, it
> > > seems that it doesn't use them (and doesn't even "register" them).
> >
> > This is because some monitors use the exceptionally stupid 24C00 or
> > 24C01 (NOT 24C01A) eeproms that respond to ALL addresses 0x50-57 with
> > the same data. The subaddress declaration keeps sensors-detect from
> > recommending the driver getting loaded 8 times, or ddcmon once and
> > eeprom 7 times.
> 
> I think all my monitors do that. But since I read in the DDC specs that
> there could be more than one 256-byte block of data, I thought it was
> just normal. I clearly remember my monitors answering on all 8
> addresses, but I don't remember if all addresses were returning the same
> data. I'll take a look next time.
> 

It's not uncommon for 8 shadows, but the usual case is the more common
24C01A chip that responds to only one address.

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
> 
>
> One additional note. In sensors-detect we scan all addresses from 0x00
> to 0x7F. I don't think it's correct. According to what I know, 0x00,
> 0x02-0x03, 0x0C and 0x7C-0x7F should not be scanned, being respectively
> the general call address, a reserved range, the alert response address
> and another reserved range. I propose to exclude these addresses from
> the scan. Then, all we need to do is add four addresses to the exclusion
> list if an IBM system is detected. This also opens a path for more
> addresses exclusions if this is needed later.
> 
> Comments welcome.


agreed. this used to be on my to-do list.
i2c-core scans all addressess too which is even worse
than having userspace do it.

Actually it's not that bad to scan all the adresses, 
it's just bad to probe them all with
each of the driver tests.

the worst one is 0x00. The others shouldn't really be a problem, I don't
think.

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

khali,
this worked fine for me on my vt1211 board.
mds

author@mordac.netroedge.com wrote:
> Update of /home/cvs/lm_sensors2/prog/detect
> In directory mordac.netroedge.com:/tmp/cvs-serv6615
> 
> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) Fix broken superio exit sequence handling.
> 
> 

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Works for me on W83627thf - in case you wanted feedback.

Sure I wanted :) Thanks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark M. Hoffman]

* author@mordac.netroedge.com <author@mordac.netroedge.com> [2004-03-02 13:05:44 -0800]:
> Update of /home/cvs/lm_sensors2/prog/detect
> In directory mordac.netroedge.com:/tmp/cvs-serv1771
> 
> Modified Files:
> 	sensors-detect 
> Log Message:
> (Khali) More Super IO chips. Reworked exit sequences policy.

Works for me on W83627thf - in case you wanted feedback.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> IIRC, you have a real sis5595 board.  Could you please try
> this out on it?  W/ kernel 2.6 as well, if you can.

FYI, I tested the new version on two non-SiS systems and didn't see any
regresssion. Good :)

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark M. Hoffman]

Hi Mark:

IIRC, you have a real sis5595 board.  Could you please try
this out on it?  W/ kernel 2.6 as well, if you can.

* author@mordac.netroedge.com <author@mordac.netroedge.com> [2004-03-14 20:23:54 -0800]:
> Update of /home/cvs/lm_sensors2/prog/detect
> In directory mordac.netroedge.com:/tmp/cvs-serv28400/prog/detect
> 
> Modified Files:
> 	sensors-detect 
> Log Message:
> (mmh) Added custom detection for some SiS chipsets.  This resolves a long-
> standing false detection problem.  Other changes in support of this custom
> detection:
> 
> 1) Changed pci_list to a hash, with keys of the form "1039:0016"
> 2) Added @kernel_version detection and comparison routine (thanks Khali)
> 3) Removed legacy sub: read_proc_pci

Thanks,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

sent it to Phil a couple of years ago.
Phil?

Mark M. Hoffman wrote:
> Hi Mark:
> 
> IIRC, you have a real sis5595 board.  Could you please try
> this out on it?  W/ kernel 2.6 as well, if you can.
> 
> * author@mordac.netroedge.com <author@mordac.netroedge.com> [2004-03-14 20:23:54 -0800]:
> 
>>Update of /home/cvs/lm_sensors2/prog/detect
>>In directory mordac.netroedge.com:/tmp/cvs-serv28400/prog/detect
>>
>>Modified Files:
>>	sensors-detect 
>>Log Message:
>>(mmh) Added custom detection for some SiS chipsets.  This resolves a long-
>>standing false detection problem.  Other changes in support of this custom
>>detection:
>>
>>1) Changed pci_list to a hash, with keys of the form "1039:0016"
>>2) Added @kernel_version detection and comparison routine (thanks Khali)
>>3) Removed legacy sub: read_proc_pci
> 
> 
> Thanks,
> 

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> Modified Files:
> 	sensors-detect 
> Log Message:
> (mds) support detection of eeprom shadows we now know are
>       software write-protect registers.

I'm not sure we want to do that. According to a number of datasheets
Rudolf pointed me to yesterday, some EEPROMs will "not care" about what
data you write to their write-protection address. "Any write" will
write-protect the EEPROM. It's not very clear whether it means "any
write byte data" or just any write, which could include quick command 0.

This is the reason why I proposed to scan the 0x30-0x37 range with read
bytes instad of quick command 0. Of course, if we do that, we cannot
detect page protection addresses. But actually, we cannot read from them
and do not want to write to them, so what's the point detecting them?

>       prevent Vaio detection from running more than once.

Mmm, was there a bug before? I can't see what it was. Actually the fact
that I was testing the Vaio EEPROM in all cases was wanted, it was
preventing Vaio EEPROMS from being first announced as SPD EEPROMs, then
Vaio ones. I agree that the second would have had a higher confidence
value anyway, but this reduced the noise for the end user, so it's the
right thing to do IMHO. (Also, there used to be a problem with
conflicting detections implying the same driver twice; sensors-detect
would recommend to exclude the "conflicting" address when loading the
module, while this address was correct and would have worked! I believe
it's now fixed for I2C addresses (but probably not ISA) but this also
explains some places where extra checks were done just to workaround
this issue.) I also agree that this approach reveals that the underlying
detection method is not the best it could be. For now, detection
functions only answer the question "what are the chances that the chip
at address X is Y?" while most of the time the question we would like to
answer to is "if the chip at X is of type Z, which subtype Y is it the
more likely to be?" However I guess it would require too much work to
change sensors-detect in that way now, which is why I had to use tricks
to achieve the same objective.

The fact that we default to SPD EEPROMs is probably not correct, BTW. It
used to be the only EEPROM type found in PCs, but now you have eeprom on
Xeons, Vaios, network adapters and more... I think we should have a
"generic EEPROM" entry and use that if none of the other test worked.

Thanks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

I see.
fixed, sorry.
mds

Jean Delvare wrote:
>>unless I'm missing something, there are no more accesses to the bus
>>at that address in scan_adapter() after the detection function
>>returns, so it isn't necessary.
> 
> 
> Wrong. There can be more than one possible chip at a given address. For
> example we detect the MAX6900 at 0x50 after the EEPROMs. After your
> change, I suspect that we actually detect it at 0x30 (where it cannot
> be) instead.
> 
> Even without that, I think we should never rely on the order of the
> entries in @chips_id, nor on the fact that some addresses are used by a
> limited number of chips. These are thing that can change quickly and
> nobody will remember that part of the detection code depended on a
> condition that isn't true anymore.
> 

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> The NM34W02 datasheet makes clear in Figure 5 what a byte write is,
> it's what we call byte-write-data.
> The ST M34C02 datasheet has the same thing in Figure 6.

OK, good point. You're definitely better than me at readin datasheets ;)

> You're right we can't guarantee it. But the datasheets I've seen
> have been quite consistent.

I take note.

> Maybe it's a value to tell the user what it is.
> But we've been telling them for years it's a "shadow",
> maybe it doesn't add any value to tell them otherwise.

Well you've documented it the way it has to be, I think it's enough if
people need additional information.

> We've been using quick write for years, and haven't ever had a report
> that somebody saw something once at 30-37 and never again.
> Of course who would notice such a thing?

Nobody, especially since we were not actually detecting it, and also
because write-protecting the eeprom would be harmless in most cases (SPD
EEPROMS for example have no reason to be updated - write-protecting them
could even be considered a good thing).

> > I would suggest that we comment out the eeprom with $chip = 2 in
> > @chips_id by default. We can leave it in if some people happen to
> > need it, but not do it by default.
> 
> If you wish. I did it as an experiment.

OK, I've done that.

> > BTW, at least one EEPROM model (Catalyst CAT34RC02) can also read
> > byte from its write-protection address, so your detection method
> > would miss it.
> 
> Correct. We'd have to add it as yet another eeprom chip type.
> Which stretches things even more.

Feel free to add this in sensors-detect, I've not done it. Also feel
free to add a "default eeprom" entry and rework eeprom_detect a little
bit if you want.

So sensors-detect is clear. I've retained ranges 0x30-0x37 and 0x50-0x5F
for probe-with-reads. I'll do i2cdetect tomorrow. I plan to use the same
ranges.

Maybe we could provide command line flags to restore original probing
(all quick writes) and all probe-with-reads (with all due warnings)?

Then I'll submit a patch for the 2.6 kernels. i2c-core needs to be
changed to issue reads instead of quick writes on the above-mentioned
range. eeprom and ddcmon drivers lose their extra quick write. i2c-piix4
lose its IBM check. I don't know if we want to even remove the unsafe
smbus check in dmi_scan, I'll see that later.

As for the 2.4 kernel modules, there's a problem there. i2c-core and
eeprom are distributed in different packages and not necessarily in
sync, since there is no explicit version checking. Thus mixing a new
i2c-core with an old eeprom would result in the extra quick write in
eeprom actually corrupt the chip. Bad. There's still the IBM check in
i2c-piix4 however. Maybe we don't want to change anything in 2.4 after
all (except ensure that the extra quickwrites are consistently issued).

Comments welcome.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

unless I'm missing something, there are no more accesses to the bus
at that address in scan_adapter() after the detection function returns,
so it isn't necessary.

Jean Delvare wrote:
> Oh, and:
> 
>   if ($chip = 2) {
> 	# check for 'shadow' write-protect register at 0x30-37
> 	i2c_set_slave_addr($file,$addr - 0x20);
> 	if(i2c_smbus_write_quick($file,$SMBUS_WRITE) >= 0 &&
> 	   i2c_smbus_read_byte_data($file,0x80) = -1) {
> 		if($checksum = 0) {
> 			return (9, $addr - 0x20);
> 		} else {
> 			return (2, $addr - 0x20);
> 		}
> 	}
>   }
> 
> Shouldn't you restore the original slave address before returning?
> 

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
>>the point is that we tell people what's at 0x30, that it's associated
>>with what's at 0x50.
> 
> 
> If we follow my idea to use reads instead of quick writes for the
> 0x30-0x37 range, then the user doesn't see 0x30 as holding a device, so
> we don't have to tell him what's there.
> 
> 
>>I went through the datasheets of the Microchip and ST parts pretty
>>carefully yesterday, it's clear that it's a byte-write-data that
>>does the write-protect.
> 
> 
> And other chips (Fairchild NM34W02) say "byte write". If that's "byte
> write" in the SMBus way, then it's a 2-byte command (as opposed to
> 3-byte byte-write-data). I wouldn't swear that we will never see a chip
> for which a quick write would be sufficent.
> 

The NM34W02 datasheet makes clear in Figure 5 what a byte write is,
it's what we call byte-write-data.
The ST M34C02 datasheet has the same thing in Figure 6.
You're right we can't guarantee it. But the datasheets I've seen
have been quite consistent.


> My point is that I don't see the benefit in telling the user that he has
> a write-protectable EEPROM, while I see the potential danger in
> quick-writing the eeprom's write-protect address.
> 

Maybe it's a value to tell the user what it is.
But we've been telling them for years it's a "shadow",
maybe it doesn't add any value to tell them otherwise.

We've been using quick write for years, and haven't ever had a report that
somebody saw something once at 30-37 and never again.
Of course who would notice such a thing?

> I would suggest that we comment out the eeprom with $chip = 2 in
> @chips_id by default. We can leave it in if some people happen to need
> it, but not do it by default.
> 

If you wish. I did it as an experiment.

> BTW, at least one EEPROM model (Catalyst CAT34RC02) can also read byte
> from its write-protection address, so your detection method would miss
> it.
> 

Correct. We'd have to add it as yet another eeprom chip type.
Which stretches things even more.


> 
>>Clearly the Vaio detection in eeprom_detect() was being called
>>both for chip=0 and chip=1, so I changed it so it was only called
>>for chip=1. I didn't realize that you did it for both to
>>reduce "noise". IMO not doing things twice is better than
>>reducing "noise".
> 
> 
> Granted. The global detection scheme is poor but making detection
> functions more complex to compensate that isn't the right thing to do, I
> admit. I think that my main reason for doing that was because
> sensors-detect used to suggest bogus module options in that case. Then I
> fixed that but did not revert the detect_eeprom function.
> 
> 
>>Perhaps a better long-term solution is to call a detection
>>routine once and allow it to return different names,
>>rather than calling it multiple times, one for each chip name.
> 
> 
> Yes, I think that would be the correct way. But this means much work...
> 

p.s. sorry I vanished from IRC on Sunday, I got busy...

lm_sensors2/prog/detect sensors-detect

[Mark Studebaker]

Jean Delvare wrote:
>>Modified Files:
>>	sensors-detect 
>>Log Message:
>>(mds) support detection of eeprom shadows we now know are
>>      software write-protect registers.
> 
> 
> I'm not sure we want to do that. According to a number of datasheets
> Rudolf pointed me to yesterday, some EEPROMs will "not care" about what
> data you write to their write-protection address. "Any write" will
> write-protect the EEPROM. It's not very clear whether it means "any
> write byte data" or just any write, which could include quick command 0.
> 
> This is the reason why I proposed to scan the 0x30-0x37 range with read
> bytes instad of quick command 0. Of course, if we do that, we cannot
> detect page protection addresses. But actually, we cannot read from them
> and do not want to write to them, so what's the point detecting them?
> 
> 


the point is that we tell people what's at 0x30, that it's associated
with what's at 0x50.

I went through the datasheets of the Microchip and ST parts pretty
carefully yesterday, it's clear that it's a byte-write-data that
does the write-protect.



>>      prevent Vaio detection from running more than once.
> 
> 
> Mmm, was there a bug before? I can't see what it was. Actually the fact
> that I was testing the Vaio EEPROM in all cases was wanted, it was
> preventing Vaio EEPROMS from being first announced as SPD EEPROMs, then
> Vaio ones. I agree that the second would have had a higher confidence
> value anyway, but this reduced the noise for the end user, so it's the
> right thing to do IMHO. (Also, there used to be a problem with
> conflicting detections implying the same driver twice; sensors-detect
> would recommend to exclude the "conflicting" address when loading the
> module, while this address was correct and would have worked! I believe
> it's now fixed for I2C addresses (but probably not ISA) but this also
> explains some places where extra checks were done just to workaround
> this issue.) I also agree that this approach reveals that the underlying
> detection method is not the best it could be. For now, detection
> functions only answer the question "what are the chances that the chip
> at address X is Y?" while most of the time the question we would like to
> answer to is "if the chip at X is of type Z, which subtype Y is it the
> more likely to be?" However I guess it would require too much work to
> change sensors-detect in that way now, which is why I had to use tricks
> to achieve the same objective.
> 

Clearly the Vaio detection in eeprom_detect() was being called
both for chip=0 and chip=1, so I changed it so it was only called
for chip=1. I didn't realize that you did it for both to
reduce "noise". IMO not doing things twice is better than
reducing "noise".

Perhaps a better long-term solution is to call a detection
routine once and allow it to return different names,
rather than calling it multiple times, one for each chip name.


> The fact that we default to SPD EEPROMs is probably not correct, BTW. It
> used to be the only EEPROM type found in PCs, but now you have eeprom on
> Xeons, Vaios, network adapters and more... I think we should have a
> "generic EEPROM" entry and use that if none of the other test worked.
> 
> Thanks.
> 

agreed

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> unless I'm missing something, there are no more accesses to the bus
> at that address in scan_adapter() after the detection function
> returns, so it isn't necessary.

Wrong. There can be more than one possible chip at a given address. For
example we detect the MAX6900 at 0x50 after the EEPROMs. After your
change, I suspect that we actually detect it at 0x30 (where it cannot
be) instead.

Even without that, I think we should never rely on the order of the
entries in @chips_id, nor on the fact that some addresses are used by a
limited number of chips. These are thing that can change quickly and
nobody will remember that part of the detection code depended on a
condition that isn't true anymore.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

> the point is that we tell people what's at 0x30, that it's associated
> with what's at 0x50.

If we follow my idea to use reads instead of quick writes for the
0x30-0x37 range, then the user doesn't see 0x30 as holding a device, so
we don't have to tell him what's there.

> I went through the datasheets of the Microchip and ST parts pretty
> carefully yesterday, it's clear that it's a byte-write-data that
> does the write-protect.

And other chips (Fairchild NM34W02) say "byte write". If that's "byte
write" in the SMBus way, then it's a 2-byte command (as opposed to
3-byte byte-write-data). I wouldn't swear that we will never see a chip
for which a quick write would be sufficent.

My point is that I don't see the benefit in telling the user that he has
a write-protectable EEPROM, while I see the potential danger in
quick-writing the eeprom's write-protect address.

I would suggest that we comment out the eeprom with $chip = 2 in
@chips_id by default. We can leave it in if some people happen to need
it, but not do it by default.

BTW, at least one EEPROM model (Catalyst CAT34RC02) can also read byte
from its write-protection address, so your detection method would miss
it.

> Clearly the Vaio detection in eeprom_detect() was being called
> both for chip=0 and chip=1, so I changed it so it was only called
> for chip=1. I didn't realize that you did it for both to
> reduce "noise". IMO not doing things twice is better than
> reducing "noise".

Granted. The global detection scheme is poor but making detection
functions more complex to compensate that isn't the right thing to do, I
admit. I think that my main reason for doing that was because
sensors-detect used to suggest bogus module options in that case. Then I
fixed that but did not revert the detect_eeprom function.

> Perhaps a better long-term solution is to call a detection
> routine once and allow it to return different names,
> rather than calling it multiple times, one for each chip name.

Yes, I think that would be the correct way. But this means much work...

- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

lm_sensors2/prog/detect sensors-detect

[Jean Delvare]

Oh, and:

  if ($chip = 2) {
	# check for 'shadow' write-protect register at 0x30-37
	i2c_set_slave_addr($file,$addr - 0x20);
	if(i2c_smbus_write_quick($file,$SMBUS_WRITE) >= 0 &&
	   i2c_smbus_read_byte_data($file,0x80) = -1) {
		if($checksum = 0) {
			return (9, $addr - 0x20);
		} else {
			return (2, $addr - 0x20);
		}
	}
  }

Shouldn't you restore the original slave address before returning?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/