[BISECTED] 3.10-rc1 OMAP1 GPIO IRQ regression

[Javier Martinez Canillas]

On Mon, Jun 24, 2013 at 9:21 AM, Tony Lindgren <tony at atomide.com> wrote:
> * Javier Martinez Canillas <martinez.javier at gmail.com> [130623 18:08]:
>> On Mon, Jun 24, 2013 at 1:43 AM, Aaro Koskinen <aaro.koskinen at iki.fi> wrote:
>> > Hi,
>> >
>> > On Mon, Jun 24, 2013 at 01:06:37AM +0200, Javier Martinez Canillas wrote:
>> >> On Mon, Jun 24, 2013 at 12:16 AM, Aaro Koskinen <aaro.koskinen at iki.fi> wrote:
>> >> > What is the status of this patch? We're already at 3.10-rc7 and GPIO
>> >> > IRQs are still broken on OMAP1.
>> >
>> > [...]
>> >
>> >> There is a problem with this patch.
>> >
>> > [...]
>> >
>> >> So I think that the correct solution is to add SPARSE_IRQ support to
>> >> omap1 and not reverting Jon's patch. Of course this may not be
>> >> possible since we are so close to 3.10 and most OMAP patches already
>> >> merged for 3.11 but we should definitely try to have this at least for
>> >> 3.12. Otherwise we won't be able to move to DT-only booting for
>> >> OMAP2+.
>> >
>> > OMAP1 does not use DT. So we could put this code under #ifdef
>> > CONFIG_ARCH_OMAP1 or similar. It's just a few lines of code. OMAP2+
>> > work should not regress OMAP1.
>> >
>> > Demanding SPARSE_IRQ support for OMAP1 should have been discussed before
>> > these changes were made. It's not reasonable to assume such things can
>> > be made during rc-cycle. Also, now, I don't think it's reasonable to
>> > wait for that to be done, as it would take until 3.12 or even later to
>> > get OMAP1 functional again.
>> >
>> > A.
>>
>> Hi,
>>
>> Yes, since we are so late in the -rc cycle and OMAP1 is currently
>> broken I agree that the only sensible solution is to revert the patch
>> for now.
>
> Agreed.
>
>> I just wanted to point out the issue that keeping the OMAP GPIO driver
>> using legacy mapping domain represents a blocker to have GPIO-IRQ
>> working with Device Tree for OMAP2+
>
> Yes. We can do the ifdef Aaro suggested, and let's also plan on
> converting omap1 to use SPARSE_IRQ. But with the ifdef we can cut
> away the dependency between these two.
>
> Regards,
>
> Tony

Ok, so something like the following patch should do it (tested on an
OMAP3 board):

>From b9e262c688fb7f3ad733f140b55dddbc8e4716e6 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javier.martinez at collabora.co.uk>
Date: Mon, 24 Jun 2013 17:13:23 +0200
Subject: [PATCH 1/1] gpio/omap: don't use linear domain mapping for OMAP1

commit ede4d7a5 ("gpio/omap: convert gpio irq domain to linear mapping")
converted the OMAP GPIO driver to use a linear mapping for the GPIO IRQ
domain instead of using a legacy mapping. Not using a legacy mapping has
a number of benefits but it requires the platform to support SPARSE_IRQ
which currently is not supported on OMAP1.

So this change caused a regression on OMAP1 platforms [1].

Since this issue is not present on all OMAP2+ platforms, there is no need to
revert the driver to use legacy domain mapping for all the platforms.

[1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg89005.html

Signed-off-by: Javier Martinez Canillas <javier.martinez at collabora.co.uk>
---
 drivers/gpio/gpio-omap.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index d3f7d2d..4a43036 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1094,6 +1094,9 @@ 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

 	match = of_match_device(of_match_ptr(omap_gpio_match), dev);

@@ -1135,11 +1138,28 @@ 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() and irq_domain_add_legacy() and just use a
+	 * linear IRQ domain mapping for all OMAP platforms.
+	 */
+	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);
-	if (!bank->domain)
+#endif
+	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;
-- 
1.7.7.6