* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
@ 2007-02-04 17:43 ` Jean Delvare
2007-02-05 13:12 ` Alan Clucas
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-04 17:43 UTC (permalink / raw)
To: lm-sensors
Hi Alan,
On Mon, 29 Jan 2007 11:58:00 +0000, Alan Clucas wrote:
> We have a DS75 on a custom board, which is attached to a Compulab iGLX
> AMD Geode based board - using the CS5536 and an unmodified scx200_acb
> driver to access the smbus/i2c.
>
> On this board is a DS75. It doesn't identify as the chip correctly with
> the lm75 driver in 2.6.19.2 due to the fact that it seems to return the
> last value read for all of its out of spec commands, rather than doing
> the address cycling which the driver expects for command >8 - e.g.
> address 8 behaves like addresses 4 through 7. (I can provide a debug log
> showing this if needed).
Yes, I'd appreciate a debug log and/or output of i2cdump for a Dallas
DS75 chip.
> With the following patch it identifies correctly and works as expected.
>
> --- linux-2.6.19.2/drivers/hwmon/lm75.c 2007-01-24 16:03:42.000000000 +0000
> +++ linux/drivers/hwmon/lm75.c 2007-01-29 11:41:45.000000000 +0000
> @@ -181,13 +181,6 @@
> /* Unused bits */
> if (conf & 0xe0)
> goto exit_free;
> -
> - /* Addresses cycling */
> - for (i = 8; i < 0xff; i += 8)
> - if (i2c_smbus_read_byte_data(new_client, i + 1)
> != conf
> - || i2c_smbus_read_word_data(new_client, i + 2)
> != hyst
> - || i2c_smbus_read_word_data(new_client, i + 3)
> != os)
> - goto exit_free;
> }
>
> /* Determine the chip type - only one kind supported! */
Your email client is wrapping the long lines so I couldn't apply the
patch above even if I wanted.
I'm not fine with this approach anyway. I fear that plain dropping the
check above will again cause non-compatible chips to be attached to the
lm75 driver. I'd rather leave the LM75 detection code as is and add
code to explicitely detect the DS75, even though we can have both
kinds advertised as "lm75" for compatibility reasons. What do you think?
Can you use perl on your system? I'd like to add detection for the DS75
to sensors-detect first, and when it's ready, port the code to the lm75
driver.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
2007-02-04 17:43 ` Jean Delvare
@ 2007-02-05 13:12 ` Alan Clucas
2007-02-05 17:20 ` Jean Delvare
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Alan Clucas @ 2007-02-05 13:12 UTC (permalink / raw)
To: lm-sensors
> Yes, I'd appreciate a debug log and/or output of i2cdump for a Dallas
> DS75 chip.
Hi Jean, this is the output I get for the device:
localhost ~ # i2cdump -y 0 0x48
No size specified (using byte-data access)
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 50 30 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P0KPPPPPPPPPPPPP
10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
20: XX XX 4b XX XX XX XX XX XX XX XX XX XX XX XX XX XXKXXXXXXXXXXXXX
30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
40: 50 00 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P.KPPPPPPPPPPPPP
50: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
60: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
70: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
80: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
90: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
a0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
b0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
c0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
d0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
e0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX 50 XX XXXXXXXXXXXXXXPX
f0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
localhost ~ # i2cdump -y 0 0x48 w
0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f
00: 8050 0030 004b 0050 0050 0050 0050 0050
08: 0050 0050 0050 0050 0050 0050 0050 0050
10: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
18: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
20: XXXX XXXX 004b XXXX XXXX XXXX XXXX XXXX
28: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
30: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
38: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
40: 8050 0000 004b 0050 0050 0050 0050 0050
48: 0050 0050 0050 0050 0050 0050 0050 0050
50: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
58: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
60: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
68: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
70: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
78: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
80: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
88: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
90: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
98: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
a0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
a8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
b0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
b8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
c0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
c8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
d0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
d8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
e0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
e8: XXXX XXXX XXXX XXXX XXXX XXXX 0050 XXXX
f0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
f8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
If there are any other particular parameters to i2cdump you'd like me to
try I'll happily do so.
> Your email client is wrapping the long lines so I couldn't apply the
> patch above even if I wanted.
Argh, does the attached one work for you (just testing, not expecting
you to apply it) and comply with guidelines - I'll see it for myself
when it arrives back from the list.
> I'm not fine with this approach anyway. I fear that plain dropping the
> check above will again cause non-compatible chips to be attached to the
> lm75 driver. I'd rather leave the LM75 detection code as is and add
> code to explicitely detect the DS75, even though we can have both
> kinds advertised as "lm75" for compatibility reasons. What do you think?
Only if we can find something else which distinguishes the device
correctly, otherwise there isn't much point as it'll detect
non-compatible chips as DS75s doing exactly the same thing anyway. The
patch which added this address cycling DID add some other tests which
the DS75 passes. Leaving it reported as an lm75 makes sense though.
> Can you use perl on your system? I'd like to add detection for the DS75
> to sensors-detect first, and when it's ready, port the code to the lm75
> driver.
Yes, I have perl. sensors-detect currently does not detect it as an
lm75, agreeing with the unpatched kernel.
We only have a couple of fully functional boards, and I may break stuff
or need to run other long term tests, so turnaround on testing may be
intermittent.
Thanks,
Alan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lm75.patch
Type: text/x-patch
Size: 553 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070205/bf9e75ec/attachment.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alanc.vcf
Type: text/x-vcard
Size: 562 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070205/bf9e75ec/attachment.vcf
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
2007-02-04 17:43 ` Jean Delvare
2007-02-05 13:12 ` Alan Clucas
@ 2007-02-05 17:20 ` Jean Delvare
2007-02-06 10:19 ` Alan Clucas
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-05 17:20 UTC (permalink / raw)
To: lm-sensors
Hi Alan,
On Mon, 05 Feb 2007 13:12:46 +0000, Alan Clucas wrote:
> > Yes, I'd appreciate a debug log and/or output of i2cdump for a Dallas
> > DS75 chip.
>
> Hi Jean, this is the output I get for the device:
>
> localhost ~ # i2cdump -y 0 0x48
> No size specified (using byte-data access)
> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
> 00: 50 30 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P0KPPPPPPPPPPPPP
> 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 20: XX XX 4b XX XX XX XX XX XX XX XX XX XX XX XX XX XXKXXXXXXXXXXXXX
> 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 40: 50 00 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P.KPPPPPPPPPPPPP
> 50: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 60: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 70: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 80: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> 90: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> a0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> b0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> c0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> d0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
> e0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX 50 XX XXXXXXXXXXXXXXPX
> f0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>
> localhost ~ # i2cdump -y 0 0x48 w
> 0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f
> 00: 8050 0030 004b 0050 0050 0050 0050 0050
> 08: 0050 0050 0050 0050 0050 0050 0050 0050
> 10: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 18: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 20: XXXX XXXX 004b XXXX XXXX XXXX XXXX XXXX
> 28: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 30: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 38: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 40: 8050 0000 004b 0050 0050 0050 0050 0050
> 48: 0050 0050 0050 0050 0050 0050 0050 0050
> 50: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 58: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 60: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 68: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 70: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 78: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 80: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 88: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 90: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> 98: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> a0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> a8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> b0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> b8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> c0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> c8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> d0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> d8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> e0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> e8: XXXX XXXX XXXX XXXX XXXX XXXX 0050 XXXX
> f0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
> f8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
Wow. I have to admit it's not exactly what I expected. The fact that
some (seemingly random) unused addresses work while others return an
error instead is rather puzzling. At least this gives us several ways
to detect the DS75.
> If there are any other particular parameters to i2cdump you'd like me to
> try I'll happily do so.
No, the above should be enough.
> > Your email client is wrapping the long lines so I couldn't apply the
> > patch above even if I wanted.
>
> Argh, does the attached one work for you (just testing, not expecting
> you to apply it) and comply with guidelines - I'll see it for myself
> when it arrives back from the list.
Yes, the attached patch worked for me, and I'm fine with patches sent
as text attachements.
> > I'm not fine with this approach anyway. I fear that plain dropping the
> > check above will again cause non-compatible chips to be attached to the
> > lm75 driver. I'd rather leave the LM75 detection code as is and add
> > code to explicitely detect the DS75, even though we can have both
> > kinds advertised as "lm75" for compatibility reasons. What do you think?
>
> Only if we can find something else which distinguishes the device
> correctly, otherwise there isn't much point as it'll detect
> non-compatible chips as DS75s doing exactly the same thing anyway. The
> patch which added this address cycling DID add some other tests which
> the DS75 passes. Leaving it reported as an lm75 makes sense though.
>
> > Can you use perl on your system? I'd like to add detection for the DS75
> > to sensors-detect first, and when it's ready, port the code to the lm75
> > driver.
>
> Yes, I have perl. sensors-detect currently does not detect it as an
> lm75, agreeing with the unpatched kernel.
>
> We only have a couple of fully functional boards, and I may break stuff
> or need to run other long term tests, so turnaround on testing may be
> intermittent.
OK. Attached is a patch against sensors-detect from our SVN repository.
Please give it a try, it should detect your DS75. Compared to the LM75
detection, I removed the cycle-over-8 test as your kernel patch did,
and additionally I'm checking that registers 0x08 to 0x0f behave the
same as 0x04 to 0x07 (which isn't true of the LM75.) If it works OK for
you, we can use the same approach in the lm75 kernel driver. Please
report.
Thanks,
--
Jean Delvare
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sensors-detect-add-DS75-support.patch
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070205/8dd0dd21/attachment-0001.pl
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (2 preceding siblings ...)
2007-02-05 17:20 ` Jean Delvare
@ 2007-02-06 10:19 ` Alan Clucas
2007-02-06 14:45 ` Jean Delvare
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Alan Clucas @ 2007-02-06 10:19 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Alan,
>
> On Mon, 05 Feb 2007 13:12:46 +0000, Alan Clucas wrote:
>>> Yes, I'd appreciate a debug log and/or output of i2cdump for a Dallas
>>> DS75 chip.
>> Hi Jean, this is the output I get for the device:
>>
>> localhost ~ # i2cdump -y 0 0x48
>> No size specified (using byte-data access)
>> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
>> 00: 50 30 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P0KPPPPPPPPPPPPP
>> 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 20: XX XX 4b XX XX XX XX XX XX XX XX XX XX XX XX XX XXKXXXXXXXXXXXXX
>> 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 40: 50 00 4b 50 50 50 50 50 50 50 50 50 50 50 50 50 P.KPPPPPPPPPPPPP
>> 50: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 60: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 70: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 80: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> 90: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> a0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> b0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> c0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> d0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>> e0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX 50 XX XXXXXXXXXXXXXXPX
>> f0: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XXXXXXXXXXXXXXXX
>>
>> localhost ~ # i2cdump -y 0 0x48 w
>> 0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f
>> 00: 8050 0030 004b 0050 0050 0050 0050 0050
>> 08: 0050 0050 0050 0050 0050 0050 0050 0050
>> 10: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 18: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 20: XXXX XXXX 004b XXXX XXXX XXXX XXXX XXXX
>> 28: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 30: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 38: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 40: 8050 0000 004b 0050 0050 0050 0050 0050
>> 48: 0050 0050 0050 0050 0050 0050 0050 0050
>> 50: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 58: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 60: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 68: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 70: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 78: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 80: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 88: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 90: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> 98: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> a0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> a8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> b0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> b8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> c0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> c8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> d0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> d8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> e0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> e8: XXXX XXXX XXXX XXXX XXXX XXXX 0050 XXXX
>> f0: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>> f8: XXXX XXXX XXXX XXXX XXXX XXXX XXXX XXXX
>
> Wow. I have to admit it's not exactly what I expected. The fact that
> some (seemingly random) unused addresses work while others return an
> error instead is rather puzzling. At least this gives us several ways
> to detect the DS75.
I did run it several times and reboot just to check - those "random"
addresses are fixed and always the same.
>>> Can you use perl on your system? I'd like to add detection for the DS75
>>> to sensors-detect first, and when it's ready, port the code to the lm75
>>> driver.
>> Yes, I have perl. sensors-detect currently does not detect it as an
>> lm75, agreeing with the unpatched kernel.
>>
>> We only have a couple of fully functional boards, and I may break stuff
>> or need to run other long term tests, so turnaround on testing may be
>> intermittent.
>
> OK. Attached is a patch against sensors-detect from our SVN repository.
> Please give it a try, it should detect your DS75. Compared to the LM75
> detection, I removed the cycle-over-8 test as your kernel patch did,
> and additionally I'm checking that registers 0x08 to 0x0f behave the
> same as 0x04 to 0x07 (which isn't true of the LM75.) If it works OK for
> you, we can use the same approach in the lm75 kernel driver. Please
> report.
We could also check for the 0x00-0x0f mirror at 0x40-0x4f if we want to
be more sure.
Your sensors detect patch works fine:
Next adapter: CS5535 ACB0 (i2c-0)
Do you want to scan it? (YES/no/selectively): yes
Client found at address 0x48
Probing for `National Semiconductor LM75'... No
Probing for `Dallas Semiconductor DS75'... Success!
(confidence 6, driver `lm75')
Probing for `National Semiconductor LM77'... No
Probing for `Dallas Semiconductor DS1621'... No
Probing for `Maxim MAX6650/MAX6651'... No
Probing for `National Semiconductor LM92'... No
Probing for `National Semiconductor LM76'... No
Probing for `Maxim MAX6633/MAX6634/MAX6635'... No
I'm happy to do the kernel patch for this. Do we want it to continue to
return kind=lm75 as well as name="lm75"; I'm not sure of all the
implications of kind.
Thanks,
Alan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alanc.vcf
Type: text/x-vcard
Size: 562 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070206/2ea06550/attachment.vcf
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (3 preceding siblings ...)
2007-02-06 10:19 ` Alan Clucas
@ 2007-02-06 14:45 ` Jean Delvare
2007-02-07 10:14 ` Alan Clucas
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-06 14:45 UTC (permalink / raw)
To: lm-sensors
Hi Alan,
On Tue, 06 Feb 2007 10:19:21 +0000, Alan Clucas wrote:
> > Wow. I have to admit it's not exactly what I expected. The fact that
> > some (seemingly random) unused addresses work while others return an
> > error instead is rather puzzling. At least this gives us several ways
> > to detect the DS75.
>
> I did run it several times and reboot just to check - those "random"
> addresses are fixed and always the same.
OK, thanks for checking this, I was wondering. I'm also curious to know
whether other DS75 chips behave exactly the same or if it's
chip-specific.
> We could also check for the 0x00-0x0f mirror at 0x40-0x4f if we want to
> be more sure.
Correct, but we also want to find the best reliability/speed tradeoff. I
expect my code will to be reliable enough so I'd rather avoid slowing
down detection unless this is really needed.
> Your sensors detect patch works fine:
>
> Next adapter: CS5535 ACB0 (i2c-0)
> Do you want to scan it? (YES/no/selectively): yes
> Client found at address 0x48
> Probing for `National Semiconductor LM75'... No
> Probing for `Dallas Semiconductor DS75'... Success!
> (confidence 6, driver `lm75')
> Probing for `National Semiconductor LM77'... No
> Probing for `Dallas Semiconductor DS1621'... No
> Probing for `Maxim MAX6650/MAX6651'... No
> Probing for `National Semiconductor LM92'... No
> Probing for `National Semiconductor LM76'... No
> Probing for `Maxim MAX6633/MAX6634/MAX6635'... No
OK, great, thanks for testing. I'll apply the patch to SVN soon.
> I'm happy to do the kernel patch for this. Do we want it to continue to
> return kind=lm75 as well as name="lm75"; I'm not sure of all the
> implications of kind.
For now libsensors needs to know about all names, otherwise it doesn't
recognizes the chip features. If we decide to have a separate prefix
for the DS75, this means we must update libsensors as well (that's a
trivial change but it must be done.) Both approaches make sense, so
it's really up to you, whether you prefer a nice separate prefix or
compatibility with older versions of libsensors. Note that if you ever
want to take benefit of the additional resolution bits of the DS75,
having a separate prefix will be useful.
I'm waiting for your patch, if you're quick enough it can make it into
kernel 2.6.21.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (4 preceding siblings ...)
2007-02-06 14:45 ` Jean Delvare
@ 2007-02-07 10:14 ` Alan Clucas
2007-02-09 9:39 ` Jean Delvare
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Alan Clucas @ 2007-02-07 10:14 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Alan,
>
> On Tue, 06 Feb 2007 10:19:21 +0000, Alan Clucas wrote:
>>> Wow. I have to admit it's not exactly what I expected. The fact that
>>> some (seemingly random) unused addresses work while others return an
>>> error instead is rather puzzling. At least this gives us several ways
>>> to detect the DS75.
>> I did run it several times and reboot just to check - those "random"
>> addresses are fixed and always the same.
>
> OK, thanks for checking this, I was wondering. I'm also curious to know
> whether other DS75 chips behave exactly the same or if it's
> chip-specific.
I've tested it on another chip from another batch with exactly the same
random addresses (output varied as the temperature of the board was
different, but otherwise looked the same). Once we have more running
systems I'll do some more tests.
>> Your sensors detect patch works fine:
>>
>> Next adapter: CS5535 ACB0 (i2c-0)
>> Do you want to scan it? (YES/no/selectively): yes
>> Client found at address 0x48
>> Probing for `National Semiconductor LM75'... No
>> Probing for `Dallas Semiconductor DS75'... Success!
>> (confidence 6, driver `lm75')
>> Probing for `National Semiconductor LM77'... No
>> Probing for `Dallas Semiconductor DS1621'... No
>> Probing for `Maxim MAX6650/MAX6651'... No
>> Probing for `National Semiconductor LM92'... No
>> Probing for `National Semiconductor LM76'... No
>> Probing for `Maxim MAX6633/MAX6634/MAX6635'... No
>
> OK, great, thanks for testing. I'll apply the patch to SVN soon.
>
>> I'm happy to do the kernel patch for this. Do we want it to continue to
>> return kind=lm75 as well as name="lm75"; I'm not sure of all the
>> implications of kind.
>
> For now libsensors needs to know about all names, otherwise it doesn't
> recognizes the chip features. If we decide to have a separate prefix
> for the DS75, this means we must update libsensors as well (that's a
> trivial change but it must be done.) Both approaches make sense, so
> it's really up to you, whether you prefer a nice separate prefix or
> compatibility with older versions of libsensors. Note that if you ever
> want to take benefit of the additional resolution bits of the DS75,
> having a separate prefix will be useful.
>
> I'm waiting for your patch, if you're quick enough it can make it into
> kernel 2.6.21.
Attached is the kernel patch and a patch for libsensors as well. The
libsensors patch does not include your sensors-detect patch.
I have changed the detected name to ds75, hence the libsensors patch. I
haven't managed to find any lm75 devices here, so I haven't tested this
on one of those, which would be good before it goes too far. The patch
is simple enough, but you never can tell.
Feedback appreciated.
Thanks,
Alan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lm-sensors-ds75-support.patch
Type: text/x-patch
Size: 1507 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070207/1e897e96/attachment.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kernel-ds75-support.patch
Type: text/x-patch
Size: 3374 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070207/1e897e96/attachment-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alanc.vcf
Type: text/x-vcard
Size: 562 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070207/1e897e96/attachment.vcf
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (5 preceding siblings ...)
2007-02-07 10:14 ` Alan Clucas
@ 2007-02-09 9:39 ` Jean Delvare
2007-02-15 10:47 ` Jean Delvare
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-09 9:39 UTC (permalink / raw)
To: lm-sensors
Hi Alan,
On Wed, 07 Feb 2007 10:14:36 +0000, Alan Clucas wrote:
> Attached is the kernel patch and a patch for libsensors as well. The
> libsensors patch does not include your sensors-detect patch.
Could you please also include the required changes to the sensors
program and possibly sensord as well? These should be trivial.
> I have changed the detected name to ds75, hence the libsensors patch. I
> haven't managed to find any lm75 devices here, so I haven't tested this
> on one of those, which would be good before it goes too far. The patch
> is simple enough, but you never can tell.
I have an LM75, I just tested and it works OK.
Comments on your kernel patch:
> diff -ur linux-2.6.19.2/drivers/hwmon/lm75.c linux/drivers/hwmon/lm75.c
> --- linux-2.6.19.2/drivers/hwmon/lm75.c 2007-02-06 15:53:57.000000000 +0000
> +++ linux/drivers/hwmon/lm75.c 2007-02-06 17:45:19.000000000 +0000
> @@ -34,7 +34,7 @@
> 0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
>
> /* Insmod parameters */
> -I2C_CLIENT_INSMOD_1(lm75);
> +I2C_CLIENT_INSMOD_2(lm75, ds75);
>
> /* Many LM75 constants specified below */
>
> @@ -153,50 +153,63 @@
> new_client->flags = 0;
>
> /* Now, we do the remaining detection. There is no identification-
> - dedicated register so we have to rely on several tricks:
> - unused bits, registers cycling over 8-address boundaries,
> - addresses 0x04-0x07 returning the last read value.
> + dedicated register so we have to rely on several tricks.
> + The LM75 and DS75 share unused bits. The LM75 has registers
> + cycling over 8-address boundaries, and addresses 0x04-0x07
> + returning the last read value.
> + The DS75 has addresses 0x04-0x0f returning the last read value,
> + but does not register cycle.
> The cycling+unused addresses combination is not tested,
> since it would significantly slow the detection down and would
> hardly add any value. */
> if (kind < 0) {
> - int cur, conf, hyst, os;
> + int cur, conf, hyst, os, addr;
>
> /* Unused addresses */
> cur = i2c_smbus_read_word_data(new_client, 0);
> conf = i2c_smbus_read_byte_data(new_client, 1);
> hyst = i2c_smbus_read_word_data(new_client, 2);
> - if (i2c_smbus_read_word_data(new_client, 4) != hyst
> - || i2c_smbus_read_word_data(new_client, 5) != hyst
> - || i2c_smbus_read_word_data(new_client, 6) != hyst
> - || i2c_smbus_read_word_data(new_client, 7) != hyst)
> - goto exit_free;
> +
> + for (addr = 0x04; addr <= 0x0f; addr++)
> + if (i2c_smbus_read_word_data(new_client, addr) != hyst)
> + break;
> +
> + if (addr < 0x08)
> + goto exit_free;
> + if (addr = 0x10)
> + kind = ds75;
> + else
> + kind = lm75;
> +
> os = i2c_smbus_read_word_data(new_client, 3);
> - if (i2c_smbus_read_word_data(new_client, 4) != os
> - || i2c_smbus_read_word_data(new_client, 5) != os
> - || i2c_smbus_read_word_data(new_client, 6) != os
> - || i2c_smbus_read_word_data(new_client, 7) != os)
> - goto exit_free;
> + for (addr = 0x04; addr <= 0x0f; addr++)
> + if (i2c_smbus_read_word_data(new_client, addr) != os)
> + break;
> +
> + if (addr < (kind = ds75 ? 0x10 : 0x08))
> + goto exit_free;
It's smart :) I admit I had no clear idea how to implement the
detection efficiently. I like your approach.
>
> /* Unused bits */
> if (conf & 0xe0)
> goto exit_free;
Note that this requires the DS75 to be in 9-bit resolution mode. The
dump you provided suggests your DS75 is in 10-bit resolution mode, so I
expect it not to be detected properly. Maybe this is OK as a first
approximation, but you may want to submit a second patch to properly
support the high resolution modes of the DS75.
>
> - /* Addresses cycling */
> - for (i = 8; i < 0xff; i += 8)
> - if (i2c_smbus_read_byte_data(new_client, i + 1) != conf
> - || i2c_smbus_read_word_data(new_client, i + 2) != hyst
> - || i2c_smbus_read_word_data(new_client, i + 3) != os)
> - goto exit_free;
> - }
> + /* Addresses cycling - LM75 only*/
Missing space before end of comment.
> + if (kind = lm75)
> + for (i = 8; i < 0xff; i += 8)
> + if (i2c_smbus_read_byte_data(new_client, i + 1) != conf
> + || i2c_smbus_read_word_data(new_client, i + 2) != hyst
> + || i2c_smbus_read_word_data(new_client, i + 3) != os)
> + goto exit_free;
I strongly suggest adding a pair of curly braces for the outer-most if
statement. Three nested statements without any curly braces is asking
for trouble on any further change.
>
> - /* Determine the chip type - only one kind supported! */
> - if (kind <= 0)
> - kind = lm75;
> + }
This change breaks the case kind = 0, you'll end up with an empty
name. You need to force the type to one of the supported ones if the
user uses force without a kind.
>
> + /* Determine the chip type */
> if (kind = lm75) {
> name = "lm75";
> }
> + else if (kind = ds75) {
> + name = "ds75";
> + }
>
> /* Fill in the remaining client fields and put it into the global list */
> strlcpy(new_client->name, name, I2C_NAME_SIZE);
>
Can you also please update Documentation/hwmon/lm75 to mention the
separate prefix?
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (6 preceding siblings ...)
2007-02-09 9:39 ` Jean Delvare
@ 2007-02-15 10:47 ` Jean Delvare
2007-02-15 11:01 ` Alan Clucas
2007-02-15 16:28 ` Jean Delvare
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-15 10:47 UTC (permalink / raw)
To: lm-sensors
> > OK. Attached is a patch against sensors-detect from our SVN repository.
> > Please give it a try, it should detect your DS75. Compared to the LM75
> > detection, I removed the cycle-over-8 test as your kernel patch did,
> > and additionally I'm checking that registers 0x08 to 0x0f behave the
> > same as 0x04 to 0x07 (which isn't true of the LM75.) If it works OK for
> > you, we can use the same approach in the lm75 kernel driver. Please
> > report.
> (...)
> Your sensors detect patch works fine:
OK, patch applied to lm-sensors SVN.
I'm still waiting for updated kernel and libsensors/sensors patches.
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (7 preceding siblings ...)
2007-02-15 10:47 ` Jean Delvare
@ 2007-02-15 11:01 ` Alan Clucas
2007-02-15 16:28 ` Jean Delvare
9 siblings, 0 replies; 11+ messages in thread
From: Alan Clucas @ 2007-02-15 11:01 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
>>> OK. Attached is a patch against sensors-detect from our SVN repository.
>>> Please give it a try, it should detect your DS75. Compared to the LM75
>>> detection, I removed the cycle-over-8 test as your kernel patch did,
>>> and additionally I'm checking that registers 0x08 to 0x0f behave the
>>> same as 0x04 to 0x07 (which isn't true of the LM75.) If it works OK for
>>> you, we can use the same approach in the lm75 kernel driver. Please
>>> report.
>> (...)
>> Your sensors detect patch works fine:
>
> OK, patch applied to lm-sensors SVN.
>
> I'm still waiting for updated kernel and libsensors/sensors patches.
It is on my todo list still - been side tracked by some other things so
don't expect it until next week. Sorry.
I'm not going to try for >9 bit precision. An interesting side effect of
running i2cdump is to change it from 9 bit precision (which it shows it
is setup for the first time it is run after a reset) to the dump that
you saw in my earlier mail. Odd. (Which is why the kernel patch I posted
earlier did work for me).
Alan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alanc.vcf
Type: text/x-vcard
Size: 562 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070215/49f271af/attachment.vcf
^ permalink raw reply [flat|nested] 11+ messages in thread* [lm-sensors] DS75
2007-01-29 11:58 [lm-sensors] DS75 Alan Clucas
` (8 preceding siblings ...)
2007-02-15 11:01 ` Alan Clucas
@ 2007-02-15 16:28 ` Jean Delvare
9 siblings, 0 replies; 11+ messages in thread
From: Jean Delvare @ 2007-02-15 16:28 UTC (permalink / raw)
To: lm-sensors
Alan,
On Thu, 15 Feb 2007 11:01:17 +0000, Alan Clucas wrote:
> Jean Delvare wrote:
> > OK, patch applied to lm-sensors SVN.
> >
> > I'm still waiting for updated kernel and libsensors/sensors patches.
>
> It is on my todo list still - been side tracked by some other things so
> don't expect it until next week. Sorry.
No worry, we're in no hurry.
> I'm not going to try for >9 bit precision. An interesting side effect of
> running i2cdump is to change it from 9 bit precision (which it shows it
> is setup for the first time it is run after a reset) to the dump that
> you saw in my earlier mail. Odd. (Which is why the kernel patch I posted
> earlier did work for me).
This is really weird. I just can't explain it. Does my new code in
sensors-detect produce the same side effect?
--
Jean Delvare
^ permalink raw reply [flat|nested] 11+ messages in thread