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

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

[Nick Warne]

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

I have a VIA board, and remember when I config'ed I2C I was a bit confused 
with what I have - I guessed logically in the end that VT82C686 _became_ 
VT82C686A _after_ VT82C686B was released.  It all seems to work OK though.

bash-2.05b# lspci
00:00.0 Host bridge: VIA Technologies, Inc. VT8363/8365 [KT133/KM133] (rev 03)
00:01.0 PCI bridge: VIA Technologies, Inc. VT8363/8365 [KT133/KM133 AGP]
00:07.0 ISA bridge: VIA Technologies, Inc. VT82C686 [Apollo Super South] (rev 
22)
00:07.1 IDE interface: VIA Technologies, Inc. 
VT82C586A/B/VT82C686/A/B/VT823x/A/C/VT8235 PIPC Bus Master IDE (rev 10)
00:07.2 USB Controller: VIA Technologies, Inc. VT6202 [USB 2.0 controller] 
(rev 10)
00:07.3 USB Controller: VIA Technologies, Inc. VT6202 [USB 2.0 controller] 
(rev 10)
00:07.4 Host bridge: VIA Technologies, Inc. VT82C686 [Apollo Super ACPI] (rev 
30)
00:09.0 Ethernet controller: 3Com Corporation 3c905C-TX/TX-M [Tornado] (rev 
78)
00:0f.0 Multimedia audio controller: Creative Labs SB Live! EMU10k1 (rev 07)
00:0f.1 Input device controller: Creative Labs SB Live! MIDI/Game Port (rev 
07)
01:00.0 VGA compatible controller: nVidia Corporation NV17 [GeForce4 MX 440] 
(rev a3)

I am OK for testing the patch if you need.

Nick

-- 
"When you're chewing on life's gristle,
Don't grumble, Give a whistle..."

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