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

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]


> >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.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]