w83781d family detection

w83781d family detection

[Jean Delvare]

I made significant changes to the w83781d family detection method in
sensors-detect.

* Separate detection sub for W83L784R/AR. Its registers differ from the
other chipsets in the family, it shouldn't have been added to
w83781d_detect in the first place.
* Ranges limited to what the specs say for I2C-only chips (see below).
* Add detection for the AS99127F rev.2 and explicit detection of the
ASB100 Bach.
* Improve code readability.
* Do not ignore the chipset ID LSB anymore. Explicit acceptation of 0x11
as a W83781D ID instead.
* Fix a bug in secondary LM75 address extraction. The previous code
returned 0x48 whatever the register could contain. I'm astonished it was
never reported. This means that most if not all chipsets in that family
use 0x48 as their secondary LM75's address.

Similar changes should be made to the w83781d driver itself, but I would
like my changes to be tested before that. My tests were successful on
three chips I own (AS99127F rev.1, W83781D and W83782D) but more tests
would be welcome.

About the address ranges:

For the W83783S and the W83L784R/AR, no address selection pins exist.
Since they cab be accessed only via the I2C bus, this make me think the
address is unlikely to be changed, so I limited the range to the default
address (0x2D). Likewise, the W83791D has 2 address selector pins, so
the range is 0x2C-0x2F (4 addresses).
For Asus chip, I chose the range 0x28-0x2F. We have no datasheet, this
seem to be a good compromise (and Alex does the same). However, the fact
that the register 0x48 doesn't hold the chip I2C address make me think
Asus doesn't want the address to be changed at all, so we could limit
the address to 0x2d. (MMH, do you confirm that your ASB100 has nothing
useful at 0x48?)

I think it's better to limit the ranges. This reduces scan time (not
significantly I guess) and limits the risks of misdetection and
corruptions. I browsed the mailing list and every log I could find fits
the new ranges. I voluntarily left the ISA chips ranges unchanged, since
in this case the address can be changed using the ISA bus and we have
reports it occured (W83627HF have been seen at 0x2c and 0x2f according
to what I read in our archives).

I'd like to thank Alex *a lot* for taking some time to tell us about how
he detects the Asus chips. I'll add a copy of his explanations to our
w83781d doc soon.

Comments welcome.

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

w83781d family detection

[Mark M. Hoffman]

* Jean Delvare <khali@linux-fr.org> [2003-07-26 11:26:57 +0200]:
> 
> I made significant changes to the w83781d family detection method in
> sensors-detect.
> 
> * Separate detection sub for W83L784R/AR. Its registers differ from the
> other chipsets in the family, it shouldn't have been added to
> w83781d_detect in the first place.
> * Ranges limited to what the specs say for I2C-only chips (see below).
> * Add detection for the AS99127F rev.2 and explicit detection of the
> ASB100 Bach.

Nice.  I tested this against Bach just now.  Timely too, since I'm
doing an independent driver for that chip.  Anyone have a preference
for the name?  If not it will be "bach.c".

> * Improve code readability.
> * Do not ignore the chipset ID LSB anymore. Explicit acceptation of 0x11
> as a W83781D ID instead.
> * Fix a bug in secondary LM75 address extraction. The previous code
> returned 0x48 whatever the register could contain. I'm astonished it was
> never reported. This means that most if not all chipsets in that family
> use 0x48 as their secondary LM75's address.
> 
> Similar changes should be made to the w83781d driver itself, but I would
> like my changes to be tested before that. My tests were successful on
> three chips I own (AS99127F rev.1, W83781D and W83782D) but more tests
> would be welcome.

Seems to work on Bach.  Will you do a patch for 2.6?  Otherwise I can.

> About the address ranges:
> 
> For the W83783S and the W83L784R/AR, no address selection pins exist.
> Since they cab be accessed only via the I2C bus, this make me think the
> address is unlikely to be changed, so I limited the range to the default
> address (0x2D). Likewise, the W83791D has 2 address selector pins, so
> the range is 0x2C-0x2F (4 addresses).
> For Asus chip, I chose the range 0x28-0x2F. We have no datasheet, this
> seem to be a good compromise (and Alex does the same). However, the fact
> that the register 0x48 doesn't hold the chip I2C address make me think
> Asus doesn't want the address to be changed at all, so we could limit
> the address to 0x2d. (MMH, do you confirm that your ASB100 has nothing
> useful at 0x48?)

AFAIK, Bach is I2C only.  Mine reads 0 from 0x48.

> I think it's better to limit the ranges. This reduces scan time (not
> significantly I guess) and limits the risks of misdetection and
> corruptions. I browsed the mailing list and every log I could find fits
> the new ranges. I voluntarily left the ISA chips ranges unchanged, since
> in this case the address can be changed using the ISA bus and we have
> reports it occured (W83627HF have been seen at 0x2c and 0x2f according
> to what I read in our archives).

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

w83781d family detection

[Jean Delvare]

> Nice.  I tested this against Bach just now.  Timely too, since I'm
> doing an independent driver for that chip.  Anyone have a preference
> for the name?  If not it will be "bach.c".

I would have chosen asb100, but well it's you writing the driver ;) And
the name ain't really important (unless you choose something as explicit
as "bt6dz1kb" of course) so do just as you want.

> > Similar changes should be made to the w83781d driver itself, but I
> > would like my changes to be tested before that. My tests were
> > successful on three chips I own (AS99127F rev.1, W83781D and
> > W83782D) but more tests would be welcome.
> 
> Seems to work on Bach.  Will you do a patch for 2.6?  Otherwise I can.

I will do the changes on our driver tomorrow, test, commit, let you
test, and then submit a patch for 2.6.

> AFAIK, Bach is I2C only.  Mine reads 0 from 0x48.

And is at I2C address 0x2d I suppose? Anyway, unless we are sure these
chips have no address selection pins, we can't conclude. However, I
expect them to be pin-to-pin compatible with their Winbond counterparts,
and as such are unlikely to have address selection pins. We have three
possibilities:

1* Leave everything unchanged.
2* Limit AS199237F and ASB100 to addres 0x2d.
3* Leave the whole 0x28-0x2f range possible but give the chips a lower
confidence if address isn't 0x2d.

I'm in favor of solution 3.

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

w83781d family detection

[Mark M. Hoffman]

* Jean Delvare <khali@linux-fr.org> [2003-07-27 23:18:56 +0200]:
> 
> > > Similar changes should be made to the w83781d driver itself, but I
> > > would like my changes to be tested before that. My tests were
> > > successful on three chips I own (AS99127F rev.1, W83781D and
> > > W83782D) but more tests would be welcome.
> > 
> > Seems to work on Bach.  Will you do a patch for 2.6?  Otherwise I can.
> 
> I will do the changes on our driver tomorrow, test, commit, let you
> test, and then submit a patch for 2.6.

Oops... I thought you already committed this.  So I haven't actually
tested it yet - but you probably knew that.  If you pass a patch to
the list first I'll give it a go.

> > AFAIK, Bach is I2C only.  Mine reads 0 from 0x48.
> 
> And is at I2C address 0x2d I suppose? Anyway, unless we are sure these

Yes.

> chips have no address selection pins, we can't conclude. However, I
> expect them to be pin-to-pin compatible with their Winbond counterparts,
> and as such are unlikely to have address selection pins. We have three
> possibilities:
> 
> 1* Leave everything unchanged.
> 2* Limit AS199237F and ASB100 to addres 0x2d.
> 3* Leave the whole 0x28-0x2f range possible but give the chips a lower
> confidence if address isn't 0x2d.
> 
> I'm in favor of solution 3.

I wouldn't mind limiting ASB100 to 0x2d, actually.  I don't recall ever
seeing it anywhere else - but if it is, we'll find out soon enough. ;)
Or choice 3 is fine too.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

w83781d family detection

[Jean Delvare]

> > I will do the changes on our driver tomorrow, test, commit, let you
> > test, and then submit a patch for 2.6.
> 
> Oops... I thought you already committed this.  So I haven't actually
> tested it yet - but you probably knew that.  If you pass a patch to
> the list first I'll give it a go.

My fault, I tested it yesterday but had to go before I could commit it.
Now this is done.

It works OK for me (as99127f, w83781d, w83782d).

Could you (and everyone who can) test the changes? I'd like to ensure it
doesn't break anything. I suggest we wait some times before submitting
the changes for inclusion into the 2.6 kernel (it is in no way a
critical fix).

> > > AFAIK, Bach is I2C only.  Mine reads 0 from 0x48.

OK, I update that in sensors-detect. We could use the fact that Asus
chips have 0 at 0x48 in our detection routines. We still need
confirmation of this for the AS99127F rev.2 though (but I don't think
any of us has one?) I suspect that this chip will be more
Winbond-compatible that the other two, since it has the Winbond
manifacturer's ID. BTW, the w83781 module won't load without forcing it
with an AS99127F rev.2 if it hasn't its address at 0x48, as the Winbond
chips do. Well, we'll see if anyone complains.

> > 1* Leave everything unchanged.
> > 2* Limit AS199237F and ASB100 to addres 0x2d.
> > 3* Leave the whole 0x28-0x2f range possible but give the chips a
> > lower confidence if address isn't 0x2d.
> > 
> > I'm in favor of solution 3.
> 
> I wouldn't mind limiting ASB100 to 0x2d, actually.  I don't recall
> ever seeing it anywhere else - but if it is, we'll find out soon
> enough. ;) Or choice 3 is fine too.

We can have slightly different strategies in sensors-detect and the
drivers themselves. I will apply solution 3 to sensors-detect. Feel free
to accept only address 0x2d for your Bach-only driver.

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

w83781d family detection

[Mark M. Hoffman]

* Jean Delvare <khali@linux-fr.org> [2003-07-31 09:20:16 +0200]:
> 
> > > I will do the changes on our driver tomorrow, test, commit, let you
> > > test, and then submit a patch for 2.6.
> > 
> > Oops... I thought you already committed this.  So I haven't actually
> > tested it yet - but you probably knew that.  If you pass a patch to
> > the list first I'll give it a go.
> 
> My fault, I tested it yesterday but had to go before I could commit it.
> Now this is done.
> 
> It works OK for me (as99127f, w83781d, w83782d).
> 
> Could you (and everyone who can) test the changes? I'd like to ensure it
> doesn't break anything. I suggest we wait some times before submitting
> the changes for inclusion into the 2.6 kernel (it is in no way a
> critical fix).

It works OK on ASB100 Bach as well.

> > > 1* Leave everything unchanged.
> > > 2* Limit AS199237F and ASB100 to addres 0x2d.
> > > 3* Leave the whole 0x28-0x2f range possible but give the chips a
> > > lower confidence if address isn't 0x2d.
> > > 
> > > I'm in favor of solution 3.
> > 
> > I wouldn't mind limiting ASB100 to 0x2d, actually.  I don't recall
> > ever seeing it anywhere else - but if it is, we'll find out soon
> > enough. ;) Or choice 3 is fine too.
> 
> We can have slightly different strategies in sensors-detect and the
> drivers themselves. I will apply solution 3 to sensors-detect. Feel free
> to accept only address 0x2d for your Bach-only driver.

OK, I'll probably do that.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com