Re: [PATCH v1] HID: bug fixes in hid-cp2112 driver

[Ellen Wang <ellen-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>]

I've separated out the cp2112_i2c_xfer() bug fix from the patch.  Should 
I submit it as v2 or a new patch altogether?

Thanks.

On 4/22/2015 1:03 AM, Wolfram Sang wrote:
>
>>> Well, at24 detects how many bytes it got and continues from there.
>>
>> That's true, but instead of returning short, the old
>> cp2112_i2c_xfer() fails out (with EIO) when the first USB operation
>> doesn't return all the bytes.  Look for "short read: %d < %d" in
>> the original version.  That's just broken.
>
> Yes.
>
>>> This shows the drawback of having I2C master drivers not in the
>>> i2c-directory: It easily misses updates to the i2c-core.
>>> We now have the i2c-quirk infrastructure (2187f03a9576c4) which this
>>> driver should make use of. It can describe this...
>>>
>>>> +    for (m = msgs; m < msgs + num; m++) {
>>>> +        /*
>>>> +         * If the top two messages are a write followed by a read,
>>>> +         * then we do them together as CP2112_DATA_WRITE_READ_REQUEST.
>>>> +         * Otherwise, process one message.
>>>> +         */
>>>> +
>>>
>>> and this and the core will check the messages for you. It should
>>> simplify your code, too.
>>
>> I didn't know about that.  Cumulus Linux is based on 3.2.something
>> (debian wheezy) and i2c-quirk came in after that.
>
> Well, actually, it came with this merge window :)
>
>> I can update the driver to use the quirk mechanism, but I would
>> prefer to do that as a separate checkin, so Cumulus can use
>> a version of hid-cp2112.c that exists somewhere in mainline
>> even if it's not the latest.
>
> Fine with me if you do the i2c-quirk update as a seperate patch.
>