linux-kernel - Re: [PATCH] hid: cp2112: support i2c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 15 Mar 2015 15:27:59 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Antonio Borneo <borneo.antonio@...il.com>
Cc:	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	linux-input <linux-input@...r.kernel.org>,
	Jonathan Cameron <jic23@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-i2c@...r.kernel.org
Subject: Re: [PATCH] hid: cp2112: support i2c_transfer() when num > 1


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


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)