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: <4094364.ACYUvMRRNa@lem-wkst-02>
Date:	Mon, 02 Sep 2013 11:38:25 +0200
From:	Lars Poeschel <poeschel@...onage.de>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	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>,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	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>
Subject: Re: [PATCH v3] gpio: interrupt consistency check for OF GPIO IRQs

Am Freitag, 30. August 2013, 13:53:45 schrieb Stephen Warren:
> On 08/29/2013 01:26 PM, Linus Walleij wrote:
> > On Tue, Aug 27, 2013 at 10:17 PM, Stephen Warren <swarren@...dotorg.org> 
wrote:
> >> On 08/26/2013 08:07 AM, Lars Poeschel wrote:
> >>> Currently the kernel is ambigously treating GPIOs and interrupts
> >>> from a GPIO controller: GPIOs and interrupts are treated as
> >>> orthogonal. This unfortunately makes it unclear how to actually
> >>> retrieve and request a GPIO line or interrupt from a GPIO
> >>> controller in the device tree probe path.
> >> 
> >> I still think that this patch is the wrong approach. Instead, the logic
> >> should be hooked into gpio_request() and request_irq(). This patch only
> >> addresses DT, and ignores anything else, hence doesn't seem like the
> >> right level of abstraction to plug in, since the issue is not related to
> >> DT.> 
> > We tried to do it that way, and it exploded. See commit
> > b4419e1a15905191661ffe75ba2f9e649f5d565e
> > "gpio/omap: auto request GPIO as input if used as IRQ via DT"
> > 
> > Here request_irq() augmented through its irqdomain to
> > issue gpio_request_one().
> > 
> > Why was this patch reverted? It seems this is what has not
> > managed to reach the audience here.
> > 
> > It turns out some drivers were already doing this:
> > 
> > request_gpio(gpio);
> > gpio_direction_input(gpio);
> > request_irq(gpio_to_irq(gpio));
> > 
> > Looks perfectly valid right?
> > 
> > Not so: after the above change, we tried to request the
> > GPIO a *second time* in the GPIO driver's irq map function,
> > and of course it failed, as it was already taken.
> > 
> > So saying that it should be done in the request_irq()
> > function is imposing this other semantic on the kernel
> > instead: you must *NOT* request the GPIO with
> > request_gpio() if you're going to use it as an IRQ.
> 
> Surely both request_gpio() and request_irq() must both request the GPIO
> (amongst other things), with the caveat that if the same driver does
> both, this specific "sharing" is allowed.

As explained in my previous mail, I do not see, how this should work.

> If that won't work, then the very core concept behind what this patch is
> attempting to do won't work.

The concept only does not work in the non-DT case, which we do neither try to 
address with this patch.

> > (Also, it force us to implement the same code in each
> > and every driver, but that is a lesser problem.)
> 
> Drivers don't have to do the request; the IRQ/GPIO core can do this,
> with drivers simply providing an irq_to_gpio() (which only returns valid
> data iff there's a 1:1 mapping between IRQs and GPIOs in that particular
> HW).
> 
> > I don't quite understand what is so hard around this.
> > We cannot get away from restricting the semantics in
> > some way, if gpio-controllers shall also be interrupt-controllers,
> > the current patch is the least intrusive I've seen so far.
> 
> Yet the current patch only addresses a limited set of cases, since it
> doesn't hook the APIs but rather parses the DT. It doesn't cover the
> non-DT case. It should if the feature is useful.

As pointed out before, only DT has this problem, that, whatever you do, 
otherwise it has no chance to request a gpio for a gpio-backed irq. Drivers 
can do this and board files also can do this.
I agree with you that it is somewhat odd, that there are two different ways, 
doing the same thing, but I don't see another way yet.
--
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/