Fix I2C handing:

[aystarik]

    1. no limit on size
    2. write of EEPROM works
    3. i2cdetect works properly in both -q (WRITE0) and -r (READ1) modes

[frank-zago]

Thanks. I tested it, and it works fine. However I think it goes a step backwards speedwise. For instance reading a 24c256 with the current driver gives this:

$ time dd if=/sys/bus/i2c/devices/i2c-11/11-0050/eeprom of=foo bs=4096
8+0 records in
8+0 records out
32768 bytes (33 kB, 32 KiB) copied, 2.817 s, 11.6 kB/s
real    0m2.819s
user    0m0.000s
sys 0m0.020s

With this patch:

$ time dd if=/sys/bus/i2c/devices/i2c-11/11-0050/eeprom of=foo bs=4096
8+0 records in
8+0 records out
32768 bytes (33 kB, 32 KiB) copied, 3.32827 s, 9.8 kB/s
real    0m3.330s
user    0m0.032s
sys 0m0.032s

Writing is not good with these chips not because of the ch341 driver, but because we currently have no way to tell the AT24 driver to write more than a byte at a time and/or wait between writes:

at24 11-0050: 32768 byte 24c256 EEPROM, writable, 1 bytes/write

Writing is not issue for other devices I have tested with, such as these small 32x64 or 32x128 displays.

[frank-zago]

I've been wondering whether it would be possible to inject a DTS file describing the i2c device, which would allow more parameters than the simple <driver, address> we can currently give. Such a solution would also work for spi.

[aystarik]

My main concern with read size was that now I can use "hexdump eeprom" and "cat eeprpom" without weird errors.

[aystarik]

Actually, it's not <driver, address>, it's <chip_type, address> and all the needed parameters are already associated with chip_type, e.g. page_size. So we only need to propagate this info.

[aystarik]

One more thing about write, before my patch at24 driver was trying to write with one byte messages and was not getting -ETIMEOUT if write of previous byte was not complete, thus the mess. Now it gets proper response and waits. So write although painfully slow, is correct.

[aystarik]

Actually, it's not <driver, address>, it's <chip_type, address> and all the needed parameters are already associated with chip_type, e.g. page_size. So we only need to propagate this info.

It could be as simple as this: 1.txt

[frank-zago]

BTW, what do you mean by "no limit on size"?

[aystarik]

Your driver is limited in size by the buffer, my change removes that this: if (dev->out_seg == SEG_COUNT) return -E2BIG;

[frank-zago]

I've merged the mfd driver, and a patch that used the same algo as this one. Hopefully all the bugs you noticed in i2c handling are now fixed.

[aystarik]

I think you've made it worse. Why did you add a 32 byte "quirk" limit?

[frank-zago]

hmm. It should have been 31 since there is the status byte too.

Anyway, that PR is actually splitting a large message into 31-bytes frames, as delimited by START/STOP, without telling the kernel that this is happening. I don't think that's right.

I've done various tests and will go back to something I did previously, which is combining the read requests, which should expand the read frames to 127 bytes.

[aystarik]

Oh, but we don't tell kernel only the USB transport we are using, for I2C frames we have separate controls (CH341_CMD_I2C_STM_STA/CH341_CMD_I2C_STM_STO) and we are able to put them in the right places. You may think of USB frames in terms of any other bus transactions -- on AXI/AHB you will be limited to talk with I2C controller in 32 bit words. This is why my algorithm works -- I don't project USB limits on I2C.