| Message ID | 1312397661-3328-1-git-send-email-robherring2@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rob, On Wed, Aug 03, 2011 at 01:54:21PM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > 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 <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > drivers/gpio/gpio-pl061.c | 32 +++++++++++++++++++++++--------- > 1 files changed, 23 insertions(+), 9 deletions(-) > [snip] > @@ -283,7 +295,7 @@ static int pl061_probe(struct amba_device *dev, const > struct amba_id *id) > * irq_chip support > */ > > - if (chip->irq_base == (unsigned) -1) > + if (chip->irq_base == NO_IRQ) Please update the comment at include/linux/amba/pl061.h as well. > return 0; > > writeb(0, chip->base + GPIOIE); /* disable irqs */ > @@ -307,11 +319,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) [snip] baruch
On Wed, Aug 3, 2011 at 7:54 PM, Rob Herring <robherring2@gmail.com> wrote: > From: Rob Herring <rob.herring@calxeda.com> > > 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 <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > 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. g.
Grant, On 08/03/2011 05:22 PM, Grant Likely wrote: > On Wed, Aug 3, 2011 at 7:54 PM, Rob Herring <robherring2@gmail.com> wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> 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 <rob.herring@calxeda.com> >> Cc: Grant Likely <grant.likely@secretlab.ca> >> --- >> 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
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); + } else { + ret = -ENODEV; + goto free_mem; + } + if (!request_mem_region(dev->res.start, resource_size(&dev->res), "pl061")) { ret = -EBUSY; @@ -267,14 +282,11 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) chip->gc.get = pl061_get_value; chip->gc.set = pl061_set_value; chip->gc.to_irq = pl061_to_irq; - chip->gc.base = pdata->gpio_base; chip->gc.ngpio = PL061_GPIO_NR; chip->gc.label = dev_name(&dev->dev); chip->gc.dev = &dev->dev; chip->gc.owner = THIS_MODULE; - chip->irq_base = pdata->irq_base; - ret = gpiochip_add(&chip->gc); if (ret) goto iounmap; @@ -283,7 +295,7 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) * irq_chip support */ - if (chip->irq_base == (unsigned) -1) + if (chip->irq_base == NO_IRQ) return 0; writeb(0, chip->base + GPIOIE); /* disable irqs */ @@ -307,11 +319,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) list_add(&chip->list, chip_list); for (i = 0; i < PL061_GPIO_NR; i++) { - if (pdata->directions & (1 << i)) - pl061_direction_output(&chip->gc, i, - pdata->values & (1 << i)); - else - pl061_direction_input(&chip->gc, i); + if (pdata) { + if (pdata->directions & (1 << i)) + pl061_direction_output(&chip->gc, i, + pdata->values & (1 << i)); + else + pl061_direction_input(&chip->gc, i); + } irq_set_chip_and_handler(i + chip->irq_base, &pl061_irqchip, handle_simple_irq);