| Message ID | alpine.DEB.2.02.1307290943000.30864@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/07/2013, at 11:47, Paul Walmsley <paul@pwsan.com> wrote: > Hi > > On Mon, 29 Jul 2013, Javier Martinez Canillas wrote: > >> I've looked at this and it seems that irq_create_mapping() does not call the >> irq_domain_ops .map function handler since OMAP1 still uses legacy domain >> mapping. I don't have an OMAP1 platform to test but could you please see if the >> following patch [1] makes your OMAP1 platforms to boot again? >> >> But I agree with Linus and probably we should just go and revert the whole >> series since it is very hard to get it right. In another thread a user reported >> that this change also broke his DTS tree. >> >> I really tried to get this right without breaking anything but there are just >> too many OMAP platforms behaving differently and most OMAP drivers are only half >> converted to DT so this is really a can of worms. >> >> Thanks a lot and sorry for the inconvenience, >> Javier >> >> [1]: >> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c >> index c57244e..f1c6da8 100644 >> --- a/drivers/gpio/gpio-omap.c >> +++ b/drivers/gpio/gpio-omap.c >> @@ -1090,8 +1090,18 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) >> * are used as interrupts. >> */ >> if (!omap_gpio_chip_boot_dt(&bank->chip)) >> - for (j = 0; j < bank->width; j++) >> - irq_create_mapping(bank->domain, j); >> + for (j = 0; j < bank->width; j++) { >> + int irq = irq_create_mapping(bank->domain, 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_chained_handler(bank->irq, gpio_irq_handler); >> irq_set_handler_data(bank->irq, bank); > > For some reason this patch didn't apply cleanly on v3.11-rc3, so > hand-applied the following patch. It still doesn't boot on v3.11-rc3: > > http://www.pwsan.com/omap/testlogs/bisect_5912osk_boot_fail_v3.11-rc3/20130729032525/boot/5912osk/5912osk_log.txt > > > - Paul > Weird since it was against -rc3, maybe my mail client corrupted somehow. I'm out of ideas now and I don't have an OMAP1 platform to further debug this issue. I guess the safer approach is to just revert these since they are causing a regression and what the patches aims to fix as been broken since the initial OMAP migration to DT anyways. Unless the current gpio-omap maintainers are willing to keep these patches and know how to fix this OMAP1 regression Thanks a lot and sorry for this mess, Javier > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index c57244e..23da829 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1090,8 +1090,19 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) > * are used as interrupts. > */ > if (!omap_gpio_chip_boot_dt(&bank->chip)) > - for (j = 0; j < bank->width; j++) > - irq_create_mapping(bank->domain, j); > + for (j = 0; j < bank->width; j++) { > + int irq = irq_create_mapping(bank->domain, 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_chained_handler(bank->irq, gpio_irq_handler); > irq_set_handler_data(bank->irq, bank); > } -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 29, 2013 at 12:19 PM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote: > I guess the safer approach is to just revert these since they are causing > a regression and what the patches aims to fix as been broken since the > initial OMAP migration to DT anyways. I have already reverted these and pushed to linux-next. Actually input-hogs was not such a good idea either, after meditating on this I realized that the information about what GPIOs are used as interrupts is a piece of information that already exist in the device tree, albeit a bit distributed. So I'm drafting an RFC to indicate how I think this should be solved. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29/07/2013, at 13:43, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Jul 29, 2013 at 12:19 PM, Javier Martinez Canillas > <javier.martinez@collabora.co.uk> wrote: > >> I guess the safer approach is to just revert these since they are causing >> a regression and what the patches aims to fix as been broken since the >> initial OMAP migration to DT anyways. > > I have already reverted these and pushed to linux-next. > Thanks for doing that and sorry for all the inconvenience. Too bad this regression was not found before the patches hit mainline and they even were on linux-next for some time but shit happens :-( > Actually input-hogs was not such a good idea either, after > meditating on this I realized that the information about what > GPIOs are used as interrupts is a piece of information that > already exist in the device tree, albeit a bit distributed. > > So I'm drafting an RFC to indicate how I think this should > be solved. > Great, I hope we can finally have a good solution for this long standing issue. > Yours, > Linus Walleij Best regards, Javier-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 29, 2013 at 2:40 PM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote: > On 29/07/2013, at 13:43, Linus Walleij <linus.walleij@linaro.org> wrote: >> So I'm drafting an RFC to indicate how I think this should >> be solved. >> > > Great, I hope we can finally have a good solution for this long standing issue. I have sent out this RFC patch, subject: "RFC: interrupt consistency check for OF GPIO IRQs" Please review the general concept of this patch and whether it would help solving the OMAP situation for v3.12. The patch as it stands will not work, and I cannot create a proper test here because I have no test target system (I am technically on vacation), but I will get to it as I get back. And folks: you shouldn't feel bad about this. The problem is not OMAPs at all, the problem has been there all the time and we have discussed it for some time, it just happened that OMAP was the first system that tried to come up with some kind of fix for it, and for this you should be thanked, even if we need to take another path in the end. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index c57244e..23da829 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1090,8 +1090,19 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) * are used as interrupts. */ if (!omap_gpio_chip_boot_dt(&bank->chip)) - for (j = 0; j < bank->width; j++) - irq_create_mapping(bank->domain, j); + for (j = 0; j < bank->width; j++) { + int irq = irq_create_mapping(bank->domain, 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_chained_handler(bank->irq, gpio_irq_handler); irq_set_handler_data(bank->irq, bank); }