Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs

[Alexander Holler]

Am 11.09.2013 18:14, schrieb Javier Martinez Canillas:
> On 09/11/2013 05:30 PM, Alexander Holler wrote:
> > Am 22.08.2013 00:02, schrieb Linus Walleij:
> > > On Tue, Aug 20, 2013 at 12:04 AM, Laurent Pinchart
> > > <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> > > > On Wednesday 31 July 2013 01:44:53 Linus Walleij wrote:
> > >
> > > > > I don't see how sharing works here, or how another user, i.e. another one
> > > > > than the user wanting to recieve the IRQ, can validly request such a line?
> > > > > What would the usecase for that valid request be?
> > > >
> > > > When the GPIO is wired to a status signal (such as an MMC card detect signal)
> > > > the driver might want to read the state of the signal independently of the
> > > > interrupt handler.
> > >
> > > That is true. But for such a complex usecase I think it's reasonable that
> > > we only specify the GPIO in the device tree, and the driver utilizing the
> > > IRQ need to take that and perform gpio_to_irq() on it, and then it still
> > > works to use it both ways.
> >
> > Hmm, the problem is that DT is seen as fixed. So if someone marks a GPIO
> > as an IRQ, it can never be used otherwise. So if you really go this way,
> > you should make this pretty clear in the documentation.
>
> DT is fixed because that describes the hardware which is fixed of course. So if
> a chip IRQ line is connected to a GPIO pin in a controller that should be
> described in the DT and that pin can't be used for anything else.

If you request an gpio for every gpio which defined as IRQ centrally, 
drivers can't just use that gpio as gpio afterwards. And there might be 
one driver which uses the irq but another one which wants the gpio. But 
if you define a gpio as irq in dt, you take away that choice.

> > Looking from the other side, why do you want to mark GPIOs as IRQs in
> > the DT at all?
>
> Because from the component point-of-view that is wired to the SoC, that GPIO is
> an IRQ line and so it has to be described.
>
> > And how will this be done? I found the way it was done in
> > the reverted patch very confusing because it needed an IRQ number. That
> > IRQ number depends on the mapping and isn't hw specific (and currently
> > just human doable because of the simple mapping).
>
> That's is not true. You don't define an IRQ number what you define is a GPIO
> number that is mapped as IRQ. The GPIO number does not depend on the mapping and
> it only depends on the GPIO controller.
>
> This has absolutely nothing to do with the reverted patches and is described in
> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
>
> The only difference is that the reverted patches did actually take an action
> when a GPIO pin was mapped as an IRQ (requesting the GPIO and as input).
>
> So for example in an OMAP board DT you can define something like this:
>
> ethernet@5,0 {
>          compatible = "smsc,lan9221", "smsc,lan9115";
>          interrupt-parent = <&gpio6>;
>          interrupts = <16 8>;
> };
>
> Since each OMAP GPIO bank has 32 GPIO pins, then what you are defining is that
> the GPIO 176 (5 * 32 + 16) will be mapped as the IRQ line for the ethernet
> controller.
>
> I explained the exact use case I'm trying to solve in the thread "Re: [PATCH v3]
> gpio: interrupt consistency check for OF GPIO IRQs" [1] if you need more
> context. I'm sure others cc'ed in this thread have different (but similar)
> requirements.

So I would extend my previous proposal
(http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg95202.html)
for a gpio_request_as_irq() such, that I would change that to 
request_irq_new(number, irq_controller) where the irq_controller would 
be the gpio-controller in case of omap (while marking gpio_to_irq() as 
deprecated).

So request_irq_new() would than do the following on omap if the 
irq-controller is a gpio-controller:

gpio_request() // This works only if the gpio was not requested before
gpio_direction_input()
(build irq-mapping)
gpio_to_irq() // This needs an irq-mapping
return request_threaded_irq()

and all drivers could replace the above sequence just with 
request_irq_new(number_from_dt, irq-controller_from_dt).

How's that?

Regards,

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