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

[Antti Palosaari <crope@iki.fi>]

On 12/24/2012 01:09 PM, Frank Schäfer wrote:
> Am 23.12.2012 15:46, schrieb Mauro Carvalho Chehab:
>> Em Sun, 23 Dec 2012 14:58:12 +0100
>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>>
>>> Am 23.12.2012 01:07, schrieb Mauro Carvalho Chehab:
>>>> Em Sun, 16 Dec 2012 19:23:28 +0100
>>>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>>>> 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.
>>> I2C adapters are supposed to fail with -EOPNOTSUPP if the message length
>>> exceeds their capabilities.
>>> Drivers must be able to handle this error, otherwise they have to be fixed.
>> Currently, afaikt, no V4L2 I2C client knows how to handle it.
>
> Maybe. Fortunately, it seems to cause no trouble.
>
>>   Ok, returning
>> -EOPNOTSUPP if the I2C data came from userspace makes sense.
>>
>>>> Btw, there was already a long discussion with regards to splitting long
>>>> I2C messages at the I2C bus or at the I2C adapters. The decision was
>>>> to do it at the I2C bus logic, as it is simpler than making a code
>>>> at each I2C client for them to properly handle -EOPNOTSUPP and implement
>>>> a fallback logic to reduce the transfer window until reach what's
>>>> supported by the device.
>>> While letting the i2c bus layer split messages sounds like the right
>>> thing to do, it is hard to realize that in practice.
>>> The reason is, that the needed algorithm depends on the capabilities and
>>> behavior of the i2c adapter _and_ the connected i2c client.
>>> The three main parameters are:
>>> - message size limits
>>> - client register width
>>> - automatic register index incrementation
>>>
>>> I don't know what has been discussed in past,
>> You'll need to dig into the ML archives. This is a recurrent theme, and,
>> we have implementations doing I2C split at bus (most cases) and a few
>> ones doing it at the client side.
>
> Yeah, I also have a working implementation of i2c block read/write
> emulation in my experimental code. ;)
>
>>> but I talked to Jean
>>> Delvare about the message size constraints a few weeks ago.
>>> He told me that it doesn't make sense to try to handle this at the i2c
>>> subsystem level. The parameters can be different for reading and
>>> writing, adapter and client and things are getting complicated quickly.
>> Jean's opinion is to push it to I2C clients (and we actually do it on a
>> few cases), but as I explained before, there are several drivers where
>> this is better done at the I2C bus driver, as the I2C implementation
>> allows doing it easily at bus level by playing with I2C STOP bits/I2C
>> start bits.
>>
>> We simply have too much I2C clients, and -EOPNOTSUPP error code doesn't
>> tell the max size of the I2C messages. Adding a complex split logic
>> for every driver is not a common practice, as just a few I2C bus bridge
>> drivers suffer from very strict limits.
>
> Yes, and even with those who have such a strict limit, it is usually not
> exceeded because the clients are too 'simple'. ;)
>
>> Also, clients that split I2C messages don't actually handle -EOPNOTSUPP.
>> Instead, they have an init parameter telling the maximum size of the
>> I2C messages accepted by the bus.
>>
>> The logic there is complex, and may require an additional logic at the
>> bus side, in order to warrant that no I2C stop/start bits will be sent
>> in the middle of a message, or otherwise the device will fail[1].
>>
>> So, it is generally simpler and more effective to just do it at the bus
>> side.
>
> Maybe. I have no opinion yet.
> My feeling is, that this should be handled by the i2c subsystem as much
> as possible, but
> a) it's complex due to the described reasons
> b) I have no complete concept yet
> c) the i2c people seem to be not very interested
> d) there is lots of other stuff with a higher priority on my TODO list

Maybe you already have seen, but I did some initial stuff year or two 
ago for implementing that but left it unimplemented as there was so much 
stuff to check and discuss in order to agree correct solution.

http://www.mail-archive.com/linux-media@vger.kernel.org/msg38840.html

There is regmap which maybe could do stuff like that, I am not sure as I 
never tested it. At least it could do some stuff top of I2C bus.

Also I heavily disagree you what goes to I2C subsystem integration. That 
is clearly stuff which resides top of I2C bus and it is *not bus 
dependent*. There is many other buses too having similar splitting logic 
like SPI?

> The reason why I started looking into this is, that the em2765 in the
> SpeedLink VAD Laplace webcam (and likely all similar chip variants, e.g.
> em2760, em277x, em25xx) can transfer only 1 byte per message to/from the
> sensor client when using the 2nd i2c bus. I don't know where this
> restriction comes from, possible explanations are:
> - because that the 2nd bus is realized using GPIO pins

Just to mention, there is existing framework for GPIO I2C.

> - because the OV2640 uses SCCB

regards
Antti

-- 
http://palosaari.fi/