Subject: [PATCH] gpio: pl061: add DT binding support

From: [email-masked] (Rob Herring)
Date: Wed, 03 Aug 2011 18:18:45 -0500

Grant,

On 08/03/2011 05:22 PM, Grant Likely wrote:
> On Wed, Aug 3, 2011 at 7:54 PM, Rob Herring <[email-masked]> wrote:
>> From: Rob Herring <[email-masked]>
>>
>> This adds devicetree binding support to the ARM pl061 driver removing the
>> platform_data dependency. When DT binding is used, the gpio numbering is
>> assigned dynamically. The interrupt assignment is converted to use the
>> irq_domain infrastructure.
>>
>> Signed-off-by: Rob Herring <[email-masked]>
>> Cc: Grant Likely <[email-masked]>
>> ---
>>  drivers/gpio/gpio-pl061.c |   32 +++++++++++++++++++++++---------
>>  1 files changed, 23 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
>> index 2c5a18f..7ea74ff 100644
>> --- a/drivers/gpio/gpio-pl061.c
>> +++ b/drivers/gpio/gpio-pl061.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/amba/bus.h>
>>  #include <linux/amba/pl061.h>
>>  #include <linux/slab.h>
>> +#include <linux/of_irq.h>
>>
>>  #define GPIODIR 0x400
>>  #define GPIOIS  0x404
>> @@ -246,6 +247,20 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
>>        if (chip == NULL)
>>                return -ENOMEM;
>>
>> +       pdata = dev->dev.platform_data;
>> +       if (pdata) {
>> +               chip->gc.base = pdata->gpio_base;
>> +               chip->irq_base = pdata->irq_base;
>> +       } else if (dev->dev.of_node) {
>> +               u32 intspec = 0;
>> +               chip->gc.base = -1;
>> +               chip->irq_base = irq_create_of_mapping(dev->dev.of_node,
>> +                                                      &intspec, 1);
> 
> This looks wrong.  intspec is always 0 here, when I would assume you
> would want the value of the interrupts property from this device's
> node.  Is this the cascade irq number?  Or is it supposed to be the
> starting interrupt number for the gpios?  If it is starting interrupt
> number, then it should actually be dynamically assigned.  If it is the
> cascade, then platform_get_irq() should work.
> 
It's the latter case that I need to retrieve. I have this platform code
assigning the linux irq numbers:

        struct device_node *node;
        int n = 0;

        node = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
        if (!node)
                panic("missing gic devicetree node\n");
        irq_domain_add_simple(node, 0);

        for_each_compatible_node(node, NULL, "arm,pl061") {
                irq_domain_add_simple(node, 160 + (8 * n));
                n++;
        }

In this case with simple translation, intspec is really a don't care.
irq_create_of_mapping just finds the domain that matches the gpio
ctrlr's node and returns the domain's irq_base (+ 0) as the translate
function will just return the intspec value for the hw_irq.

Have I missed something? There is not yet any dynamic assignment of
linux irq numbers?

Rob