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

[Linus Walleij]

On Wed, Mar 27, 2013 at 5:09 PM, Stephen Warren <swarren at 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.

> 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.

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)
{
     struct resource *r;
     int irq;
     int err;

     r = devm_kmalloc(&pdev->dev, sizeof(*r), GFP_KERNEL);
     if (!r)
        return -ENOMEM;
     memset(r, 0, sizeof(*r));
     err = request_gpio(gpio);
     if (err)
        return err;
     irq = gpio_to_irq(gpio);
     if (irq < 0)
        return irq;
     r->start = irq;
     r->end = irq;
     r->flags = IORESOURCE_IRQ | flags;
     pdev->num_resources = 1;
     pdev->resource = res;
};

int gpio_populate_irq_resource(struct platform_device *pdev, int gpio,
                              unsigned flags, struct resource *r)
{
     int irq;
     int err;

     err = request_gpio(gpio);
     if (err)
        return err;
     irq = gpio_to_irq(gpio);
     if (irq < 0)
        return irq;
     r->start = irq;
     r->end = irq;
     r->flags = IORESOURCE_IRQ | flags;
};


(Add permutations for either filling in a struct resource *r from
the board code, or krealloc() to extend an existing dynamic allocation
or whatever, surely these helpers can be written.)

So this is for a pure platform data case (based on
arch/arm/mach-ux500/board-mop500.c, which currently
relies on hard-coded GPIO and IRQ numbers):

static struct resource ethernet_res[] = {
        {
                .name = "smsc911x-memory",
                .start = (0x5000 << 16),
                .end  =  (0x5000 << 16) + 0xffff,
                .flags = IORESOURCE_MEM,
        },
        {  }, /* To be filled in with GPIO IRQ */
};

static struct platform_device ethernet = {
        .name           = "smsc911x",
        .dev            = {
                .platform_data = &....,
        },
};

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.

I can implement this and convert the Snowball if it
helps.

> 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.
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.

> And finally, even if we ignore dedicated IRQ input pins, and assume that
> every IRQ input is really a GPIO, why should the driver be forced to
> call both request_irq() and gpio_request(); that's something that should
> really be dealt with inside the IRQ subsystem or relevant IRQ driver.
> Otherwise, every other driver that uses IRQs has to be burdened with the
> code to do that.

I agree it need not be in the driver if all it wants is an IRQ.

I was thinking more like the above.

Yours,
Linus Walleij