Messages in this thread | | | Date | Mon, 22 Apr 2013 12:02:19 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v3] irqchip: Add support for ARMv7-M's NVIC |
| |
Hello,
(for the new readers of this thread: This is about using
u32 mask = 1 << (d->hwirq % 32);
instead of
u32 mask = 1 << (d->irq - gc->irq_base);
in the callbacks for the irq generic chip.)
On Fri, Apr 19, 2013 at 05:09:27PM +0200, Uwe Kleine-König wrote: > On Thu, Apr 18, 2013 at 11:35:22AM +0200, Thomas Gleixner wrote: > > On Wed, 17 Apr 2013, Uwe Kleine-König wrote: > > How is that different from what the generic irq chip implementation > > does? The only difference is that mask is generated by d->hwirq and > > not by d->irq. And due to the fact, that you use a full linear mapping > > between hwirq and virq the generic code simply works. > I'm not sure what you mean when you say "full linear mapping". AFAICT > using irq_domain_add_linear doesn't imply that two consecutive hardware > irq numbers get consecutive Linux irq numbers, so using d->irq won't work. > > > Even if it would not work, it would be trivial to extend the generic > > chip with that functionality instead of hacking another slightly > > different copy of the same thing. > I will try that and report back. I wonder if using hwirq % 32 should work everywhere where now d->irq - gc->irqbase is used. Depending on d->hwirq and not d->irq has the upside of working with non-legacy irq domains, too.
Looking at next-20130419 the affected functions (irq_gc_mask_disable_reg, irq_gc_mask_set_bit, irq_gc_mask_clr_bit, irq_gc_unmask_enable_reg, irq_gc_ack_set_bit, irq_gc_ack_clr_bit, irq_gc_mask_disable_reg_and_ack, irq_gc_eoi, irq_gc_set_wake) are used in:
arch/arm/mach-davinci/irq.c arch/arm/mach-imx/avic.c arch/arm/mach-imx/tzic.c arch/arm/mach-omap2/irq.c arch/arm/mach-omap2/prm_common.c
arch/arm/mach-s5p64x0/common.c -> uses irq_base=101 for irq_alloc_generic_chip arch/arm/plat-orion/gpio.c -> depends on how orion_gpio_of_init is called. No callers found.
arch/arm/plat-orion/irq.c arch/arm/plat-samsung/irq-vic-timer.c -> used for a single irq that isn't a multiple of 32
arch/arm/plat-samsung/s5p-irq-gpioint.c -> would need % 8?
arch/mips/jz4740/gpio.c -> JZ4740_IRQ_GPIO(0) != JZ4740_IRQ_GPIO0 ? -> uses 56 + i * 32 as irqbase
arch/mips/jz4740/irq.c -> uses 8 as irqbase
arch/sh/boards/mach-se/7343/irq.c -> uses irq_base = irq_linear_revmap(se7343_irq_domain, 0) where se7343_irq_domain is a linear domain. AFAICT this is a bug. (After adding the domain they map all irqs in increasing order which currently seems to guarantee that it works. But IMHO it should use a legacy domain.)
arch/sh/boards/mach-se/7722/irq.c as above.
drivers/gpio/gpio-mxc.c drivers/gpio/gpio-mxs.c drivers/gpio/gpio-omap.c drivers/gpio/gpio-sodaville.c drivers/irqchip/irq-sirfsoc.c drivers/mfd/jz4740-adc.c -> uses platform_get_irq(pdev, 1) as irq_base for 5 irqs.
For the uncommented files using %32 instead of -gc->irq_base should work.
So it seems I cannot just substitute how the mask is called.
The options I see are:
- introduce a new set of functions Do you have a nice naming scheme? irq_gc_unmask_enable_reg_hwirqmod32? Or should I rename the existing ones to irq_gc_unmask_enable_reg_irqbaseoffset? - use u32 mask = 1 << (d->hwirq - gc->irq_base) % 32; This is ugly but might work assuming irq_base == 0 for chips with irq domain support and hwirq == irq for the others.
I'm not lucky with the options, so I'm looking forward to suggestions.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |