Re: [PATCH v2 2/5] em28xx: respect the message size constraints for i2c transfers

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

Hi Sascha,

Am 02.01.2013 21:45, schrieb Sascha Sommer:
> Hello,
>
> Am Sat, 22 Dec 2012 22:07:46 -0200
> schrieb Mauro Carvalho Chehab <mchehab@redhat.com>:
>
>> Em Sun, 16 Dec 2012 19:23:28 +0100
>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>>
>>> The em2800 can transfer up to 4 bytes per i2c message.
>>> All other em25xx/em27xx/28xx chips can transfer at least 64 bytes
>>> per message.
>>>
>>> I2C adapters should never split messages transferred via the I2C
>>> subsystem into multiple message transfers, because the result will
>>> almost always NOT be the same as when the whole data is transferred
>>> to the I2C client in a single message.
>>> If the message size exceeds the capabilities of the I2C adapter,
>>> -EOPNOTSUPP should be returned.
>>>
>>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-i2c.c |   44
>>> ++++++++++++++------------------- 1 Datei geändert, 18 Zeilen
>>> hinzugefügt(+), 26 Zeilen entfernt(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
>>> b/drivers/media/usb/em28xx/em28xx-i2c.c index 44533e4..c508c12
>>> 100644 --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>>> @@ -50,14 +50,18 @@ do
>>> {							\ } while
>>> (0) 
>>>  /*
>>> - * em2800_i2c_send_max4()
>>> - * send up to 4 bytes to the i2c device
>>> + * em2800_i2c_send_bytes()
>>> + * send up to 4 bytes to the em2800 i2c device
>>>   */
>>> -static int em2800_i2c_send_max4(struct em28xx *dev, u8 addr, u8
>>> *buf, u16 len) +static int em2800_i2c_send_bytes(struct em28xx
>>> *dev, u8 addr, u8 *buf, u16 len) {
>>>  	int ret;
>>>  	int write_timeout;
>>>  	u8 b2[6];
>>> +
>>> +	if (len < 1 || len > 4)
>>> +		return -EOPNOTSUPP;
>>> +
>> Except if you actually tested it with all em2800 devices, I think that
>> this change just broke it for em2800.
>>
>> Maybe Sascha could review this patch series on his em2800 devices.
>>
>> Those devices are limited, and just like other devices (cx231xx for
>> example), the I2C bus need to split long messages, otherwise the I2C
>> devices will fail.
>>
> The only device that I own is the Terratec Cinergy 200 USB.
> Unfortunately I left it in my parents house so I won't be able to
> test the patch within the next two weeks. I don't know if any of the
> other devices ever transfered more than 4 bytes but as far as I
> remember the windows driver of the cinergy 200 usb did not do this.
> The traces obtained from it were the only information I had during
> development. On first sight, the splitting code looks wrong.

Thanks for your reply !
I have a Terratec Cinergy 200 USB, too, and I used this device for
testing the code.
You are right, the Windows driver never transfers more than 4 bytes
(verified with USB-logs).
Do you know if there is something like a control flag for non-stopping
i2c transfers ?

Maybe you also noticed the following tread:
http://www.spinics.net/lists/linux-media/msg57442.html

Do you remember any details about your device ?
One thing not mentioned in this tread is, that there seem to be multiple
chip IDs for the EM2800.
The em28xx only knows about ID=7 and I assume that's what you device
uses. But the chip in my device uses ID=4...

Regards,
Frank

> Regards
>
> Sascha
>
>