* i2c-i801: Regression between 2.6.19-1 & 2.6.23.9
@ 2008-01-09 11:08 Ivo Manca
[not found] ` <dba8564e0801090308j34215f98rd758dff3702b194-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Ivo Manca @ 2008-01-09 11:08 UTC (permalink / raw)
To: i2c-GZX6beZjE8VD60Wz+7aTrA
Hey all,
Trying to test interrupt support for i801 I learned that somehow
between kernel version 2.6.22.14 and 2.6.23.9, something went wrong
with the bus driver. Is this a known issue?
When I try to use i2cdump to block-read an EEP OM, my i2cbus hangs.
Please find my output for 2.6.19-1 (working), 2.6.22.14-72 (working)
and 2.6.23.9-85 (not working).
Note: SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R)
Ivo
[root@localhost ~]# uname -r
2.6.19-1.2911.fc6
[root@localhost ~]# modprobe i2c-dev
[root@localhost ~]# i2cdetect -y 0
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: XX XX XX XX XX 08 XX XX XX XX XX XX XX
10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX
50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX
60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX
70: XX XX XX 73 XX XX XX XX
[root@localhost ~]# i2cdump -y 0 0x55 s
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.??
10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u
[root@localhost ~]# i2cdump -y 0 0x55 s
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.??
10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u
====
[root@localhost ~]# uname -r
2.6.22.14-72.fc6
[root@localhost ~]# modprobe i2c-dev
[root@localhost ~]# i2cdetect -y 0
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: XX XX XX XX XX 08 XX XX XX XX XX XX XX
10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX
40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX
50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX
60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX
70: XX XX XX 73 XX XX XX XX
[root@localhost ~]# i2cdump -y 0 0x55 s
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.??
10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u
[root@localhost ~]# i2cdump -y 0 0x55 s
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.??
10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u
====
[root@localhost ~]# uname -r
2.6.23.9-85.fc8
[root@localhost ~]# modprobe i2c-dev
[root@localhost ~]# i2cdetect -y 0
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- 08 -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- 44 -- -- -- -- -- -- -- -- -- -- --
50: 50 -- -- -- 54 55 -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- 69 -- -- -- -- -- --
70: -- -- -- 73 -- -- -- --
[root@localhost ~]# i2cdump -y 0 0x55 s
Error: Block read failed, return code -1
[root@localhost ~]# i2cdump -y 0 0x55 s
Error: Block read failed, return code -1
(i2c Bus hangs completely, poweroff needed.)
^ permalink raw reply [flat|nested] 7+ messages in thread[parent not found: <dba8564e0801090308j34215f98rd758dff3702b194-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: i2c-i801: Regression between 2.6.19-1 & 2.6.23.9 [not found] ` <dba8564e0801090308j34215f98rd758dff3702b194-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-01-09 12:53 ` Jean Delvare [not found] ` <20080109135341.461688d1-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2008-01-09 12:53 UTC (permalink / raw) To: Ivo Manca; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA Hi Ivo, On Wed, 9 Jan 2008 12:08:28 +0100, Ivo Manca wrote: > Trying to test interrupt support for i801 I learned that somehow > between kernel version 2.6.22.14 and 2.6.23.9, something went wrong > with the bus driver. Is this a known issue? No, you are the first person to report this problem as far as I know. > When I try to use i2cdump to block-read an EEP OM, my i2cbus hangs. What is an "EEP OM"? Please note that EEPROMs typically want I2C block reads (mode "i" of i2cdump) and not SMBus block reads. Using the wrong mode shouldn't hang the bus though. > Please find my output for 2.6.19-1 (working), 2.6.22.14-72 (working) > and 2.6.23.9-85 (not working). > > Note: SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R) > > Ivo > > [root@localhost ~]# uname -r > 2.6.19-1.2911.fc6 > > [root@localhost ~]# modprobe i2c-dev > [root@localhost ~]# i2cdetect -y 0 > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: XX XX XX XX XX 08 XX XX XX XX XX XX XX > 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX > 50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX > 60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX > 70: XX XX XX 73 XX XX XX XX > > [root@localhost ~]# i2cdump -y 0 0x55 s > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? > 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u > > [root@localhost ~]# i2cdump -y 0 0x55 s > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? > 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u > > ==== > > [root@localhost ~]# uname -r > 2.6.22.14-72.fc6 > > [root@localhost ~]# modprobe i2c-dev > > [root@localhost ~]# i2cdetect -y 0 > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: XX XX XX XX XX 08 XX XX XX XX XX XX XX > 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX > 40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX > 50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX > 60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX > 70: XX XX XX 73 XX XX XX XX > > [root@localhost ~]# i2cdump -y 0 0x55 s > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? > 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u > > [root@localhost ~]# i2cdump -y 0 0x55 s > 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef > 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? > 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u So 2.6.22 works as good as 2.6.19 did, right? > > ==== > > [root@localhost ~]# uname -r > 2.6.23.9-85.fc8 > > [root@localhost ~]# modprobe i2c-dev > > [root@localhost ~]# i2cdetect -y 0 > 0 1 2 3 4 5 6 7 8 9 a b c d e f > 00: -- -- -- -- -- 08 -- -- -- -- -- -- -- > 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- > 40: -- -- -- -- 44 -- -- -- -- -- -- -- -- -- -- -- > 50: 50 -- -- -- 54 55 -- -- -- -- -- -- -- -- -- -- > 60: -- -- -- -- -- -- -- -- -- 69 -- -- -- -- -- -- > 70: -- -- -- 73 -- -- -- -- I note that this version of i2cdetect is more recent than the one you used when testing 2.6.19-1.2911.fc6 and 2.6.22.14-72.fc6 kernels. I suppose that this is the case for i2cdump as well... To make sure that this is really an i2c-i801 driver issue and not a user-space issue, can you please try again using the exact same version of i2cdetect and i2cdump? > > [root@localhost ~]# i2cdump -y 0 0x55 s > Error: Block read failed, return code -1 > > [root@localhost ~]# i2cdump -y 0 0x55 s > Error: Block read failed, return code -1 > > (i2c Bus hangs completely, poweroff needed.) Note that i2cdump relies on i2c-dev, so the problem could be in i2c-dev. There were changes to i2c-dev between 2.6.22 and 2.6.23, that affect I2C block reads, but not SMBus block reads. There were two large patches applied to i2c-i801: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ca8b9e32a11a7cbfecbef00c8451a79fe1af392e http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7edcb9abb594a8f3b4ca756e03d01c870aeae127 You may also want to check what fedora-specific patches your kernels include for i2c-i801 and i2c-dev, if any. Can you reproduce the problem with a vanilla 2.6.23.12 kernel? On my side I'll check if I can reproduce the problem on one of my test systems. I don't test SMBus block reads very often so I could have missed it. -- Jean Delvare ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20080109135341.461688d1-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>]
* Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9 [not found] ` <20080109135341.461688d1-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> @ 2008-01-09 13:47 ` Ivo Manca [not found] ` <4784D068.8080401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Ivo Manca @ 2008-01-09 13:47 UTC (permalink / raw) To: Jean Delvare; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA, Hans de Goede Hey Jean, Jean Delvare wrote: > Hi Ivo, > > On Wed, 9 Jan 2008 12:08:28 +0100, Ivo Manca wrote: > >> Trying to test interrupt support for i801 I learned that somehow >> between kernel version 2.6.22.14 and 2.6.23.9, something went wrong >> with the bus driver. Is this a known issue? >> > > No, you are the first person to report this problem as far as I know. > > >> When I try to use i2cdump to block-read an EEP OM, my i2cbus hangs. >> > > What is an "EEP OM"? > Sorry, that should read "EEPROM" ofcourse. > Please note that EEPROMs typically want I2C block reads (mode "i" of > i2cdump) and not SMBus block reads. Using the wrong mode shouldn't hang > the bus though. > I know; however, i2cdump states my bus doesn't have i2c block read capabilities. That's why I used SMBus block reads, which seemed to work properly. >> Please find my output for 2.6.19-1 (working), 2.6.22.14-72 (working) >> and 2.6.23.9-85 (not working). >> >> Note: SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R) >> >> Ivo >> >> [root@localhost ~]# uname -r >> 2.6.19-1.2911.fc6 >> >> [root@localhost ~]# modprobe i2c-dev >> [root@localhost ~]# i2cdetect -y 0 >> 0 1 2 3 4 5 6 7 8 9 a b c d e f >> 00: XX XX XX XX XX 08 XX XX XX XX XX XX XX >> 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX >> 50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX >> 60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX >> 70: XX XX XX 73 XX XX XX XX >> >> [root@localhost ~]# i2cdump -y 0 0x55 s >> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef >> 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? >> 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u >> >> [root@localhost ~]# i2cdump -y 0 0x55 s >> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef >> 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? >> 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u >> >> ==== >> >> [root@localhost ~]# uname -r >> 2.6.22.14-72.fc6 >> >> [root@localhost ~]# modprobe i2c-dev >> >> [root@localhost ~]# i2cdetect -y 0 >> 0 1 2 3 4 5 6 7 8 9 a b c d e f >> 00: XX XX XX XX XX 08 XX XX XX XX XX XX XX >> 10: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 20: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 30: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX >> 40: XX XX XX XX 44 XX XX XX XX XX XX XX XX XX XX XX >> 50: 50 XX XX XX 54 55 XX XX XX XX XX XX XX XX XX XX >> 60: XX XX XX XX XX XX XX XX XX 69 XX XX XX XX XX XX >> 70: XX XX XX 73 XX XX XX XX >> >> [root@localhost ~]# i2cdump -y 0 0x55 s >> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef >> 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? >> 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u >> >> [root@localhost ~]# i2cdump -y 0 0x55 s >> 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef >> 00: 08 07 0d 0a 01 40 00 04 60 70 00 82 08 00 01 0e ?????@.?`p.??.?? >> 10: 04 0c 01 02 20 c0 75 70 00 00 48 30 48 2a 40 75 ???? ?up..H0H*@u >> > > So 2.6.22 works as good as 2.6.19 did, right? > Correct. I had to test this at university, since I don't have to proper hardware at home. Because they machine's there are running FC6 with 2.6.19 kernels, I just wanted to be sure the problem didn't start between 2.6.19 and 2.6.22.9. Being in a hurry, I forgot to change the subject of this mail (and of course, only noticed just after pushing the send button). >> ==== >> >> [root@localhost ~]# uname -r >> 2.6.23.9-85.fc8 >> >> [root@localhost ~]# modprobe i2c-dev >> >> [root@localhost ~]# i2cdetect -y 0 >> 0 1 2 3 4 5 6 7 8 9 a b c d e f >> 00: -- -- -- -- -- 08 -- -- -- -- -- -- -- >> 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- >> 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- >> 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- >> 40: -- -- -- -- 44 -- -- -- -- -- -- -- -- -- -- -- >> 50: 50 -- -- -- 54 55 -- -- -- -- -- -- -- -- -- -- >> 60: -- -- -- -- -- -- -- -- -- 69 -- -- -- -- -- -- >> 70: -- -- -- 73 -- -- -- -- >> > > I note that this version of i2cdetect is more recent than the one you > used when testing 2.6.19-1.2911.fc6 and 2.6.22.14-72.fc6 kernels. I > suppose that this is the case for i2cdump as well... To make sure that > this is really an i2c-i801 driver issue and not a user-space issue, can > you please try again using the exact same version of i2cdetect and > i2cdump? > True. I used an old version of the fedora lm_sensors package. I updated both installations to the newest version available @ ATrpms.net. This did not make any difference. >> [root@localhost ~]# i2cdump -y 0 0x55 s >> Error: Block read failed, return code -1 >> >> [root@localhost ~]# i2cdump -y 0 0x55 s >> Error: Block read failed, return code -1 >> >> (i2c Bus hangs completely, poweroff needed.) >> > > Note that i2cdump relies on i2c-dev, so the problem could be in > i2c-dev. There were changes to i2c-dev between 2.6.22 and 2.6.23, that > affect I2C block reads, but not SMBus block reads. Not sure if it helps, but the 2.6.23.9 i2c_i801 module loaded on a 2.6.22.9 kernel gave the same error message and problems. Yes: I'm aware that I'm not supposed to test modules like that ;) > There were two large > patches applied to i2c-i801: > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ca8b9e32a11a7cbfecbef00c8451a79fe1af392e > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7edcb9abb594a8f3b4ca756e03d01c870aeae127 > I already noticed these patches while trying to convert my patch, which adds interrupt support, to 2.6.23.9. I was happy to see them, since I was always curious why defines were not used for statusses and why "/* set 32 byte buffer */" was there without doing anything ;) Since I was using an ICH5/ICH5R, which is not listed in the switch statement at the probe function, I should be defaulting to isich = 0 and therefor using i801_block_transaction_byte_by_byte. I'll look into the exact changes made here. > You may also want to check what fedora-specific patches your kernels > include for i2c-i801 and i2c-dev, if any. Can you reproduce the problem > with a vanilla 2.6.23.12 kernel? > > On my side I'll check if I can reproduce the problem on one of my test > systems. I don't test SMBus block reads very often so I could have > missed it. > I'll check with Hans for fedora specific patches and install a vanilla kernel as soon as time permits; I hope tomorrow. Thanks, Ivo ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <4784D068.8080401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9 [not found] ` <4784D068.8080401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2008-01-09 14:42 ` Jean Delvare [not found] ` <20080109154216.3cec6053-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2008-01-09 14:42 UTC (permalink / raw) To: Ivo Manca; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA, Hans de Goede On Wed, 09 Jan 2008 14:47:20 +0100, Ivo Manca wrote: > Jean Delvare wrote: > > What is an "EEP OM"? > > Sorry, that should read "EEPROM" ofcourse. Err, of course. I'm not very smart today it seems... > > Please note that EEPROMs typically want I2C block reads (mode "i" of > > i2cdump) and not SMBus block reads. Using the wrong mode shouldn't hang > > the bus though. > > I know; however, i2cdump states my bus doesn't have i2c block read > capabilities. That's why I used SMBus block reads, which seemed to work > properly. OK. This is an important fact when tracking this regression. I guess that the 1st byte returned by your EEPROM is 0x80, which is NOT a valid SMBus block length. This means that it is expected that an error is returned. It wasn't the case before (the invalid length was adjust to the nearest valid valid length, i.e. 32) but that was changed in 2.6.23 to return an error instead, and I think this is the right thing to do: > if (i == 1 && read_write == I2C_SMBUS_READ) { > len = inb_p(SMBHSTDAT0); > - if (len < 1) > - len = 1; > - if (len > 32) > - len = 32; > + if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) { > + result = -1; > + goto END; > + } > data->block[0] = len; > } So, the fact that you get an error with 2.6.23 when you did not in 2.6.22 is expected, this is not a regression. I get the same error on my test system. The only difference is that it does NOT hang the bus for me. BTW, I2C block read is now supported in -mm: http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-i801-04-add-support-for-i2c-block-read.patch Note that the chip you have at 0x69 is most certainly a clock chip and many clock chip do support the SMBus block read transaction. This is how I test it, maybe you can try this and see if it hangs as well or not. > Since I was using an ICH5/ICH5R, which is not listed in the switch > statement at the probe function, I should be defaulting to isich = 0 and > therefor using i801_block_transaction_byte_by_byte. I'll look into the > exact changes made here. The ICH5 _is_ listed in the switch statement: > case PCI_DEVICE_ID_INTEL_82801EB_3: So it should use i801_block_transaction_by_block(). You might want to test removing this case from the switch statement to force the driver to use i801_block_transaction_byte_by_byte() instead, and see if it makes a difference. > > On my side I'll check if I can reproduce the problem on one of my test > > systems. I don't test SMBus block reads very often so I could have > > missed it. I did that, with an ICH5 as well, I get the error when using an SMBus block read on an EEPROM, as expected, but the bus never hangs. I have no idea why it hangs for you and not for me. If you figure it out, please let me know. -- Jean Delvare ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20080109154216.3cec6053-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>]
* Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9 [not found] ` <20080109154216.3cec6053-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> @ 2008-01-09 18:09 ` Ivo Manca [not found] ` <47850DDB.5080101-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Ivo Manca @ 2008-01-09 18:09 UTC (permalink / raw) To: Jean Delvare; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA, Hans de Goede Jean Delvare wrote: > On Wed, 09 Jan 2008 14:47:20 +0100, Ivo Manca wrote: > >> Jean Delvare wrote: >> >>> What is an "EEP OM"? >>> >> Sorry, that should read "EEPROM" ofcourse. >> > > Err, of course. I'm not very smart today it seems... > > >>> Please note that EEPROMs typically want I2C block reads (mode "i" of >>> i2cdump) and not SMBus block reads. Using the wrong mode shouldn't hang >>> the bus though. >>> >> I know; however, i2cdump states my bus doesn't have i2c block read >> capabilities. That's why I used SMBus block reads, which seemed to work >> properly. >> > > OK. This is an important fact when tracking this regression. I guess > that the 1st byte returned by your EEPROM is 0x80, which is NOT a valid > SMBus block length. This means that it is expected that an error is > returned. It wasn't the case before (the invalid length was adjust to > the nearest valid valid length, i.e. 32) but that was changed in 2.6.23 > to return an error instead, and I think this is the right thing to do: > Ahhhh, seeing the code this way makes perfect sense. Funny, that actual correcting something makes me feel like it's an error. >> if (i == 1 && read_write == I2C_SMBUS_READ) { >> len = inb_p(SMBHSTDAT0); >> - if (len < 1) >> - len = 1; >> - if (len > 32) >> - len = 32; >> + if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) { >> + result = -1; >> + goto END; >> + } >> data->block[0] = len; >> } >> > > So, the fact that you get an error with 2.6.23 when you did not in > 2.6.22 is expected, this is not a regression. I get the same error on > my test system. The only difference is that it does NOT hang the bus > for me. > Thanks for showing me; I agree it's not a regression. However, the bus normally did not hang and now it does, which kinda feels like an awful downside. Maybe I'm just too lazy to unplug the power cord every time I test something ;). > BTW, I2C block read is now supported in -mm: > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-i801-04-add-support-for-i2c-block-read.patch > Some more to apply to my patch then :-). Thanks for noticing me, now I can properly test interrupt support tomorrow. > Note that the chip you have at 0x69 is most certainly a clock chip and > many clock chip do support the SMBus block read transaction. This is > how I test it, maybe you can try this and see if it hangs as well or > not. > > >> Since I was using an ICH5/ICH5R, which is not listed in the switch >> statement at the probe function, I should be defaulting to isich = 0 and >> therefor using i801_block_transaction_byte_by_byte. I'll look into the >> exact changes made here. >> > > The ICH5 _is_ listed in the switch statement: > >> case PCI_DEVICE_ID_INTEL_82801EB_3: >> > Doh. Not my day either. > So it should use i801_block_transaction_by_block(). You might want to > test removing this case from the switch statement to force the driver > to use i801_block_transaction_byte_by_byte() instead, and see if it > makes a difference. > Will try soon. >>> On my side I'll check if I can reproduce the problem on one of my test >>> systems. I don't test SMBus block reads very often so I could have >>> missed it. >>> > > I did that, with an ICH5 as well, I get the error when using an SMBus > block read on an EEPROM, as expected, but the bus never hangs. I have > no idea why it hangs for you and not for me. If you figure it out, > please let me know. > > I'll still have to do quite a lot of testing, so if I stumble acros the caus, I'll let you know. Just a quick update: Interrupt support seemed to work well for both block and byte reads in 2.6.22.9. However, the code was too ugly and full with awful hacks, so I've converted it to a proper patch for both 2.6.22.9 and (later) 2.6.23.9. Since I do not have the proper hardware at home, I had to wait for today & tomorrow to test. I'm very curious whether it will work or not, and especially, how fast it'll be with i2c block reads. Last test results shows: Old driver: (time 25x i2cdump -s) real 0m27.375s user 0m0.008s sys 0m0.023s -- I2C_SMBUS_QUICK(nodev) 0.00222 I2C_SMBUS_QUICK 0.00238 I2C_SMBUS_BYTE 0.00203 I2C_SMBUS_BYTE_DATA 0.00203 I2C_SMBUS_WORD_DATA 0.00216 New driver: (time 25x i2cdump -s) ./bla (i2c-dump 25x) real 0m24.215s user 0m0.013s sys 0m0.175s -- I2C_SMBUS_QUICK(nodev) 0.00112 I2C_SMBUS_QUICK 0.00112 I2C_SMBUS_BYTE 0.00110 I2C_SMBUS_BYTE_DATA 0.00113 I2C_SMBUS_WORD_DATA 0.00108 ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <47850DDB.5080101-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9 [not found] ` <47850DDB.5080101-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2008-01-10 14:09 ` Jean Delvare [not found] ` <20080110150917.646c2677-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2008-01-10 14:09 UTC (permalink / raw) To: Ivo Manca; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA Hi Ivo, On Wed, 09 Jan 2008 19:09:31 +0100, Ivo Manca wrote: > I'll still have to do quite a lot of testing, so if I stumble acros the > caus, I'll let you know. OK, great. > Just a quick update: > Interrupt support seemed to work well for both block and byte reads in > 2.6.22.9. However, the code was too ugly and full with awful hacks, so > I've converted it to a proper patch for both 2.6.22.9 and (later) > 2.6.23.9. Since I do not have the proper hardware at home, I had to wait > for today & tomorrow to test. > > I'm very curious whether it will work or not, and especially, how fast > it'll be with i2c block reads. I am very interested in this as well. I have several ICH3-M, ICH5 and ICH7-based systems here for testing as soon as you have something good enough to be published. For devices which support the block buffer (i.e. ICH4 and later), Oleg's patch improved speed dramatically for block transactions already. I don't think that you will win much with interrupts here, except maybe for HZ=100. I think that you will win a lot more on short transactions, where polled mode has to wait for at least one jiffie (and more like 2 on practice) and interrupt mode could be one or two orders of magnitude faster than this. For hardware monitoring chips with a lot of registers (e.g. LM85 or ADM1026) the speedup should be very visible. > Last test results shows: > > Old driver: > (time 25x i2cdump -s) > real 0m27.375s > user 0m0.008s > sys 0m0.023s > -- > I2C_SMBUS_QUICK(nodev) 0.00222 > I2C_SMBUS_QUICK 0.00238 > I2C_SMBUS_BYTE 0.00203 > I2C_SMBUS_BYTE_DATA 0.00203 > I2C_SMBUS_WORD_DATA 0.00216 What value of HZ are you using? It matters for polled mode. From the figures above I'd guess HZ=1000. Try again with HZ=250 or even HZ=100 if you want more impressive figures ;) > New driver: > (time 25x i2cdump -s) > ./bla (i2c-dump 25x) > real 0m24.215s > user 0m0.013s > sys 0m0.175s > -- > I2C_SMBUS_QUICK(nodev) 0.00112 > I2C_SMBUS_QUICK 0.00112 > I2C_SMBUS_BYTE 0.00110 > I2C_SMBUS_BYTE_DATA 0.00113 > I2C_SMBUS_WORD_DATA 0.00108 So that would be a 2x improvement for short transactions at HZ=1000... and 20x at HZ=100. Very nice :) -- Jean Delvare ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20080110150917.646c2677-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>]
* I2c-i801 interrupt support (was: Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9) [not found] ` <20080110150917.646c2677-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> @ 2008-02-08 16:22 ` Ivo Manca 0 siblings, 0 replies; 7+ messages in thread From: Ivo Manca @ 2008-02-08 16:22 UTC (permalink / raw) To: Jean Delvare; +Cc: Oleg Ryjkov, i2c-GZX6beZjE8VD60Wz+7aTrA, Hans de Goede Hey Jean, Jean Delvare wrote: > Hi Ivo, > > > I am very interested in this as well. I have several ICH3-M, ICH5 and > ICH7-based systems here for testing as soon as you have something good > enough to be published. > > For devices which support the block buffer (i.e. ICH4 and later), > Oleg's patch improved speed dramatically for block transactions > already. I don't think that you will win much with interrupts here, > except maybe for HZ=100. I think that you will win a lot more on short > transactions, where polled mode has to wait for at least one jiffie > (and more like 2 on practice) and interrupt mode could be one or two > orders of magnitude faster than this. For hardware monitoring chips > with a lot of registers (e.g. LM85 or ADM1026) the speedup should be > very visible. > > Sadly I haven't been able to test this improvement due to lack of (available) hardware. The last test showed a bug which crashes the controller but I think I've already fixed it. Luckily, Hans de Goede arranged a spare PC which I can use for testing and development. I hope to pick it up soon and development should speed up then! So, thanks to the HHS (Hague University) for making a system available for testing and thanks to Hans for arranging this! > What value of HZ are you using? It matters for polled mode. From the > figures above I'd guess HZ=1000. Try again with HZ=250 or even HZ=100 > if you want more impressive figures ;) > Default, so 1000 I suppose. >> New driver: >> (time 25x i2cdump -s) >> ./bla (i2c-dump 25x) >> real 0m24.215s >> user 0m0.013s >> sys 0m0.175s >> -- >> I2C_SMBUS_QUICK(nodev) 0.00112 >> I2C_SMBUS_QUICK 0.00112 >> I2C_SMBUS_BYTE 0.00110 >> I2C_SMBUS_BYTE_DATA 0.00113 >> I2C_SMBUS_WORD_DATA 0.00108 >> > > So that would be a 2x improvement for short transactions at HZ=1000... > and 20x at HZ=100. Very nice :) > Seems quite nice yes, let's just hope I/we can get it stable enough! Ivo _______________________________________________ i2c mailing list i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org http://lists.lm-sensors.org/mailman/listinfo/i2c ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-02-08 16:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-09 11:08 i2c-i801: Regression between 2.6.19-1 & 2.6.23.9 Ivo Manca
[not found] ` <dba8564e0801090308j34215f98rd758dff3702b194-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-01-09 12:53 ` Jean Delvare
[not found] ` <20080109135341.461688d1-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-01-09 13:47 ` i2c-i801: Regression between 2.6.22.9 " Ivo Manca
[not found] ` <4784D068.8080401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2008-01-09 14:42 ` Jean Delvare
[not found] ` <20080109154216.3cec6053-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-01-09 18:09 ` Ivo Manca
[not found] ` <47850DDB.5080101-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2008-01-10 14:09 ` Jean Delvare
[not found] ` <20080110150917.646c2677-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-02-08 16:22 ` I2c-i801 interrupt support (was: Re: i2c-i801: Regression between 2.6.22.9 & 2.6.23.9) Ivo Manca
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox