Re: [PATCH] hid: cp2112: support i2c_transfer() when num > 1

From: Wolfram Sang
Date: Sun Mar 15 2015 - 10:27:51 EST



> >> 1) in few kernel drivers i2c_transfer() has been used to
> >> simplify the code by replacing a sequence of i2c_master_send()
> >> and i2c_master_recv(), e.g. in i2c-hid.c and iio subsystem.
> >> Those drivers will fail if used with current cp2112 driver.
> >
> > I see two options for those:
> >
> > 1) revert the simplifications and sacrifice a bit of performance
> > to support the widest number of adapters
> > 2) use the quirk infrastructure from above to query the
> > quirks of the adapter to chose between fast or compatible
>
> Could this be an extension of your quirks?
> I mean, moving in i2c-core the switch between fast or compatible?
> The caller should only state if combined transfer is strictly required
> by the device on I2C bus.

I'd first need some arguments that this is not micro-optimization :)

> I have check most of the I2C adapter drivers. At least for the 6
> drivers above, reading the code I don't see anything that implements
> the repeated start. But I do not have the HW to run a test.
> It's definitively possible I misread them.

I can't guarantee this in detail. The qup driver is quite new, so I am
quite sure I asked for that. However, I don't know for the USB drivers,
I have to trust the driver authors. puv3? Predates me, uh, let's not
talk about it... The main thing still stands: If they send stop after
each msg, this is a bug.

> >> To fix 1) and considering 2), rewrite i2c_transfer() in case
> >> of num > 1 as a loop of non-combined i2c transactions.
> >
> > For the above reasons, NAK.
>
> Ok, agree to drop this patch.

Thanks.

> > And why is this driver in hid? This is clearly an I2C master driver?
>
> Actually it should be a MFD, since it implements I2C/SMB master and GPIO.
> It uses HID over USB, that is probably the reason it is here.

Yup, from what I glimpsed, it should really be an MFD driver.

Attachment: signature.asc
Description: Digital signature