linux-kernel - Re: [PATCH V3 1/5] i2c: Add irq_gpio field to struct i2c_client, i2c_board

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:	Tue, 20 Sep 2011 09:54:48 +0100
From:	Jonathan Cameron <jic23@....ac.uk>
To:	Grant Likely <grant.likely@...retlab.ca>
CC:	Jean Delvare <khali@...ux-fr.org>,
	Stephen Warren <swarren@...dia.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Ben Dooks <ben-linux@...ff.org>, Arnd Bergmann <arnd@...db.de>,
	Russell King <linux@....linux.org.uk>,
	Andrew Chew <achew@...dia.com>, linux-iio@...r.kernel.org,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
	linux-tegra@...r.kernel.org, linux-i2c@...r.kernel.org,
	spi-devel-general@...ts.sourceforge.net
Subject: Re: [PATCH V3 1/5] i2c: Add irq_gpio field to struct i2c_client,
 i2c_board_info.

On 09/20/11 05:16, Grant Likely wrote:
> On Fri, Sep 02, 2011 at 08:56:20AM +0200, Jean Delvare wrote:
>> Stephen,
>>
>> Can you please fix your e-mail client / system / whatever so that your
>> patch series are no longer sent duplicated?
>>
>> On Thu,  1 Sep 2011 16:04:27 -0600, Stephen Warren wrote:
>>> Some devices use a single pin as both an IRQ and a GPIO. In that case,
>>> irq_gpio is the GPIO ID for that pin. Not all drivers use this feature.
>>> Where they do, and the use of this feature is optional, and the system
>>> wishes to disable this feature, this field must be explicitly set to a
>>> defined invalid GPIO ID, such as -1.
>>>
>>> Signed-off-by: Stephen Warren <swarren@...dia.com>
>>> ---
>>> v3: Also add the field to i2c_board_info, and copy the field from
>>>     i2c_board_info to i2c_client upon instantiation
>>
>> I don't get the idea. The i2c core doesn't make any use of the field,
>> and that field will only be used by a few drivers amongst the 420+
>> i2c drivers in the tree. This looks like a waste of memory. What's wrong
>> with including the new field in the private platform or arch data
>> structure for drivers which need it?
> 
> I have to second the concern; but for a different reason.  This
> shouldn't even remotely be necessary.  If the pin is used as an
> interrupt, then interrupt controller driver (which I would assume is
> also the gpio controller driver) should be responsible for setting up
> the pin so that it can be used correctly as a irq line.  Why does the
> gpio number need to be explicitly passed?
The particular driver covered here is somewhat of a false positive.
It really ought to be rewritten to do everything 'properly' with
interrupts.  Right now no one with the inclination has the hardware
to fix it up.

The nasty case we are trying to cover is peripherals that use level
interrupts talking to gpio chips that only do edge triggered ones.
We use the pin as an irq but have to query it as a gpio to discover
if the sneaky chip raised it again without it actually going low.

There are sometimes work arounds involving polling registers on
the device to check if the interrupt is active, but give the bus is
slow, pinging the gpio to find out it's state is often much faster.

So it's a dirty hack for dirty hardware.  Having said that I agree
that it's a niche case and certainly shouldn't be part of any core
code, unless it is done right at the core in the generic interrupt
code and previous discussions suggest that is tricky to say the least!


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/