Re: [PATCH v1] HID: support multiple and large i2c transfers in hid-cp2112

[Ellen Wang <ellen@cumulusnetworks.com>]

On 05/29/2015 08:38 PM, Antonio Borneo wrote:
> On Sat, May 30, 2015 at 1:32 AM, Ellen Wang <ellen@cumulusnetworks.com> wrote:
>>
>> On 05/29/2015 09:28 AM, Antonio Borneo wrote:
>>>
>>> CP2112 is not able to combine messages since unable to repeat the start
>>> bits.
>>> It can only send simple transfers or a combined read-after-write.
>>>
>>> Here you cannot simply loop on all the messages and send them one by
>>> one because there is no way to force the repeated start bit in
>>> between.
>>> You can find more details in this thread about CP2112
>>> https://lkml.org/lkml/2015/3/15/64
>>> and here info about the repeated start bit under "Combined transactions"
>>> Documentation/i2c/i2c-protocol
>>>
>>> Anyway your idea to introduce read-after-write is valid.
>>> I suggest you to check the drivers i2c-opal.c and i2c-pmcmsp.c ; at a
>>> quick check I think they have similar limitation as CP2112.
>>> You could replicate the same check, supporting only num==1 (always)
>>> and num==2 (only if msg[0] is write and msg[1] is read).
>>>
>>> Best Regards,
>>> Antonio
>>
>>
>> It don't think the code attempts to generate repeated start.  It simply
>> issues and completes each message as separate transfers.  It's not different
>> from calling cp2112_i2c_xfer() repeatedly with single messages, except in
>> the bracketing hid_hw_power().
>
> cp2112_i2c_xfer() is the lower level side of i2c_transfer().
> i2c_transfer() requires that only one Stop bit is sent at the end of
> last message. Between messages a repeated start is mandatory.
> In general case, CP2112 cannot do that, that's why the original code
> returns error if num != 1
>
> It's not correct to issue each message as separate transfer inside
> cp2112_i2c_xfer(); the caller of i2c_transfer() expects the messages
> to be sent with repeated start.
>
> With you patch you highlight that there is another case that should be
> implemented.
> CP2112 can handle the case of num==2 when msg[0] is write and msg[1] is read.
> There are other devices with similar limitations, as I pointed above.
>
> Extending cp2112_i2c_xfer() to support a combined read-after-write is
> ok, while issuing each message without forcing repeated start is
> incorrect.
>
> Best Regards,
> Antonio

OK.  I neglected to read the later messages in the thread you pointed me 
to.  Oddly, I submitted my patch weeks before that thread, and was given 
exactly the opposite feedback, which was that the fix was fine in 
principle (but I should separate out some unrelated bug fixes).  And I 
agreed to using the quirks mechanism in a later patch.

Do you already have a patch that addresses the whole issue?  I just 
rewrote my code to handle single messages and the write-read case, as 
you suggested.

Thanks