On Mon, Jun 24, 2013 at 9:21 AM, Tony Lindgren <[email protected]> wrote:
> * Javier Martinez Canillas <[email protected]> [130623 18:08]:
>> On Mon, Jun 24, 2013 at 1:43 AM, Aaro Koskinen <[email protected]> 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 <[email protected]> 
>> >> 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 <[email protected]>
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/[email protected]/msg89005.html

Signed-off-by: Javier Martinez Canillas <[email protected]>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to