linux-kernel - Re: [PATCH v3] gpio: interrupt consistency check for OF GPIO IRQs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <522F8DB7.8070800@collabora.co.uk>
Date:	Tue, 10 Sep 2013 23:23:03 +0200
From:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	Lars Poeschel <poeschel@...onage.de>,
	Mark Brown <broonie@...nel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Lars Poeschel <larsi@....tu-dresden.de>,
	Grant Likely <grant.likely@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ian.campbell@...rix.com>,
	Kumar Gala <galak@...eaurora.org>,
	Pawel Moll <pawel.moll@....com>,
	Tomasz Figa <tomasz.figa@...il.com>,
	Enric Balletbo i Serra <eballetbo@...il.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Kevin Hilman <khilman@...aro.org>,
	Balaji T K <balajitk@...com>,
	Tony Lindgren <tony@...mide.com>,
	Jon Hunter <jgchunter@...il.com>, joelf@...com
Subject: Re: [PATCH v3] gpio: interrupt consistency check for OF GPIO IRQs

On 09/10/2013 09:52 PM, Stephen Warren wrote:
> On 09/10/2013 07:56 AM, Javier Martinez Canillas wrote:
> ...
>> The only thing that this patch tries to solve is when a driver expect to request
>> a IRQ and it doesn't care if is a real IRQ line from an interrupt controller or
>> a GPIO pin mapped as an IRQ.
> 
> That can be solved in the interrupt chip driver. The fact the previous
> attempt didn't work doesn't mean that it's impossible.
>

Indeed. Unfortunately that patch-set got merged as a fix quite late on the -rc
cycle so it was safer to just revert the patches instead of analyzing the
regression and providing a fix.

If Linus is fond about taking a local fix for gpio-omap then we can try again
as a RFC with a better test coverage than before (although the patches had
several tested and acked-by but no one tested on OMAP1 until the patches were
merged) getting some TI folks in the loop who have access to those ancient OMAP1
devices. That way we can repost as a patch just once we are definitely sure that
it won't cause a regression on any OMAP platform.

>> With board files we used to explicitly do the GPIO setup
>> (gpio_{request,direction_input}) but with DT these board files are gone and we
>> need a way to setup a GPIO implicitly when is mapped as an IRQ.
> 
> Well, that's just an example of hacking around something in a board file
> that should have been fixed in the GPIO/IRQ controller.
> 
>> That's the only use case that this patch covers.
> ...
>> Also, it would be great if we can find a temporary solution so we can finally
>> have ethernet working with DT on most OMAP2+ boards. Since I expect that doing
>> the mentioned change on gpiolib would take at least a couple of kernel releases.
> 
> Really? I thought this patch was error-checking to make sure that
> different drivers didn't request a GPIO and an IRQ that are actually the
> same signal. This patch shouldn't affect any functionality except in
> cases where there's a bug in the DT (incorrect GPIO/IRQ passed to some
> driver).
> 

Yes, it doesn't do any error-checking neither prevent a driver to request a GPIO
and an IRQ that are the same signal (as long as this is supported by the
GPIO/IRQ controller and its driver). The only thing that Linus patch does is to
auto request a GPIO for pins that are mapped as IRQ in DT (i.e: interrupt-parent
= <&gpio6>).

The name of the function introduced by Linus is
of_gpiochip_interrupt_consistency_check() but probably a better name is
of_gpiochip_autorequest_irq() or something like that.

A similar behavior could be obtained by let's say calling gpio_request() in
irq_create_of_mapping() if you wanted to add the logic in the IRQ chip DT core
instead of doing it in the GPIO chip DT core as Linus does with his patch.

That's why I don't understand why Linus patch could be an issue for drivers that
needs to request both the GPIO and the mapped IRQ.

If this is already supported then nothing will break. If the driver tries to
request the GPIO twice because this is already made by the DT core then I think
is a bug in the driver and has to be fixed to support DT since there won't be
any need to do this manually anymore.

Best regards,
Javier
--
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/