[PATCH 0/5] GPIO OMAP driver changes for v3.16

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Javier Martinez Canillas]

Hi Linus,

Now that you have sent your changes for v3.15 to Torvalds, here are some
changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements
so nothing here is -rc material.

The biggest change is Patch 4 that converts the driver to use the newly
introduced generic irqchip helpers in the gpiolib core which allows to
remove some driver specific logic that should really be generic.

Aaro and Paul,

I've just build tested using omap1_defconfig since I don't have any OMAP1
board to test. It would be nice if you could test using any of your boards
to see if this patch-set does not introduce a regression. Specially on those
using MPUIO interrupt generation since I'm not familiar with it and I could
get something wrong while studying the code.

I prefer this set to not be merged until we have some testing on OMAP1 SoCs.

Santosh and Kevin,

If you agree I would like to be added to the list of maintainers for this
driver since that is what I've been doing lately anyways and that way tools
like get_maintainer.pl will hint people posting patches that I'm interested
in being cc'ed for this driver.

The patch-set is composed of the following patches: 

Javier Martinez Canillas (5):
  gpio: omap: convert to use irq_domain_add_simple()
  gpio: omap: check gpiochip_add() return value
  gpio: omap: add a GPIO_OMAP option instead of using ARCH_OMAP
  gpio: omap: convert driver to use gpiolib irqchip
  MAINTAINERS: update GPIO OMAP driver entry

 MAINTAINERS              |   1 +
 drivers/gpio/Kconfig     |   8 +++
 drivers/gpio/Makefile    |   2 +-
 drivers/gpio/gpio-omap.c | 128 +++++++++++++++++++++++------------------------
 4 files changed, 73 insertions(+), 66 deletions(-)

Thanks a lot and best regards,
Javier

-- 
1.9.0

[PATCH 1/5] gpio: omap: convert to use irq_domain_add_simple()

[Javier Martinez Canillas]

The GPIO OMAP driver supports different OMAP SoC families and
not all of them have the needed support to use the linear IRQ
domain mapping like OMAP1 that use the legacy domain mapping.

But this special check is not necessary since the simple IRQ
domain mapping is able to handle both cases. Having a zero
IRQ offset will be interpreted as a linear domain case while
a non-zero value will be interpreted as a legacy domain case.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/gpio/gpio-omap.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 19b886c..3ee9b8d 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1138,9 +1138,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	const struct omap_gpio_platform_data *pdata;
 	struct resource *res;
 	struct gpio_bank *bank;
-#ifdef CONFIG_ARCH_OMAP1
-	int irq_base;
-#endif
+	int irq_base = 0;
 
 	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
 
@@ -1185,21 +1183,16 @@ static int omap_gpio_probe(struct platform_device *pdev)
 #ifdef CONFIG_ARCH_OMAP1
 	/*
 	 * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
-	 * irq_alloc_descs() and irq_domain_add_legacy() and just use a
-	 * linear IRQ domain mapping for all OMAP platforms.
+	 * irq_alloc_descs() since a base IRQ offset will no longer be needed.
 	 */
 	irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
 	if (irq_base < 0) {
 		dev_err(dev, "Couldn't allocate IRQ numbers\n");
 		return -ENODEV;
 	}
-
-	bank->domain = irq_domain_add_legacy(node, bank->width, irq_base,
-					     0, &irq_domain_simple_ops, NULL);
-#else
-	bank->domain = irq_domain_add_linear(node, bank->width,
-					     &irq_domain_simple_ops, NULL);
 #endif
+	bank->domain = irq_domain_add_simple(node, bank->width, irq_base,
+					     &irq_domain_simple_ops, NULL);
 	if (!bank->domain) {
 		dev_err(dev, "Couldn't register an IRQ domain\n");
 		return -ENODEV;
-- 
1.9.0

[PATCH 1/5] gpio: omap: convert to use irq_domain_add_simple()

[Santosh Shilimkar]

On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
> The GPIO OMAP driver supports different OMAP SoC families and
> not all of them have the needed support to use the linear IRQ
> domain mapping like OMAP1 that use the legacy domain mapping.
> 
> But this special check is not necessary since the simple IRQ
> domain mapping is able to handle both cases. Having a zero
> IRQ offset will be interpreted as a linear domain case while
> a non-zero value will be interpreted as a legacy domain case.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/gpio/gpio-omap.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 19b886c..3ee9b8d 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1138,9 +1138,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  	const struct omap_gpio_platform_data *pdata;
>  	struct resource *res;
>  	struct gpio_bank *bank;
> -#ifdef CONFIG_ARCH_OMAP1
> -	int irq_base;
> -#endif
> +	int irq_base = 0;
>  
>  	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
>  
> @@ -1185,21 +1183,16 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  #ifdef CONFIG_ARCH_OMAP1
>  	/*
>  	 * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
> -	 * irq_alloc_descs() and irq_domain_add_legacy() and just use a
> -	 * linear IRQ domain mapping for all OMAP platforms.
> +	 * irq_alloc_descs() since a base IRQ offset will no longer be needed.
>  	 */
>  	irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
>  	if (irq_base < 0) {
>  		dev_err(dev, "Couldn't allocate IRQ numbers\n");
>  		return -ENODEV;
>  	}
> -
> -	bank->domain = irq_domain_add_legacy(node, bank->width, irq_base,
> -					     0, &irq_domain_simple_ops, NULL);
> -#else
> -	bank->domain = irq_domain_add_linear(node, bank->width,
> -					     &irq_domain_simple_ops, NULL);
>  #endif
> +	bank->domain = irq_domain_add_simple(node, bank->width, irq_base,
> +					     &irq_domain_simple_ops, NULL);
>  	if (!bank->domain) {
>  		dev_err(dev, "Couldn't register an IRQ domain\n");
>  		return -ENODEV;
> 
Looks good.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 2/5] gpio: omap: check gpiochip_add() return value

[Javier Martinez Canillas]

The gpiochip_add() function can fail if the chip cannot
be registered so the return value has to be checked and
the error propagated in case it happens.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/gpio/gpio-omap.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 3ee9b8d..e717888 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1081,10 +1081,11 @@ omap_mpuio_alloc_gc(struct gpio_bank *bank, unsigned int irq_start,
 			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
 }
 
-static void omap_gpio_chip_init(struct gpio_bank *bank)
+static int omap_gpio_chip_init(struct gpio_bank *bank)
 {
 	int j;
 	static int gpio;
+	int ret;
 
 	/*
 	 * REVISIT eventually switch from OMAP-specific gpio structs
@@ -1110,7 +1111,11 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
 	}
 	bank->chip.ngpio = bank->width;
 
-	gpiochip_add(&bank->chip);
+	ret = gpiochip_add(&bank->chip);
+	if (ret) {
+		dev_err(bank->dev, "Could not register gpio chip\n", ret);
+		return ret;
+	}
 
 	for (j = 0; j < bank->width; j++) {
 		int irq = irq_create_mapping(bank->domain, j);
@@ -1139,6 +1144,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct gpio_bank *bank;
 	int irq_base = 0;
+	int ret;
 
 	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
 
@@ -1223,7 +1229,11 @@ static int omap_gpio_probe(struct platform_device *pdev)
 		mpuio_init(bank);
 
 	omap_gpio_mod_init(bank);
-	omap_gpio_chip_init(bank);
+
+	ret = omap_gpio_chip_init(bank);
+	if (ret)
+		return ret;
+
 	omap_gpio_show_rev(bank);
 
 	pm_runtime_put(bank->dev);
-- 
1.9.0

[PATCH 2/5] gpio: omap: check gpiochip_add() return value

[Santosh Shilimkar]

On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
> The gpiochip_add() function can fail if the chip cannot
> be registered so the return value has to be checked and
> the error propagated in case it happens.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 3/5] gpio: omap: add a GPIO_OMAP option instead of using ARCH_OMAP

[Javier Martinez Canillas]

The ARCH_OMAP config option was used to built the GPIO OMAP
driver but this is not consistent with the rest of the GPIO
drivers that have their own Kconfig option.

Also, this make it harder to add dependencies or reverse
dependencies (i.e: select) since that would mean touching the
sub-arch config option.

So is better to add a boolean Kconfig option for this driver
that defaults to true if ARCH_OMAP is enabled.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/gpio/Kconfig  | 7 +++++++
 drivers/gpio/Makefile | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 92d8e9a..bfe5cf0 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -243,6 +243,13 @@ config GPIO_OCTEON
 	  Say yes here to support the on-chip GPIO lines on the OCTEON
 	  family of SOCs.
 
+config GPIO_OMAP
+	bool "TI OMAP GPIO support"
+	default y if ARCH_OMAP
+	depends on ARM && ARCH_OMAP
+	help
+	  Say yes here to enable GPIO support for TI OMAP SoCs.
+
 config GPIO_PL061
 	bool "PrimeCell PL061 GPIO support"
 	depends on ARM_AMBA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 6309aff..d10f6a9 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -58,7 +58,7 @@ obj-$(CONFIG_GPIO_MVEBU)        += gpio-mvebu.o
 obj-$(CONFIG_GPIO_MXC)		+= gpio-mxc.o
 obj-$(CONFIG_GPIO_MXS)		+= gpio-mxs.o
 obj-$(CONFIG_GPIO_OCTEON)	+= gpio-octeon.o
-obj-$(CONFIG_ARCH_OMAP)		+= gpio-omap.o
+obj-$(CONFIG_GPIO_OMAP)		+= gpio-omap.o
 obj-$(CONFIG_GPIO_PCA953X)	+= gpio-pca953x.o
 obj-$(CONFIG_GPIO_PCF857X)	+= gpio-pcf857x.o
 obj-$(CONFIG_GPIO_PCH)		+= gpio-pch.o
-- 
1.9.0

[PATCH 3/5] gpio: omap: add a GPIO_OMAP option instead of using ARCH_OMAP

[Santosh Shilimkar]

On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
> The ARCH_OMAP config option was used to built the GPIO OMAP
> driver but this is not consistent with the rest of the GPIO
> drivers that have their own Kconfig option.
> 
> Also, this make it harder to add dependencies or reverse
> dependencies (i.e: select) since that would mean touching the
> sub-arch config option.
> 
> So is better to add a boolean Kconfig option for this driver
> that defaults to true if ARCH_OMAP is enabled.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 4/5] gpio: omap: convert driver to use gpiolib irqchip

[Javier Martinez Canillas]

Converts the GPIO OMAP driver to register its chained irq
handler and irqchip using the helpers in the gpiolib core.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/gpio/Kconfig     |   1 +
 drivers/gpio/gpio-omap.c | 107 ++++++++++++++++++++++-------------------------
 2 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bfe5cf0..2092b1d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -247,6 +247,7 @@ config GPIO_OMAP
 	bool "TI OMAP GPIO support"
 	default y if ARCH_OMAP
 	depends on ARM && ARCH_OMAP
+	select GPIOLIB_IRQCHIP
 	help
 	  Say yes here to enable GPIO support for TI OMAP SoCs.
 
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e717888..8cc9e91 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -24,7 +24,6 @@
 #include <linux/pm.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/irqdomain.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/gpio.h>
 #include <linux/platform_data/gpio-omap.h>
@@ -52,7 +51,6 @@ struct gpio_bank {
 	struct list_head node;
 	void __iomem *base;
 	u16 irq;
-	struct irq_domain *domain;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
 	struct gpio_regs context;
@@ -95,11 +93,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
 	return bank->chip.base + gpio_irq;
 }
 
-static int omap_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+static inline struct gpio_bank *_irq_data_get_bank(struct irq_data *d)
 {
-	struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
-
-	return irq_find_mapping(bank->domain, offset);
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	return container_of(chip, struct gpio_bank, chip);
 }
 
 static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
@@ -479,7 +476,7 @@ static int gpio_is_input(struct gpio_bank *bank, int mask)
 
 static int gpio_irq_type(struct irq_data *d, unsigned type)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned gpio = 0;
 	int retval;
 	unsigned long flags;
@@ -514,14 +511,6 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
 		return -EINVAL;
 	}
 
-	retval = gpio_lock_as_irq(&bank->chip, offset);
-	if (retval) {
-		dev_err(bank->dev, "unable to lock offset %d for IRQ\n",
-			offset);
-		spin_unlock_irqrestore(&bank->lock, flags);
-		return retval;
-	}
-
 	bank->irq_usage |= 1 << GPIO_INDEX(bank, gpio);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
@@ -664,7 +653,7 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
 /* Use disable_irq_wake() and enable_irq_wake() functions from drivers */
 static int gpio_wake_enable(struct irq_data *d, unsigned int enable)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 
 	return _set_gpio_wakeup(bank, gpio, enable);
@@ -732,11 +721,12 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	unsigned int bit;
 	struct gpio_bank *bank;
 	int unmasked = 0;
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct irq_chip *irqchip = irq_desc_get_chip(desc);
+	struct gpio_chip *chip = irq_get_handler_data(irq);
 
-	chained_irq_enter(chip, desc);
+	chained_irq_enter(irqchip, desc);
 
-	bank = irq_get_handler_data(irq);
+	bank = container_of(chip, struct gpio_bank, chip);
 	isr_reg = bank->base + bank->regs->irqstatus;
 	pm_runtime_get_sync(bank->dev);
 
@@ -764,7 +754,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 		configured, we could unmask GPIO bank interrupt immediately */
 		if (!level_mask && !unmasked) {
 			unmasked = 1;
-			chained_irq_exit(chip, desc);
+			chained_irq_exit(irqchip, desc);
 		}
 
 		if (!isr)
@@ -784,7 +774,8 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 			if (bank->toggle_mask & (1 << bit))
 				_toggle_gpio_edge_triggering(bank, bit);
 
-			generic_handle_irq(irq_find_mapping(bank->domain, bit));
+			generic_handle_irq(irq_find_mapping(bank->chip.irqdomain,
+							    bit));
 		}
 	}
 	/* if bank has any level sensitive GPIO pin interrupt
@@ -793,13 +784,13 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	interrupt */
 exit:
 	if (!unmasked)
-		chained_irq_exit(chip, desc);
+		chained_irq_exit(irqchip, desc);
 	pm_runtime_put(bank->dev);
 }
 
 static void gpio_irq_shutdown(struct irq_data *d)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 	unsigned long flags;
 	unsigned offset = GPIO_INDEX(bank, gpio);
@@ -821,7 +812,7 @@ static void gpio_irq_shutdown(struct irq_data *d)
 
 static void gpio_ack_irq(struct irq_data *d)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 
 	_clear_gpio_irqstatus(bank, gpio);
@@ -829,7 +820,7 @@ static void gpio_ack_irq(struct irq_data *d)
 
 static void gpio_mask_irq(struct irq_data *d)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 	unsigned long flags;
 
@@ -841,7 +832,7 @@ static void gpio_mask_irq(struct irq_data *d)
 
 static void gpio_unmask_irq(struct irq_data *d)
 {
-	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct gpio_bank *bank = _irq_data_get_bank(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 	unsigned int irq_mask = GPIO_BIT(bank, gpio);
 	u32 trigger = irqd_get_trigger_type(d);
@@ -1085,6 +1076,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
 {
 	int j;
 	static int gpio;
+	int irq_base = 0;
 	int ret;
 
 	/*
@@ -1098,7 +1090,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
 	bank->chip.direction_output = gpio_output;
 	bank->chip.set_debounce = gpio_debounce;
 	bank->chip.set = gpio_set;
-	bank->chip.to_irq = omap_gpio_to_irq;
 	if (bank->is_mpuio) {
 		bank->chip.label = "mpuio";
 		if (bank->regs->wkup_en)
@@ -1113,24 +1104,46 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
 
 	ret = gpiochip_add(&bank->chip);
 	if (ret) {
-		dev_err(bank->dev, "Could not register gpio chip\n", ret);
+		dev_err(bank->dev, "Could not register gpio chip %d\n", ret);
 		return ret;
 	}
 
+#ifdef CONFIG_ARCH_OMAP1
+	/*
+	 * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
+	 * irq_alloc_descs() since a base IRQ offset will no longer be needed.
+	 */
+	irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
+	if (irq_base < 0) {
+		dev_err(bank->dev, "Couldn't allocate IRQ numbers\n");
+		return -ENODEV;
+	}
+#endif
+
+	ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
+				   irq_base, gpio_irq_handler,
+				   IRQ_TYPE_NONE);
+
+	if (ret) {
+		dev_err(bank->dev, "Couldn't add irqchip to gpiochip %d\n", ret);
+		ret = gpiochip_remove(&bank->chip);
+		return -ENODEV;
+	}
+
+	gpiochip_set_chained_irqchip(&bank->chip, &gpio_irq_chip,
+				     bank->irq, gpio_irq_handler);
+
 	for (j = 0; j < bank->width; j++) {
-		int irq = irq_create_mapping(bank->domain, j);
+		int irq = irq_find_mapping(bank->chip.irqdomain, j);
 		irq_set_lockdep_class(irq, &gpio_lock_class);
-		irq_set_chip_data(irq, bank);
 		if (bank->is_mpuio) {
 			omap_mpuio_alloc_gc(bank, irq, bank->width);
-		} else {
-			irq_set_chip_and_handler(irq, &gpio_irq_chip,
-						 handle_simple_irq);
-			set_irq_flags(irq, IRQF_VALID);
+			irq_set_chip_and_handler(irq, NULL, NULL);
+			set_irq_flags(irq, 0);
 		}
 	}
-	irq_set_chained_handler(bank->irq, gpio_irq_handler);
-	irq_set_handler_data(bank->irq, bank);
+
+	return 0;
 }
 
 static const struct of_device_id omap_gpio_match[];
@@ -1143,7 +1156,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	const struct omap_gpio_platform_data *pdata;
 	struct resource *res;
 	struct gpio_bank *bank;
-	int irq_base = 0;
 	int ret;
 
 	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
@@ -1166,6 +1178,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 
 	bank->irq = res->start;
 	bank->dev = dev;
+	bank->chip.dev = dev;
 	bank->dbck_flag = pdata->dbck_flag;
 	bank->stride = pdata->bank_stride;
 	bank->width = pdata->bank_width;
@@ -1186,24 +1199,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
 				pdata->get_context_loss_count;
 	}
 
-#ifdef CONFIG_ARCH_OMAP1
-	/*
-	 * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
-	 * irq_alloc_descs() since a base IRQ offset will no longer be needed.
-	 */
-	irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
-	if (irq_base < 0) {
-		dev_err(dev, "Couldn't allocate IRQ numbers\n");
-		return -ENODEV;
-	}
-#endif
-	bank->domain = irq_domain_add_simple(node, bank->width, irq_base,
-					     &irq_domain_simple_ops, NULL);
-	if (!bank->domain) {
-		dev_err(dev, "Couldn't register an IRQ domain\n");
-		return -ENODEV;
-	}
-
 	if (bank->regs->set_dataout && bank->regs->clr_dataout)
 		bank->set_dataout = _set_gpio_dataout_reg;
 	else
@@ -1215,7 +1210,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	bank->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(bank->base)) {
-		irq_domain_remove(bank->domain);
+		irq_domain_remove(bank->chip.irqdomain);
 		return PTR_ERR(bank->base);
 	}
 
-- 
1.9.0

[PATCH 4/5] gpio: omap: convert driver to use gpiolib irqchip

[Santosh Shilimkar]

On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
> Converts the GPIO OMAP driver to register its chained irq
> handler and irqchip using the helpers in the gpiolib core.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/gpio/Kconfig     |   1 +
>  drivers/gpio/gpio-omap.c | 107 ++++++++++++++++++++++-------------------------
>  2 files changed, 52 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index bfe5cf0..2092b1d 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -247,6 +247,7 @@ config GPIO_OMAP
>  	bool "TI OMAP GPIO support"
>  	default y if ARCH_OMAP
>  	depends on ARM && ARCH_OMAP
> +	select GPIOLIB_IRQCHIP
>  	help
>  	  Say yes here to enable GPIO support for TI OMAP SoCs.
>  
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index e717888..8cc9e91 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -24,7 +24,6 @@
>  #include <linux/pm.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/irqdomain.h>
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/gpio.h>
>  #include <linux/platform_data/gpio-omap.h>
> @@ -52,7 +51,6 @@ struct gpio_bank {
>  	struct list_head node;
>  	void __iomem *base;
>  	u16 irq;
> -	struct irq_domain *domain;
>  	u32 non_wakeup_gpios;
>  	u32 enabled_non_wakeup_gpios;
>  	struct gpio_regs context;
> @@ -95,11 +93,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>  	return bank->chip.base + gpio_irq;
>  }
>  
> -static int omap_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +static inline struct gpio_bank *_irq_data_get_bank(struct irq_data *d)
>  {
> -	struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
> -
> -	return irq_find_mapping(bank->domain, offset);
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	return container_of(chip, struct gpio_bank, chip);
>  }
>  
>  static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
> @@ -479,7 +476,7 @@ static int gpio_is_input(struct gpio_bank *bank, int mask)
>  
>  static int gpio_irq_type(struct irq_data *d, unsigned type)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned gpio = 0;
>  	int retval;
>  	unsigned long flags;
> @@ -514,14 +511,6 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
>  		return -EINVAL;
>  	}
>  
> -	retval = gpio_lock_as_irq(&bank->chip, offset);
> -	if (retval) {
> -		dev_err(bank->dev, "unable to lock offset %d for IRQ\n",
> -			offset);
> -		spin_unlock_irqrestore(&bank->lock, flags);
> -		return retval;
> -	}
> -
>  	bank->irq_usage |= 1 << GPIO_INDEX(bank, gpio);
>  	spin_unlock_irqrestore(&bank->lock, flags);
>  
> @@ -664,7 +653,7 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
>  /* Use disable_irq_wake() and enable_irq_wake() functions from drivers */
>  static int gpio_wake_enable(struct irq_data *d, unsigned int enable)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
>  
>  	return _set_gpio_wakeup(bank, gpio, enable);
> @@ -732,11 +721,12 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
>  	unsigned int bit;
>  	struct gpio_bank *bank;
>  	int unmasked = 0;
> -	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct irq_chip *irqchip = irq_desc_get_chip(desc);
> +	struct gpio_chip *chip = irq_get_handler_data(irq);
>  
> -	chained_irq_enter(chip, desc);
> +	chained_irq_enter(irqchip, desc);
>  
> -	bank = irq_get_handler_data(irq);
> +	bank = container_of(chip, struct gpio_bank, chip);
>  	isr_reg = bank->base + bank->regs->irqstatus;
>  	pm_runtime_get_sync(bank->dev);
>  
> @@ -764,7 +754,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
>  		configured, we could unmask GPIO bank interrupt immediately */
>  		if (!level_mask && !unmasked) {
>  			unmasked = 1;
> -			chained_irq_exit(chip, desc);
> +			chained_irq_exit(irqchip, desc);
>  		}
>  
>  		if (!isr)
> @@ -784,7 +774,8 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
>  			if (bank->toggle_mask & (1 << bit))
>  				_toggle_gpio_edge_triggering(bank, bit);
>  
> -			generic_handle_irq(irq_find_mapping(bank->domain, bit));
> +			generic_handle_irq(irq_find_mapping(bank->chip.irqdomain,
> +							    bit));
>  		}
>  	}
>  	/* if bank has any level sensitive GPIO pin interrupt
> @@ -793,13 +784,13 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
>  	interrupt */
>  exit:
>  	if (!unmasked)
> -		chained_irq_exit(chip, desc);
> +		chained_irq_exit(irqchip, desc);
>  	pm_runtime_put(bank->dev);
>  }
>  
>  static void gpio_irq_shutdown(struct irq_data *d)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
>  	unsigned long flags;
>  	unsigned offset = GPIO_INDEX(bank, gpio);
> @@ -821,7 +812,7 @@ static void gpio_irq_shutdown(struct irq_data *d)
>  
>  static void gpio_ack_irq(struct irq_data *d)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
>  
>  	_clear_gpio_irqstatus(bank, gpio);
> @@ -829,7 +820,7 @@ static void gpio_ack_irq(struct irq_data *d)
>  
>  static void gpio_mask_irq(struct irq_data *d)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
>  	unsigned long flags;
>  
> @@ -841,7 +832,7 @@ static void gpio_mask_irq(struct irq_data *d)
>  
>  static void gpio_unmask_irq(struct irq_data *d)
>  {
> -	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +	struct gpio_bank *bank = _irq_data_get_bank(d);
>  	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
>  	unsigned int irq_mask = GPIO_BIT(bank, gpio);
>  	u32 trigger = irqd_get_trigger_type(d);
> @@ -1085,6 +1076,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>  {
>  	int j;
>  	static int gpio;
> +	int irq_base = 0;
>  	int ret;
>  
>  	/*
> @@ -1098,7 +1090,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>  	bank->chip.direction_output = gpio_output;
>  	bank->chip.set_debounce = gpio_debounce;
>  	bank->chip.set = gpio_set;
> -	bank->chip.to_irq = omap_gpio_to_irq;
>  	if (bank->is_mpuio) {
>  		bank->chip.label = "mpuio";
>  		if (bank->regs->wkup_en)
> @@ -1113,24 +1104,46 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>  
>  	ret = gpiochip_add(&bank->chip);
>  	if (ret) {
> -		dev_err(bank->dev, "Could not register gpio chip\n", ret);
> +		dev_err(bank->dev, "Could not register gpio chip %d\n", ret);
>  		return ret;
>  	}
>  
> +#ifdef CONFIG_ARCH_OMAP1
> +	/*
> +	 * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
> +	 * irq_alloc_descs() since a base IRQ offset will no longer be needed.
> +	 */
> +	irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
> +	if (irq_base < 0) {
> +		dev_err(bank->dev, "Couldn't allocate IRQ numbers\n");
> +		return -ENODEV;
> +	}
> +#endif
> +
Do we still need above after first patch ? Would be good
to get rid of above ugly #ifdef on the middle of the code.

[PATCH 4/5] gpio: omap: convert driver to use gpiolib irqchip

[Linus Walleij]

On Thu, Apr 10, 2014 at 7:39 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:

>> +#ifdef CONFIG_ARCH_OMAP1
>> +     /*
>> +      * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
>> +      * irq_alloc_descs() since a base IRQ offset will no longer be needed.
>> +      */
>> +     irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
>> +     if (irq_base < 0) {
>> +             dev_err(bank->dev, "Couldn't allocate IRQ numbers\n");
>> +             return -ENODEV;
>> +     }
>> +#endif
>> +
> Do we still need above after first patch ? Would be good
> to get rid of above ugly #ifdef on the middle of the code.

I don't think so actually, simple irqdomain adds descriptors
for irqbase != 0, and the gpiochip irqchip helpers call
irq_create_mapping() on all offsets.

Preferably a separate patch on top of this removing that code
though so it's bisectable.

Yours,
Linus Walleij

[PATCH 4/5] gpio: omap: convert driver to use gpiolib irqchip

[Javier Martinez Canillas]

Hello Linus and Santosh,

On Thu, Apr 10, 2014 at 7:45 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Apr 10, 2014 at 7:39 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>> On Sunday 06 April 2014 10:58 AM, Javier Martinez Canillas wrote:
>
>>> +#ifdef CONFIG_ARCH_OMAP1
>>> +     /*
>>> +      * REVISIT: Once we have OMAP1 supporting SPARSE_IRQ, we can drop
>>> +      * irq_alloc_descs() since a base IRQ offset will no longer be needed.
>>> +      */
>>> +     irq_base = irq_alloc_descs(-1, 0, bank->width, 0);
>>> +     if (irq_base < 0) {
>>> +             dev_err(bank->dev, "Couldn't allocate IRQ numbers\n");
>>> +             return -ENODEV;
>>> +     }
>>> +#endif
>>> +
>> Do we still need above after first patch ? Would be good
>> to get rid of above ugly #ifdef on the middle of the code.

When working on this series I tried to remove the #ifdef but as far as
I understood is not currently possible until OMAP1 supports sparse irq
numbering.

Not so long ago the GPIO OMAP driver used the legacy domain mapping
for all OMAP SoCs and Jon Hunter converted to use linear domain
mapping on commit ede4d7a ("gpio/omap: convert gpio irq domain to
linear mapping").

Unfortunately that change caused a regression on OMAP1 platforms as
reported by Aaro [0] so I had to partially revert the linear domain
mapping for OMAP1 platforms on commit 397ead ("gpio/omap: don't use
linear domain mapping for OMAP1") introducing this ugly ifdefery in
the middle of the code.

>
> I don't think so actually, simple irqdomain adds descriptors
> for irqbase != 0, and the gpiochip irqchip helpers call
> irq_create_mapping() on all offsets.
>

Looking at irq_domain_add_simple() [1] I see that it only calls
irq_alloc_descs() and irq_domain_associate_many() if first_irq > 0 and
CONFIG_SPARSE_IRQ is enabled (which is not the case for
omap1_defconfig).

So, if I got this correctly removing the #ifdef for OMAP1 and calling
irq_domain_add_simple() is functionally equivalent to what Jon's patch
did that broke omap1. That is would have the same effect that just
calling irq_domain_add_linear() [2] for OMAP1.

> Preferably a separate patch on top of this removing that code
> though so it's bisectable.
>

If I remember correctly we did that partial revert because we were in
a -rc cycle and I didn't have time to investigate why
irq_domain_add_linear() does not work on omap1. I could try to do this
as a follow up patch but unfortunately I don't have access to any
omap1 platform to do further debug.

Please let me know if I got something wrong while looking at the code.

> Yours,
> Linus Walleij
> --

Thanks a lot and best regards,
Javier

[0]: http://www.mail-archive.com/linux-omap at vger.kernel.org/msg89005.html
[1]: http://lxr.free-electrons.com/source/kernel/irq/irqdomain.c#L123
[2]: http://lxr.free-electrons.com/source/include/linux/irqdomain.h#L139

[PATCH 5/5] MAINTAINERS: update GPIO OMAP driver entry

[Javier Martinez Canillas]

I've been maintaining this driver by fixing all issues
found while migrating OMAP2+ towards Device Tree based
booting and keeping it up-to-date by using the latest
infraestructure that is provided by the GPIO subsystem.

It would be nice if people know that I care about this
driver and put me in copy when sending patches for it.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f728ac2..fc0972e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6374,6 +6374,7 @@ F:	drivers/usb/*/*omap*
 F:	arch/arm/*omap*/usb*
 
 OMAP GPIO DRIVER
+M:	Javier Martinez Canillas <javier@dowhile0.org>
 M:	Santosh Shilimkar <santosh.shilimkar@ti.com>
 M:	Kevin Hilman <khilman@deeprootsystems.com>
 L:	linux-omap at vger.kernel.org
-- 
1.9.0

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Linus Walleij]

On Sun, Apr 6, 2014 at 4:58 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> Now that you have sent your changes for v3.15 to Torvalds, here are some
> changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements
> so nothing here is -rc material.

I like this series so I have applied them for v3.16, pending some ACK
from Kevin &| Santosh.

> The biggest change is Patch 4 that converts the driver to use the newly
> introduced generic irqchip helpers in the gpiolib core which allows to
> remove some driver specific logic that should really be generic.

Excellent, I will take a closer look at this, it seems there is some
cruft still in but let me look.

Yours,
Linus Walleij

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Aaro Koskinen]

Hi,

On Thu, Apr 10, 2014 at 07:29:26PM +0200, Linus Walleij wrote:
> On Sun, Apr 6, 2014 at 4:58 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
> 
> > Now that you have sent your changes for v3.15 to Torvalds, here are some
> > changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements
> > so nothing here is -rc material.
> 
> I like this series so I have applied them for v3.16, pending some ACK
> from Kevin &| Santosh.

I tried these patches on OMAP1 on top of today's Torvalds git
(4ba85265790ba3681deeaf73f018c0eb829a7341).

On Amstrad E3 I'm getting the following logs:

[    0.156491] omap_gpio omap_gpio.0: Runtime PM disabled, clock forced on.
[    0.164604] genirq: Setting trigger mode 0 for irq 64 failed (gpio_irq_type+0x0/0x1f0)
[    0.165418] genirq: Setting trigger mode 0 for irq 65 failed (gpio_irq_type+0x0/0x1f0)
[    0.166133] genirq: Setting trigger mode 0 for irq 66 failed (gpio_irq_type+0x0/0x1f0)
[    0.166838] genirq: Setting trigger mode 0 for irq 67 failed (gpio_irq_type+0x0/0x1f0)
[...]
[    0.182856] genirq: Setting trigger mode 0 for irq 79 failed (gpio_irq_type+0x0/0x1f0)
[    0.186887] omap_gpio omap_gpio.1: Could not get gpio dbck
[    0.189308] genirq: Setting trigger mode 0 for irq 95 failed (gpio_irq_type+0x0/0x1f0)
[...]
[    0.203121] genirq: Setting trigger mode 0 for irq 110 failed (gpio_irq_type+0x0/0x1f0)

However it still seems to work. The serio is only GPIO IRQ and it
triggers when I press the external keyboard.

The same happens also on Nokia 770:

[    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
[    0.119201] genirq: Setting trigger mode 0 for irq 129 failed (gpio_irq_type+0x0/0x220)
[...]
[    0.124999] genirq: Setting trigger mode 0 for irq 143 failed (gpio_irq_type
+0x0/0x220)
[    0.126831] omap_gpio omap_gpio.1: Could not get gpio dbck
[    0.127258] OMAP GPIO hardware version 1.1
[    0.127624] omap_gpio omap_gpio.2: Could not get gpio dbck
[    0.128204] omap_gpio omap_gpio.3: Could not get gpio dbck
[    0.128753] omap_gpio omap_gpio.4: Could not get gpio dbck

Here also GPIO IRQs (touchscreen, Retu) still work.

A.

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Javier Martinez Canillas]

Hello Aaro,

Thanks a lot for testing the series!

On Thu, Apr 10, 2014 at 9:30 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 07:29:26PM +0200, Linus Walleij wrote:
>> On Sun, Apr 6, 2014 at 4:58 PM, Javier Martinez Canillas
>> <javier.martinez@collabora.co.uk> wrote:
>>
>> > Now that you have sent your changes for v3.15 to Torvalds, here are some
>> > changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements
>> > so nothing here is -rc material.
>>
>> I like this series so I have applied them for v3.16, pending some ACK
>> from Kevin &| Santosh.
>
> I tried these patches on OMAP1 on top of today's Torvalds git
> (4ba85265790ba3681deeaf73f018c0eb829a7341).
>
> On Amstrad E3 I'm getting the following logs:
>
> [    0.156491] omap_gpio omap_gpio.0: Runtime PM disabled, clock forced on.
> [    0.164604] genirq: Setting trigger mode 0 for irq 64 failed (gpio_irq_type+0x0/0x1f0)
> [    0.165418] genirq: Setting trigger mode 0 for irq 65 failed (gpio_irq_type+0x0/0x1f0)
> [    0.166133] genirq: Setting trigger mode 0 for irq 66 failed (gpio_irq_type+0x0/0x1f0)
> [    0.166838] genirq: Setting trigger mode 0 for irq 67 failed (gpio_irq_type+0x0/0x1f0)
> [...]
> [    0.182856] genirq: Setting trigger mode 0 for irq 79 failed (gpio_irq_type+0x0/0x1f0)
> [    0.186887] omap_gpio omap_gpio.1: Could not get gpio dbck
> [    0.189308] genirq: Setting trigger mode 0 for irq 95 failed (gpio_irq_type+0x0/0x1f0)
> [...]
> [    0.203121] genirq: Setting trigger mode 0 for irq 110 failed (gpio_irq_type+0x0/0x1f0)
>
> However it still seems to work. The serio is only GPIO IRQ and it
> triggers when I press the external keyboard.
>
> The same happens also on Nokia 770:
>
> [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
> [    0.119201] genirq: Setting trigger mode 0 for irq 129 failed (gpio_irq_type+0x0/0x220)
> [...]
> [    0.124999] genirq: Setting trigger mode 0 for irq 143 failed (gpio_irq_type
> +0x0/0x220)
> [    0.126831] omap_gpio omap_gpio.1: Could not get gpio dbck
> [    0.127258] OMAP GPIO hardware version 1.1
> [    0.127624] omap_gpio omap_gpio.2: Could not get gpio dbck
> [    0.128204] omap_gpio omap_gpio.3: Could not get gpio dbck
> [    0.128753] omap_gpio omap_gpio.4: Could not get gpio dbck
>
> Here also GPIO IRQs (touchscreen, Retu) still work.
>
> A.

I don't have those errors when booting on my DM3730 IGEPv2 board but
it seems that for some reason on omap1  __irq_set_trigger() complains
when IRQ_TYPE_NONE is used as a default flag when calling
gpiochip_irqchip_add()

Could you please test the following patch and tell me if your board
still works and makes the errors go away?

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8cc9e91..5bc8aec 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)

        ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
                                   irq_base, gpio_irq_handler,
-                                  IRQ_TYPE_NONE);
+                                  IRQ_TYPE_LEVEL_LOW);

        if (ret) {
                dev_err(bank->dev, "Couldn't add irqchip to gpiochip
%d\n", ret);

Best regards,
Javier

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Aaro Koskinen]

Hi,

On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
> > The same happens also on Nokia 770:
> >
> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
> 
> I don't have those errors when booting on my DM3730 IGEPv2 board but
> it seems that for some reason on omap1  __irq_set_trigger() complains
> when IRQ_TYPE_NONE is used as a default flag when calling
> gpiochip_irqchip_add()
> 
> Could you please test the following patch and tell me if your board
> still works and makes the errors go away?

Now it complains about mode 8...

[    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
+0x0/0x220)

A.

> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 8cc9e91..5bc8aec 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
> 
>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>                                    irq_base, gpio_irq_handler,
> -                                  IRQ_TYPE_NONE);
> +                                  IRQ_TYPE_LEVEL_LOW);
> 
>         if (ret) {
>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
> %d\n", ret);
> 
> Best regards,
> Javier

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Javier Martinez Canillas]

Hello Aaro,

On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>> > The same happens also on Nokia 770:
>> >
>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>
>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>> it seems that for some reason on omap1  __irq_set_trigger() complains
>> when IRQ_TYPE_NONE is used as a default flag when calling
>> gpiochip_irqchip_add()
>>
>> Could you please test the following patch and tell me if your board
>> still works and makes the errors go away?
>
> Now it complains about mode 8...
>
> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
> +0x0/0x220)
>
> A.
>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 8cc9e91..5bc8aec 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>>
>>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>>                                    irq_base, gpio_irq_handler,
>> -                                  IRQ_TYPE_NONE);
>> +                                  IRQ_TYPE_LEVEL_LOW);
>>
>>         if (ret) {
>>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
>> %d\n", ret);
>>
>> Best regards,
>> Javier

Thanks for testing. Unfortunately I'm out of ideas on why that error
could be shown and I don't have a way to further debug it without an
omap1 board. I wonder why that pr_err() message is shown or why it is
still working when an error happens.

Maybe Linus or Santosh could give us a hint on what is happening here.

Best regards,
Javier

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Linus Walleij]

On Fri, Apr 11, 2014 at 5:03 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Aaro,
>
> On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>> Hi,
>>
>> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>>> > The same happens also on Nokia 770:
>>> >
>>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>>
>>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>>> it seems that for some reason on omap1  __irq_set_trigger() complains
>>> when IRQ_TYPE_NONE is used as a default flag when calling
>>> gpiochip_irqchip_add()
>>>
>>> Could you please test the following patch and tell me if your board
>>> still works and makes the errors go away?
>>
>> Now it complains about mode 8...
>>
>> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
>> +0x0/0x220)
>>
>> A.
>>
>>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>>> index 8cc9e91..5bc8aec 100644
>>> --- a/drivers/gpio/gpio-omap.c
>>> +++ b/drivers/gpio/gpio-omap.c
>>> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>>>
>>>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>>>                                    irq_base, gpio_irq_handler,
>>> -                                  IRQ_TYPE_NONE);
>>> +                                  IRQ_TYPE_LEVEL_LOW);
>>>
>>>         if (ret) {
>>>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
>>> %d\n", ret);
>>>
>>> Best regards,
>>> Javier
>
> Thanks for testing. Unfortunately I'm out of ideas on why that error
> could be shown and I don't have a way to further debug it without an
> omap1 board. I wonder why that pr_err() message is shown or why it is
> still working when an error happens.
>
> Maybe Linus or Santosh could give us a hint on what is happening here.

Isn't an edge IRQ more apropriate as default then?

The code contains this:

    if (!bank->regs->leveldetect0 &&
        (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
        return -EINVAL;

Meaning sometimes the banks don't support level IRQs.

Yours,
Linus Walleij

[PATCH 0/5] GPIO OMAP driver changes for v3.16

[Javier Martinez Canillas]

Hello Aaro,

On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>> > The same happens also on Nokia 770:
>> >
>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>
>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>> it seems that for some reason on omap1  __irq_set_trigger() complains
>> when IRQ_TYPE_NONE is used as a default flag when calling
>> gpiochip_irqchip_add()
>>
>> Could you please test the following patch and tell me if your board
>> still works and makes the errors go away?
>
> Now it complains about mode 8...
>
> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
> +0x0/0x220)
>
> A.
>

On thread [1] was reported a regression on a Sitara AM335x board
because the irq .map function handler used on the gpiolib called
irq_set_type() for each pin on the bank.

In the case of that board, a GPIO pin is used to control the SDRAM
termination regulator and calling irq_set_type() sets the GPIO as
input thus making the board to fail on boot. But this may also explain
why you had those error logs on your OMAP1 board when applying these
patches since that changes GPIO OMAP semantics when the driver
expected irq_set_type() to only be called when a IRQ is requested.

After some discussion Linus proposed a patch [2] which I think should
also fix your issue. Can you please give it a try and provide your
Tested-by for that patch?

Thanks a lot and best regards,
Javier

[1]: http://marc.info/?t=139817273800014&r=1&w=2
[2] https://patchwork.kernel.org/patch/4041881/