On Tue, Dec 11, 2012 at 4:20 PM, Thomas Petazzoni <thomas.petazz...@free-electrons.com> wrote:
> Unfortunately, this creates the following warning at boot time for each > GPIO bank: Grant has a patch in his irqdomain tree that will turn this warning into a simple pr_info() thing instead. It's not that bad... The immediate problem will soon go away. > Of course, the fix should be to remove the irq_alloc_descs() from the > driver prior to calling irq_domain_add_simple(). But the thing is that > our gpio-mvebu driver uses the > irq_alloc_generic_chip()/irq_setup_generic_chip() infrastructure, which > it seems requires a legacy IRQ domain (it needs the base IRQ number). Actually it looks like a core infrastructure issue. Sorry for not spotting this in the first place: First you allocate some descriptors, just any descriptors, with mvchip->irqbase = irq_alloc_descs(-1, 0, ngpios, -1); Then you allocate a generic chip using the obtained descriptor base: gc = irq_alloc_generic_chip("mvebu_gpio_irq", 2, mvchip->irqbase, mvchip->membase, handle_level_irq); Then you set up the generic chip with a nailed down IRQ base number from step 1: irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0, IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE); This, if I understand it correctly, is done because you have two different chip types on the generic chip: one for high/low level IRQ and another for rising/falling. (Which is a very nice way to use the generic chip!) Finally set up the IRQ domain: mvchip->domain = irq_domain_add_simple(np, mvchip->chip.ngpio, mvchip->irqbase, &irq_domain_simple_ops, mvchip); So the problem is that you cannot allocate a generic chip without having a base IRQ at that point, if I understand correctly. If this was not necessary you would not need to allocate descriptors in advance. Or rather: the *real* problem, which will face anyone trying to implement a combined edge+level IRQ chip in a driver, is that the generic irqchip does not play well with irqdomain. Using legacy in this case is clearly wrong, the generic IRQ chip is not one least bit legacy, it's top-of-the-line IRQ handling, using generic code as we want. However it seems generic chips cannot handle sparse IRQs at all, it requires the descriptors to be allocated before we create and instance of it. So I see two solutions: - Fix the generic chip to handle sparse IRQs by patching a lot in kernel/irq/generic-chip.c and allowing to pass something like < 0 for irq base. - Add something like irq_domain_add_generic() for generic chips and handle the oddities there. The latter would be a pretty straight-forward wrapper to legacy domain as of now. Any preference? Or should we just consider generic irqchips a legacy case? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/