Re: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver

[Stephen Warren <swarren@wwwdotorg.org>]

On 03/27/2013 02:55 PM, Linus Walleij wrote:
> On Wed, Mar 27, 2013 at 5:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
>> It's simply that if a device emits an IRQ, there's no reason to assume
>> that the IRQ is in fact a GPIO. It might be a dedicated IRQ input pin
>> and not something that gpiolib (or any other GPIO API) knows about.
> 
> OK (following the first example, that's what I have in the Nomadik
> or Snowball already).
> 
>> Another possibility:
>>
>> (Device IRQ output wired directly into dedicated IRQ input pin, and that
>> pin has no GPIO capabilities)
>>
>> +----------------------------+
>> | SoC                        |         +--------+
>> |      IRQ 5                 | DEV_IRQ | Device |
>> | GIC <----------------------|---------|-IRQ    |
>> |                            |         +--------+
>> +----------------------------+
>>
>> As such, the driver /must/ receive an "int irq" in platform data.
> 
> Nah. Lets pass a struct resource * of type IORESOURCE_IRQ.
> But I understand the necessity to pass the IRQ number somehow.

Oh sure. I tend to consider the resources part of platform data,
although I agree they really aren't.

>> If it
>> received an "int gpio", then there would be no way for the driver to
>> support boards that routed that interrupt signal to a dedicated IRQ pin
>> on the SoC, rather than to a GPIO pin that just happened to have
>> interrupt generation capabilities.
> 
> This is the case for some SMSC911x clients like the snowball
> routing it to a GPIO, whereas I think the RealView and Integrator/CP
> has a dedicated IRQ line for it on the FPGA so it's a good example.
> 
> In such cases the right thing to do is for the platform
> code populating the platform data with the int irq field, or device tree
> core, or whatever piece of code that knows about
> the actual GPIO shall start from the GPIO, request it and
> then convert it to an IRQ.

So board code could easily do that; plenty of opportunity to put
whatever custom code is needed right there.

For DT core code to do that, we'd need some alternative way of
specifying interrupts. That would change the DT bindings for interrupts.
I don't think we can do that...

> If it seems like identical boilerplate in every machine we can
> simply centralize it to gpiolib. Something like
> 
> int gpio_add_single_irq_resource(struct platform_device *pdev, int
> gpio, unsigned flags)
... [code to convert GPIO to IRQ and create IORESOURCE_IRQ from it]

> int gpio_populate_irq_resource(struct platform_device *pdev, int gpio,
>                               unsigned flags, struct resource *r)
... [code to create IORESOURCE_IRQ from the IRQ]

...
> gpio_populate_irq_resource(&ethernet, 17,
>      IORESOURCE_IRQ_HIGHEDGE, &ethernet_res[1]);
> platform_device_register(&ethernet);
> 
> That populates the device with the single GPIO IRQ from
> the GPIO number in a smooth way.
> 
> Of course it has to be done after the GPIO driver is up
> and running so may require some other refactoring,
> but it should work.

That latter issue also somewhat scuppers this approach; then you have to
somehow run a bunch of your board file code inter-mixed with various
driver probe() calls. That will quickly get nasy.

And it doesn't address how the DT core will know when to call
gpio_add_single_irq_resource() vs. gpio_populate_irq_resource() without
changing the DT binding for interrupts.

>> And then, given that irq_to_gpio() isn't semantically possible, there's
>> no way for the driver to be able to gpio_request() anything, since it
>> doesn't, can't, and shouldn't know whether there is in fact any GPIO ID
>> that it should request, let alone what that GPIO ID is.
> 
> That's cool. The driver does not have to know the IRQ is backed by a
> GPIO, but somewhere in the system something should know this.

Something already does; the irqchip driver for the IRQ in question would
always know whether it's programming a plain IRQ controller, or an IRQ
controller that's part of a GPIO controller. In the GPIO case, the
irqchip driver also has enought HW-specific information to perform
whatever GPIO<->IRQ number conversion is needed.

> Such as board code or DT core (I'm thinking drivers/of/platform.c code
> calling gpiolib-of.c to do this mapping for DT, for example).
> 
> And by introducing the requirement that such GPIO lines be
> GPIO input hogs in the device tree else warn we can get a nice
> closure in the DT case: the GPIO subsystem knows the actual
> GPIO line as a hog that is request and sets as input at probe,
> the referencing device node only gets the IRQ but at runtime
> the device node instatiation for the IRQ consumer will ask
> the gpiolib to check its hog list to make sure one of these will
> match that IRQ, then request it.

"Hogs" sounds like pinctrl. pinctrl doesn't support the concept of hogs
on GPIOs, since it only knows about pins not GPIOs really; even with the
integration with gpiolib, pinctrl gets informed when GPIOs are
requested, not when setting up pinctrl states ahead of time, and also
this notification is only so we can mark the pin related to the GPIO
in-use, not really to actually hog it. If pinctrl started hogging GPIOs,
I think me might end up with both pinctrl and gpiolib calling
each-other, and hence have nasty circular dependencies.

Or, were you thinking of creating some kind of hogging system for
gpiolib itself?