* 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
* 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
* 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
* 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
* 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
* 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
* 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