I2C block reads with i2c-viapro: testers wanted

I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi all,

I am implementing I2C block reads in the i2c-viapro driver, and am
looking for testers. I was able to test on my own VT8237R chip, it works
OK, now I'd need to know how it works on older VIA south bridges, namely
the VT8235 and the VT82C686B. South bridges before that (VT82C686A,
VT8233A and older) are supposed not to work according to the datasheets,
but a confirmation would be welcome, who knows, it might simply not be
documented.

My experimental patch follows. I have enabled the I2C block read
function for all VIA south bridges, so that it can be tested on all
chips. I'll restrict that after the test phase, of course.

The easiest way to test the patch is to use i2c-viapro in conjunction
with the eeprom driver. This supposes that you do actually have a VIA
south bridge with EEPROMs (typically SPD) on the SMBus. If not, you
won't be able to test, sorry.

In order to verify whether I2C block reads work for you, just compare
the contents of this file:
  /sys/bus/i2c/devices/0-0050/eeprom
before and after applying the patch (and cycling i2c-viapro, obviously).
If it works, the contents should be identical. Note that the bus number
(0 above) and exact address (0050 above) may change depending on the
hardware setup.

You can also use lm_sensors' utilities to test the I2C block read
function: i2cdump has an I2C block mode ("i"), and even "sensors" will
display the SPD information. If it's correct after applying the patch,
it means that the I2C block read function is working OK for you.

On my system, the dump is down from over 2 seconds without the patch to
below 0.2 second with the patch, which proves how efficient I2C block
reads are and explains why I want to implement this function.

Thanks.

 drivers/i2c/busses/i2c-viapro.c |   40 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 38 insertions(+), 2 deletions(-)

--- linux-2.6.13-rc6.orig/drivers/i2c/busses/i2c-viapro.c	2005-08-08 18:55:48.000000000 +0200
+++ linux-2.6.13-rc6/drivers/i2c/busses/i2c-viapro.c	2005-08-09 22:52:56.000000000 +0200
@@ -88,6 +88,7 @@
 #define VT596_BYTE_DATA  0x08
 #define VT596_WORD_DATA  0x0C
 #define VT596_BLOCK_DATA 0x14
+#define VT596_I2C_BLOCK_DATA	0x34
 
 
 /* If force is set to anything different from 0, we forcibly enable the
@@ -107,6 +108,9 @@
 
 static struct i2c_adapter vt596_adapter;
 
+#define FEATURE_I2CBLOCK	(1<<0)
+static int vt596_features;
+
 /* Another internally used function */
 static int vt596_transaction(void)
 {
@@ -242,9 +246,21 @@
 		}
 		size = VT596_BLOCK_DATA;
 		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
+		       SMBHSTADD);
+		outb_p(command, SMBHSTCMD);
+		if (read_write == I2C_SMBUS_WRITE) {
+			dev_warn(&vt596_adapter.dev,
+				 "I2C block write not supported!\n");
+			return -1;
+		}
+		outb_p(I2C_SMBUS_BLOCK_MAX, SMBHSTDAT0);
+		size = VT596_I2C_BLOCK_DATA;
+		break;
 	}
 
-	outb_p((size & 0x1C) + (ENABLE_INT9 & 1), SMBHSTCNT);
+	outb_p((size & 0x3C) + (ENABLE_INT9 & 1), SMBHSTCNT);
 
 	if (vt596_transaction()) /* Error in transaction */
 		return -1;
@@ -267,6 +283,7 @@
 		data->word = inb_p(SMBHSTDAT0) + (inb_p(SMBHSTDAT1) << 8);
 		break;
 	case VT596_BLOCK_DATA:
+	case VT596_I2C_BLOCK_DATA:
 		data->block[0] = inb_p(SMBHSTDAT0);
 		if (data->block[0] > I2C_SMBUS_BLOCK_MAX)
 			data->block[0] = I2C_SMBUS_BLOCK_MAX;
@@ -280,9 +297,15 @@
 
 static u32 vt596_func(struct i2c_adapter *adapter)
 {
-	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+	u32 func = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
 	    I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
 	    I2C_FUNC_SMBUS_BLOCK_DATA;
+
+#if 0
+	if (vt596_features & FEATURE_I2CBLOCK)
+#endif
+		func |= I2C_FUNC_SMBUS_READ_I2C_BLOCK;
+	return func;
 }
 
 static struct i2c_algorithm smbus_algorithm = {
@@ -391,6 +414,19 @@
 		vt596_pdev = NULL;
 	}
 
+	if (pdev->device == PCI_DEVICE_ID_VIA_8235
+	 || pdev->device == PCI_DEVICE_ID_VIA_8237) {
+		vt596_features |= FEATURE_I2CBLOCK;
+	} else if (pdev->device == PCI_DEVICE_ID_VIA_82C686_4) {
+		u8 rev;
+
+		/* VT82C686B (rev 0x40) does support I2C block mode, but
+		   VT82C686A (rev 0x30) doesn't. */
+		if (!pci_read_config_byte(pdev, PCI_REVISION_ID, &rev)
+		 && rev >= 0x40)
+			vt596_features |= FEATURE_I2CBLOCK;
+	}
+
 	/* Always return failure here.  This is to allow other drivers to bind
 	 * to this pci device.  We don't really want to have control over the
 	 * pci device, we only wanted to read as few register values from it.


-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Hinko Kocevar]

Jean Delvare wrote:

> The easiest way to test the patch is to use i2c-viapro in conjunction
> with the eeprom driver. This supposes that you do actually have a VIA
> south bridge with EEPROMs (typically SPD) on the SMBus. If not, you
> won't be able to test, sorry.
> 
> In order to verify whether I2C block reads work for you, just compare
> the contents of this file:
>   /sys/bus/i2c/devices/0-0050/eeprom

I've tested your patch on gericom X5 with VIA chipset and it works fine 
without/with your patch (no diff in eeprom contents). Here is the lspci info:

noa linux # lspci
0000:00:00.0 Host bridge: VIA Technologies, Inc. VT8753 [P4X266 AGP] (rev 01)
0000:00:01.0 PCI bridge: VIA Technologies, Inc. VT8633 [Apollo Pro266 AGP]
0000:00:03.0 CardBus bridge: Texas Instruments PCI1410 PC card Cardbus 
Controller (rev 02)
0000:00:07.0 USB Controller: VIA Technologies, Inc. USB (rev 50)
0000:00:07.1 USB Controller: VIA Technologies, Inc. USB (rev 50)
0000:00:07.2 USB Controller: VIA Technologies, Inc. USB 2.0 (rev 51)
0000:00:0a.0 FireWire (IEEE 1394): VIA Technologies, Inc. IEEE 1394 Host 
Controller (rev 46)
0000:00:11.0 ISA bridge: VIA Technologies, Inc. VT8233 PCI to ISA Bridge
0000:00:11.1 IDE interface: VIA Technologies, Inc. VT82C586/B/686A/B PIPC Bus 
Master IDE (rev 06)
0000:00:11.2 USB Controller: VIA Technologies, Inc. USB (rev 23)
0000:00:11.3 USB Controller: VIA Technologies, Inc. USB (rev 23)
0000:00:11.5 Multimedia audio controller: VIA Technologies, Inc. VT8233 AC97 
Audio Controller (rev 30)
0000:00:11.6 Communication controller: VIA Technologies, Inc. Intel 537 [AC97 
Modem] (rev 70)
0000:00:12.0 Ethernet controller: VIA Technologies, Inc. VT6102 [Rhine-II] (rev 70)
0000:01:00.0 VGA compatible controller: nVidia Corporation NV17 [GeForce4 440 
Go 64M] (rev a3)

regards,
hinko k

-- 
..because under Linux "if something is possible in principle,
then it is already implemented or somebody is working on it".

					--LKI

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Hinko,

> > In order to verify whether I2C block reads work for you, just
> > compare the contents of this file:
> >   /sys/bus/i2c/devices/0-0050/eeprom
> 
> I've tested your patch on gericom X5 with VIA chipset and it works
> fine without/with your patch (no diff in eeprom contents).
> (...)
> 0000:00:11.0 ISA bridge: VIA Technologies, Inc. VT8233 PCI to ISA Bridge

This is a surprising result, as the VT8233 datasheet didn't mention the
I2C block mode. I'll add this model to the list. This also suggests that
the VT8233A may support it as well - if someone could test that.

I have to admit I don't know exactly in which order the different south
bridges were designed and released by VIA. I think the following order
is correct:

VT82C596
VT82C596B
VT82C686A
VT82C686B
VT8235
VT8237

But I don't know where the VT8231, VT8233 and VT8233A should be inserted
in this list. If anyone can tell me...

Could you try running "i2cdump 0 0x50" and "i2cdump 0 0x50 i" (with the
patch still applied), and compare both the outputs and the time each
command takes? You should see similar outputs, but the second command
should be magnitudes faster. This would confirm that the I2C block mode
works as intended on your VT8233 chip.

(0 and 0x50 may be different values on your system, please adapt
depending on where in the sysfs tree the eeprom file appeared.)

Thanks for the tests.
-- 
Jean Delvare

Re: [lm-sensors] Re: I2C block reads with i2c-viapro: testers wanted

[Martin Drab]

On Wed, 10 Aug 2005, Jean Delvare wrote:

> Hi Hinko,
> 
> > > In order to verify whether I2C block reads work for you, just
> > > compare the contents of this file:
> > >   /sys/bus/i2c/devices/0-0050/eeprom
> > 
> > I've tested your patch on gericom X5 with VIA chipset and it works
> > fine without/with your patch (no diff in eeprom contents).
> > (...)
> > 0000:00:11.0 ISA bridge: VIA Technologies, Inc. VT8233 PCI to ISA Bridge
> 
> This is a surprising result, as the VT8233 datasheet didn't mention the
> I2C block mode. I'll add this model to the list. This also suggests that
> the VT8233A may support it as well - if someone could test that.
> 
> I have to admit I don't know exactly in which order the different south
> bridges were designed and released by VIA. I think the following order
> is correct:
> 
> VT82C596
> VT82C596B
> VT82C686A
> VT82C686B
> VT8235
> VT8237
> 
> But I don't know where the VT8231, VT8233 and VT8233A should be inserted
> in this list. If anyone can tell me...

I guess it's just the way it seems:

VT82C596
VT82C596B
VT82C686A
VT82C686B
VT8231
VT8233
VT8233A
VT8235
VT8237

Martin

Re: [lm-sensors] Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Martin,

> > But I don't know where the VT8231, VT8233 and VT8233A should be
> > inserted in this list. If anyone can tell me...
> 
> I guess it's just the way it seems:
> 
> VT82C596
> VT82C596B
> VT82C686A
> VT82C686B
> VT8231
> VT8233
> VT8233A
> VT8235
> VT8237

I'd agree for VT8233 and VT8233A according to some searches I did this
morning. However, the VT8231 doesn't seem to fit in the list. Here are
the facts I am aware of that led me to this conclusion:

* It seems to appear only on specific boards, such as the Epia ones, and
is rarely mentioned on common hardware sites, compared to all the other
ones.

* It was presented by VIA at the Cebit in march 2002, which corresponds
to the release of the VT8233A.

* The support was added to the i2c-viapro driver in May 2002, that is,
after VT8233 (October 2001) and VT8233A (March 2002.)

* Its PCI ID is completely different (0x8231, while all other supported
devices are in the 0x3000-0x31FF range.)

So I think it doesn't exactly fit in the release timeline, and is more
likely a custom product derived from the VT82C686 (A or B) rather than a
linear evolution.

For this reason, I won't extrapolate the results to the VT8231. I will
only enable I2C block support for it if someone with such a chip agrees
to test the support first.

Thanks,
-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Hinko Kocevar]

Jean Delvare wrote:
> 
> Could you try running "i2cdump 0 0x50" and "i2cdump 0 0x50 i" (with the
> patch still applied), and compare both the outputs and the time each
> command takes? You should see similar outputs, but the second command
> should be magnitudes faster. This would confirm that the I2C block mode
> works as intended on your VT8233 chip.

Hmm, not really. Here it takes 6 seconds for the first test nad about 5 seconds 
for the second test (I just read the WARNING - need to substract 5s from the 
results...).

noa xtrm # time i2cdump 0 0x50
No size specified (using byte-data access)
   WARNING! This program can confuse your I2C bus, cause data loss and worse!
   I will probe file /dev/i2c-0, address 0x50, mode byte
   You have five seconds to reconsider and press CTRL-C!

      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 80 08 07 0d 09 02 40 00 04 75 75 00 82 10 00 01    ??????@.?uu.??.?
10: 0e 04 0c 01 02 20 00 a0 75 00 00 50 3c 50 2d 20    ????? .?u..P<P-
20: 90 90 50 50 00 00 00 00 00 00 00 00 00 00 00 00    ??PP............
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a7    ...............?
40: ce 00 00 00 00 00 00 00 01 4d 34 20 37 30 4c 33    ?.......?M4 70L3
50: 32 32 34 44 54 30 2d 43 42 30 20 30 44 02 51 45    224DT0-CB0 0D?QE
60: 0b c8 db 00 4c 39 41 37 43 33 4e 00 00 00 00 00    ???.L9A7C3N.....
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
80: 00 20 12 02 22 07 ff ff ff ff ff 18 04 16 ff ff    . ??"?.....???..
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................

real	0m6.033s
user	0m0.000s
sys	0m0.004s
noa xtrm # time i2cdump 0 0x50 i
   WARNING! This program can confuse your I2C bus, cause data loss and worse!
   I will probe file /dev/i2c-0, address 0x50, mode i2c block
   You have five seconds to reconsider and press CTRL-C!

      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 80 08 07 0d 09 02 40 00 04 75 75 00 82 10 00 01    ??????@.?uu.??.?
10: 0e 04 0c 01 02 20 00 a0 75 00 00 50 3c 50 2d 20    ????? .?u..P<P-
20: 90 90 50 50 00 00 00 00 00 00 00 00 00 00 00 00    ??PP............
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a7    ...............?
40: ce 00 00 00 00 00 00 00 01 4d 34 20 37 30 4c 33    ?.......?M4 70L3
50: 32 32 34 44 54 30 2d 43 42 30 20 30 44 02 51 45    224DT0-CB0 0D?QE
60: 0b c8 db 00 4c 39 41 37 43 33 4e 00 00 00 00 00    ???.L9A7C3N.....
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
80: 00 20 12 02 22 07 ff ff ff ff ff 18 04 16 ff ff    . ??"?.....???..
90: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
a0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
b0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
c0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................

real	0m5.174s
user	0m0.001s
sys	0m0.002s

while simple cat takes a lot less time:
noa xtrm # time dd if=/sys/bus/i2c/devices/0-0050/eeprom bs=4 | hexdump -C
00000000  80 08 07 0d 09 02 40 00  04 75 75 00 82 10 00 01  |......@..uu.....|
00000010  0e 04 0c 01 02 20 00 a0  75 00 00 50 3c 50 2d 20  |..... ..u..P<P- |
00000020  90 90 50 50 00 00 00 00  00 00 00 00 00 00 00 00  |..PP............|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 a7  |................|
00000040  ce 00 00 00 00 00 00 00  01 4d 34 20 37 30 4c 33  |.........M4 70L3|
00000050  32 32 34 44 54 30 2d 43  42 30 20 30 44 02 51 45  |224DT0-CB0 0D.QE|
00000060  0b c8 db 00 4c 39 41 37  43 33 4e 00 00 00 00 00  |....L9A7C3N.....|
00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000080  00 20 12 02 22 07 ff ff  ff ff ff 18 04 16 ff ff  |. .."...........|
00000090  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
64+0 zapisov na vhodu
64+0 zapisov na izhodu
00000100

real	0m0.600s
user	0m0.002s
sys	0m0.005s

It seems pretty fast in all cases (i2cdump, i2cdump block and cat) and results 
are close too (taing WARNING delay into account).

regards,
hinko k
-- 
..because under Linux "if something is possible in principle,
then it is already implemented or somebody is working on it".

					--LKI

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Hinko,

> > Could you try running "i2cdump 0 0x50" and "i2cdump 0 0x50 i" (with
> > the patch still applied), and compare both the outputs and the time
> > each command takes? You should see similar outputs, but the second
> > command should be magnitudes faster. This would confirm that the I2C
> > block mode works as intended on your VT8233 chip.
> 
> Hmm, not really. Here it takes 6 seconds for the first test nad about
> 5 seconds  for the second test (I just read the WARNING - need to
> substract 5s from the  results...).

With a recent version of i2cdump (2.8.8 or later), you can use the -y
flag, which will skip this delay. This is very convenient for timing
tests.

That being said...

> noa xtrm # time i2cdump 0 0x50
> (...)
> real	0m6.033s
> (...)
> noa xtrm # time i2cdump 0 0x50 i
> (...)
> real	0m5.174s

This is 1.033s down to 0.174s. This is just great, I2C block reads work
and allow faster dumps, as expected.

> while simple cat takes a lot less time:
> noa xtrm # time dd if=/sys/bus/i2c/devices/0-0050/eeprom bs=4

This goes through the eeprom driver, which has an internal cache, so the
results are not suitable for timing comparisons.

Thanks a lot for the testing again :)
-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Krzysztof Halasa]

Jean Delvare <khali@linux-fr.org> writes:

> This is 1.033s down to 0.174s. This is just great, I2C block reads work
> and allow faster dumps, as expected.

Forgive my ignorance, but does it depend on the south bridge at all?
Isn't the block read capability just a function of EEPROM chip on
(I assume) RAM module?
-- 
Krzysztof Halasa

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Krzysztof,

> Forgive my ignorance, but does it depend on the south bridge at all?
> Isn't the block read capability just a function of EEPROM chip on
> (I assume) RAM module?

EEPROMs can be read using three different access modes: data byte reads
(each transaction asks for one byte at a given offset), continuous byte
reads (first transaction sets offset to 0, each subsequent transaction
reads one byte and the offset auto-increments), and I2C block reads (one
single transaction sets the initial offset and reads the whole block
[1]).

Now, just because most EEPROMs support these three modes doesn't mean
you can always use them. It depends on which bus the EEPROM is sitting
on, and which driver is used to control that bus.

If the bus is a true I2C bus, most of which are known as bit-banging and
are driven by the i2c-algo-bit driver, then all three types of accesses
will work, and we'll use I2C block reads because it's faster. However,
EEPROMs are also often found on SMBus busses, those controller only
implement a subset of all possible I2C commands. Where this subset
includes I2C block reads, we can implement it in the driver and use it.
This is exactly what I am doing with i2c-viapro right now.

More generally, for an I2C/SMBus command to be usable, both the target
chip and the bus controller must support it, and the bus controller
driver must implement it.

I hope it's clearer now :)

[1] Due to an internal limitation in the Linux kernel, the maximum block
size that can be read is actually 32 bytes, so several block reads are
needed to retrieve larger chunks of data.

-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Krzysztof Halasa]

Jean Delvare <khali@linux-fr.org> writes:

> However,
> EEPROMs are also often found on SMBus busses, those controller only
> implement a subset of all possible I2C commands.

You mean one can't drive clock and data lines at will, and the controller
(some hardware) does it instead, based on commands received from the
program (and, for example, the program interface is parallel, not 2-wire
serial). Right?

But wait, even then does the controller really know anything about
I^2C commands? How would it differentiate between, say, 8-bit and
16-bit reads? Or is it just an 8-bit EEPROM bus?

Does it do START and STOP automatically as well?

> [1] Due to an internal limitation in the Linux kernel, the maximum block
> size that can be read is actually 32 bytes, so several block reads are
> needed to retrieve larger chunks of data.

BTW: Some devices can't output all their contents in one transaction (or
with continuous byte/word read) - they are limited to one bank.
If you want to access another bank, you have to select it first.
But I think you know things like that better than me.
-- 
Krzysztof Halasa

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Krzysztof,

> > However,
> > EEPROMs are also often found on SMBus busses, those controller only
> > implement a subset of all possible I2C commands.
> 
> You mean one can't drive clock and data lines at will, and the
> controller (some hardware) does it instead, based on commands received
> from the program (and, for example, the program interface is parallel,
> not 2-wire serial). Right?

Partly right. Actually, most SMBus controllers work the following way:
you program a number of registers (typically SMBus transaction type,
target chip address, target register address or command, and the data to
send in the case of a write transaction), then you tell the chip to
initiate the transaction. Then you poll for the transaction to be over
(or use an interupt, but most our SMBus drivers use polling), and read
the result in some register in the case of a read transaction.

> But wait, even then does the controller really know anything about
> I^2C commands? How would it differentiate between, say, 8-bit and
> 16-bit reads? Or is it just an 8-bit EEPROM bus?

No, it is still physically a 2-wire serial bus. The limitation is due to
the fast that the SMBus controller knows of a limited number of
transactions, such as Send Byte, Read Byte, Read Word etc. If the SMBus
controller doesn't know of the SMBus command you want to use (in my
case, I2C block read), then there is no way to do it, because we have no
direct control over the serial line.

> Does it do START and STOP automatically as well?

Absolutely. The good thing is that SMBus masters are not CPU intensive,
contrary to bit-banging I2C adapters.

-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Krzysztof Halasa]

Hi,

Jean Delvare <khali@linux-fr.org> writes:

> Partly right. Actually, most SMBus controllers work the following way:
> you program a number of registers (typically SMBus transaction type,
> target chip address, target register address or command, and the data to
> send in the case of a write transaction),

So, with one transaction, can I write an arbitrary number of bytes to
the device, and then, in the same transaction, can I read one (or no,
or with some controllers, more than one) byte(s) back?

>> But wait, even then does the controller really know anything about
>> I^2C commands? How would it differentiate between, say, 8-bit and
>> 16-bit reads? Or is it just an 8-bit EEPROM bus?
>
> No, it is still physically a 2-wire serial bus.

Sure, I rather meant "a bus _for_ I^2C EEPROMs which use 8-bit (+3)
addressing".

> The limitation is due to
> the fast that the SMBus controller knows of a limited number of
> transactions, such as Send Byte, Read Byte, Read Word etc. If the SMBus
> controller doesn't know of the SMBus command you want to use (in my
> case, I2C block read), then there is no way to do it, because we have no
> direct control over the serial line.

Interesting. Still, that limits it to 8-bit-addressable EEPROMs.

Are such things documented somewhere on the net (some datasheet maybe)?
-- 
Krzysztof Halasa

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Krzysztof,

> So, with one transaction, can I write an arbitrary number of bytes to
> the device, and then, in the same transaction, can I read one (or no,
> or with some controllers, more than one) byte(s) back?

In I2C mode, you can even alternate as many read and write sequences you
want in a single transaction. The target chip would of course need to
know how to interpret such a transaction though. I've never seen this
possibility used so far.

In SMBus mode, you are limited by the transaction types that have been
defined, but a number of them are composed of a write transaction and a
read transaction (separated by what is known as a "repeated start").
Read Byte, Read Word, Read Block and Read I2C Block are such
transactions. See the SMBus specification for the details.

> Interesting. Still, that limits it to 8-bit-addressable EEPROMs.

Depends on the SMBus. A SMBus controller could implement 16-bit address
I2C block transfers. I don't think I saw one do so far, but it's still
possible in theory.

Also note that this can easily be emulated using a Write Byte command
(which writes 2 bytes on the bus) to set the 16-bit addressed EEPROM
offset, then using continuous Receive Byte commands to get the data.
This is of course slower than a block read though.

> Are such things documented somewhere on the net (some datasheet
> maybe)?

The I2C specifications are available from Philips. The SMBus
specifications are available from smbus.org. Intel also has good
datasheets for all ICH chips.

http://www.semiconductors.philips.com/markets/mms/protocols/i2c/
http://www.smbus.org/specs/

More generally, see the lm_sensors project's links page:
http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/useful_addresses.html
and also:
http://secure.netroedge.com/~lm78/docs.html

Hope that helps,
-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Krzysztof Halasa]

Jean Delvare <khali@linux-fr.org> writes:

> In I2C mode, you can even alternate as many read and write sequences you
> want in a single transaction. The target chip would of course need to
> know how to interpret such a transaction though. I've never seen this
> possibility used so far.

Is this mode supported by the common (such as VIA south bridge)
controllers?

> In SMBus mode, you are limited by the transaction types that have been
> defined, but a number of them are composed of a write transaction and a
> read transaction (separated by what is known as a "repeated start").
> Read Byte, Read Word, Read Block and Read I2C Block are such
> transactions. See the SMBus specification for the details.

Ok. I guest there are just different codes for 8- and 16-bit addressing.

> The I2C specifications are available from Philips. The SMBus
> specifications are available from smbus.org. Intel also has good
> datasheets for all ICH chips.
>
> http://www.semiconductors.philips.com/markets/mms/protocols/i2c/
> http://www.smbus.org/specs/
>
> More generally, see the lm_sensors project's links page:
> http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/useful_addresses.html
> and also:
> http://secure.netroedge.com/~lm78/docs.html
>
> Hope that helps,

Sure. Thanks.
-- 
Krzysztof Halasa

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Krzysztof,

> > In I2C mode, you can even alternate as many read and write sequences
> > you want in a single transaction. The target chip would of course
> > need to know how to interpret such a transaction though. I've never
> > seen this possibility used so far.
> 
> Is this mode supported by the common (such as VIA south bridge)
> controllers?

No it's not, for the simple reason that south bridges are almost always
SMBus controllers, not I2C controllers.

That being said, I'd be surprised if it ever matters. I never saw such
transactions, and if some chip was to accept them, it would most
probably also accept a sequence of more simple, SMBus-compatible
transactions to achieve the same goal.

If you have further questions, they are welcome but I'd suggest that you
drop LKML from the recipients and follow up on the lm_sensors list only,
as we are getting somewhat off-topic now.

-- 
Jean Delvare

Re: [lm-sensors] I2C block reads with i2c-viapro: testers wanted

[Mark M. Hoffman]

Hi Jean:

* Jean Delvare <khali@linux-fr.org> [2005-08-09 23:13:28 +0200]:
> I am implementing I2C block reads in the i2c-viapro driver, and am
> looking for testers. I was able to test on my own VT8237R chip, it works
> OK, now I'd need to know how it works on older VIA south bridges, namely
> the VT8235 and the VT82C686B. South bridges before that (VT82C686A,
> VT8233A and older) are supposed not to work according to the datasheets,
> but a confirmation would be welcome, who knows, it might simply not be
> documented.
> 
> My experimental patch follows. I have enabled the I2C block read
> function for all VIA south bridges, so that it can be tested on all
> chips. I'll restrict that after the test phase, of course.

I fired it up on one of the older chipsets...

# lspci
00:00.0 Host bridge: VIA Technologies, Inc. VT82C693A/694x [Apollo PRO133x] (rev c4)
00:01.0 PCI bridge: VIA Technologies, Inc. VT82C598/694x [Apollo MVP3/Pro133x AGP]
00:07.0 ISA bridge: VIA Technologies, Inc. VT82C596 ISA [Mobile South] (rev 23)
00:07.1 IDE interface: VIA Technologies, Inc. VT82C586A/B/VT82C686/A/B/VT823x/A/C PIPC Bus Master IDE (rev 10)
00:07.2 USB Controller: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (rev 11)
00:07.3 Host bridge: VIA Technologies, Inc. VT82C596 Power Management (rev 30)

# lspci -n
00:00.0 Class 0600: 1106:0691 (rev c4)
00:01.0 Class 0604: 1106:8598
00:07.0 Class 0601: 1106:0596 (rev 23)
00:07.1 Class 0101: 1106:0571 (rev 10)
00:07.2 Class 0c03: 1106:3038 (rev 11)
00:07.3 Class 0600: 1106:3050 (rev 30)

As you suspected, it didn't work.

# i2cdump -y 0 0x50 i
Error: Block read failed, return code 0

> On my system, the dump is down from over 2 seconds without the patch to
> below 0.2 second with the patch, which proves how efficient I2C block
> reads are and explains why I want to implement this function.

I also found something interesting, by accident.  With a stock FC4 kernel,
the i2cdump clocked at 1.02s total.  With 2.6.13-rc6, it took 5.11s!  The
only relevant difference that I can see is that the FC4 kernel uses HZ of
1000 while my 2.6.13-rc6 kernel uses 100.  Because the i2c-viapro is a
polling driver, it becomes slower as HZ decreases.

I.e. the improvement you are seeing with byte vs. block xfers is quite
exaggerated because we poll the device relatively infrequently.  *All*
the xfers are slower than they could be w/ a non-polling driver.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

Re: [lm-sensors] I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Mark,

> > My experimental patch follows. I have enabled the I2C block read
> > function for all VIA south bridges, so that it can be tested on all
> > chips. I'll restrict that after the test phase, of course.
> 
> I fired it up on one of the older chipsets...
> 
> # lspci
> (...)
> 00:07.0 ISA bridge: VIA Technologies, Inc. VT82C596 ISA [Mobile South] (rev 23)
> (...)
> 00:07.3 Host bridge: VIA Technologies, Inc. VT82C596 Power Management (rev 30)

Oh, you do have one of these old 596 chips. Interesting. Could you
possibly take a look at the i2c-viapro driver and comment on that part:

	if ((pci_read_config_word(pdev, id->driver_data, &vt596_smba)) ||
	    !(vt596_smba & 0x1)) {
		/* try 2nd address and config reg. for 596 */
		if (id->device == PCI_DEVICE_ID_VIA_82C596_3 &&
		    !pci_read_config_word(pdev, SMBBA2, &vt596_smba) &&
		    (vt596_smba & 0x1)) {
			smb_cf_hstcfg = 0x84;

It looks like a hack to me, and I was wondering if we could clean it up
(possibly by using pci quirks). I've not studied the problem in details
yet, but comments and help would be welcome, and the fact that you do
have a chip for testing will help anyway :)

> As you suspected, it didn't work.
> 
> # i2cdump -y 0 0x50 i
> Error: Block read failed, return code 0

Alright, it matches the datasheet and my expectations.

> I also found something interesting, by accident.  With a stock FC4
> kernel, the i2cdump clocked at 1.02s total.  With 2.6.13-rc6, it took
> 5.11s!  The only relevant difference that I can see is that the FC4
> kernel uses HZ of 1000 while my 2.6.13-rc6 kernel uses 100.  Because
> the i2c-viapro is a polling driver, it becomes slower as HZ decreases.

Absolutely true. This is what improved the speed of most SMBus drivers
in 2.6 when compared with 2.4: HZ=1000. Now we're stepping back a bit
with HZ=250.

> I.e. the improvement you are seeing with byte vs. block xfers is quite
> exaggerated because we poll the device relatively infrequently.  *All*
> the xfers are slower than they could be w/ a non-polling driver.

Oh well, even if we were using interrupts instead of polling, the block
transactions are still better as they are still twice as fast, would
generate less interrupts, and are generally less CPU-intensive. That
being said, I plan to implement interrupts in i2c-viapro at a later
time. I have no idea how to do that, but I'll take a look at what you
did in your i2c-i801 reimplementation, it should help. I simply seem to
remember that you expected problems for block transactions in interrupt
more, and don't know why exactly.

Maybe we can use interrupts for short commands and polling for big
transactions. I see no technical problem doing that - although this
would make the driver larger and more complex.

BTW, it would be great if you could finalize your i2c-i801
reimplementation so that we can add it to Linux 2.6. I remember it gave
me much better performance than the original one, and the code was
looking much nicer as well.

Thanks for the tests :)
-- 
Jean Delvare

Re: I2C block reads with i2c-viapro: testers wanted

[Salah Coronya]

I have a VT8235 chipset, I applied the patch to my kernel 
(2.6.12-gentoo-r6), comapred the "before" and "after" eeproms in /sys 
with diff and they are the same.

So it seems to work with VT8235.

Re: I2C block reads with i2c-viapro: testers wanted

[Jean Delvare]

Hi Salah,

[Salah Coronya]
> I have a VT8235 chipset, I applied the patch to my kernel
> (2.6.12-gentoo-r6), compared the "before" and "after" eeproms in /sys
> with diff and they are the same.

Beware that eeprom files are binary files, so "diff" might not be the
more suitable tool for the job. "cmp" would probably do better.
Nevertheless, I think I remember that diff can at least differenciate
between identical and different binary files, so your test must still be
valid.

> So it seems to work with VT8235.

Yes, it does. Antonino A. Daplas reported success on his VT8235 in
private as well, so I am now confident that this chip is OK. The
question remains for VT82C686A and B though.

Thanks for your help!
--
Jean Delvare