[PATCH 02/16] drivers/gpio: gpio-nomadik: Add support for irqdomains

[Linus Walleij]

On Tue, Apr 17, 2012 at 12:43 PM, Lee Jones <lee.jones at linaro.org> wrote:

> diff --git a/arch/arm/mach-ux500/cpu.c b/arch/arm/mach-ux500/cpu.c
> index d11f389..af12139 100644
> --- a/arch/arm/mach-ux500/cpu.c
> +++ b/arch/arm/mach-ux500/cpu.c
> @@ -30,6 +30,7 @@
>
>  void __iomem *_PRCMU_BASE;
>
> +/* FIXME: should we set up the GPIO domain here? */

Why on earth would we want to do that?

Get rid of the comment simply...

> diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> index 2c2b53c..1322ca8 100644
> --- a/drivers/gpio/gpio-nomadik.c
> +++ b/drivers/gpio/gpio-nomadik.c
> @@ -22,6 +22,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> +#include <linux/irqdomain.h>
>  #include <linux/slab.h>
>
>  #include <asm/mach/irq.h>
> @@ -677,7 +678,6 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>        bitmask = nmk_gpio_get_bitmask(gpio);
>        if (!nmk_chip)
>                return -EINVAL;
> -
>        if (type & IRQ_TYPE_LEVEL_HIGH)
>                return -EINVAL;
>        if (type & IRQ_TYPE_LEVEL_LOW)
> @@ -782,6 +782,33 @@ static void nmk_gpio_secondary_irq_handler(unsigned int irq,
>        __nmk_gpio_irq_handler(irq, desc, status);
>  }
>
> +#ifdef CONFIG_IRQ_DOMAIN

No, modify Kconfig to select IRQ_DOMAIN for the driver instead.
We want this also for the non-DT case.

> +int nmk_gpio_irq_map(struct irq_domain *d, unsigned int irq,
> +                         irq_hw_number_t hwirq)
> +{
> +       return 0;
> +}
> +
> +const struct irq_domain_ops nmk_gpio_irq_simple_ops = {
> +       .map = nmk_gpio_irq_map,
> +       .xlate = irq_domain_xlate_twocell,
> +};

This is just a copy of irq_domain_simple_ops so get rid of that
and use the simple ops directly.

> +
> +struct irq_domain *nmk_gpio_irq_domain_add(struct device_node *np,
> +                                       struct nmk_gpio_platform_data *pdata)
> +{
> +       return irq_domain_add_legacy(np, NMK_GPIO_PER_CHIP,
> +                       NOMADIK_GPIO_TO_IRQ(pdata->first_gpio),
> +                       0, &nmk_gpio_irq_simple_ops, NULL);

irq_domain_simple_ops then.

> +}
> +#else
> +struct irq_domain *nmk_gpio_irq_domain_add(struct device_node *np,
> +                                       struct nmk_gpio_platform_data *pdata)
> +{
> +       return NULL;
> +}
> +#endif

And since this is not going to be #ifdef:ed, just inline it.

> +
>  static int nmk_gpio_init_irq(struct nmk_gpio_chip *nmk_chip)
>  {
>        unsigned int first_irq;
> @@ -1072,6 +1099,7 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev)
>  {
>        struct nmk_gpio_platform_data *pdata = dev->dev.platform_data;
>        struct device_node *np = dev->dev.of_node;
> +       struct irq_domain *domain = NULL;
>        struct nmk_gpio_chip *nmk_chip;
>        struct gpio_chip *chip;
>        struct resource *res;
> @@ -1096,13 +1124,20 @@ static int __devinit nmk_gpio_probe(struct platform_device *dev)
>                if (of_property_read_u32(np, "gpio-bank", &dev->id)) {
>                        dev_err(&dev->dev, "gpio-bank property not found\n");
>                        ret = -EINVAL;
> -                       goto out_dt;
> +                       goto out;
>                }
>
>                pdata->first_gpio = dev->id * NMK_GPIO_PER_CHIP;
>                pdata->num_gpio   = NMK_GPIO_PER_CHIP;
>        }
>
> +       domain = nmk_gpio_irq_domain_add(np, pdata);
> +       if (!domain) {
> +               pr_err("%s: Failed to create irqdomain - required for DT\n",

Not just for DT, let's do this always.

> +                       np->full_name);
> +               return -ENOSYS;
> +       }
> +
>        res = platform_get_resource(dev, IORESOURCE_MEM, 0);
>        if (!res) {
>                ret = -ENOENT;
> @@ -1189,7 +1224,6 @@ out_release:
>  out:
>        dev_err(&dev->dev, "Failure %i for GPIO %i-%i\n", ret,
>                  pdata->first_gpio, pdata->first_gpio+31);
> -out_dt:
>        if (np)
>                kfree(pdata);

This patch is not complete:

__nmk_gpio_irq_handler() and friends need to be rewritten to use the mapper.
i.e. no first_irq = NOMADIK_GPIO_TO_IRQ() etc in the interrupt handler, use the
domain to translate the HW IRQ using irq_find_mapping(domain, irq), that's the
whole point of the domain IIRC.

Yours,
Linus Walleij