| Message ID | alpine.LFD.2.02.1305031514230.2886@ionos (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
The ongoing device tree support for ARM is creating new irq chip drivers in drivers/irqchip/ in a frenzy. Quite some of them are ripping out the generic irq chip implementation from arch/arm/* and just creating the same mess of duplicated code again, which was cleaned up with the generic irq chip implementation with a lot of effort. Sigh! I already prodded a few people in reviews to tackle that issue with no outcome. Even more sigh! Poor Sebastian triggered me into rant mode, but he ad hoc volunteered to give it a try. YAY! Though he asked for a bit of kickstart help. So I squeezed out a few spare cycles and implemented the basics as far as I think that they should work. The following series contains the missing bits and pieces including a somehow forgotten and now slightly modified series from Gerlando adding support for irq chips which need separate mask caches for different chip (control flow) types. At the moment this supports only linear irq domains, but it could be extended to other types as well if the need arises. Though the ARM chips are pretty much all about linear domains AFAICT. It also lacks support for removing an irq domain at the moment, but that should be rather trivial to fix. The last patch in the series is a blind conversion of the irq-sun4i irq chip driver, completely untested and not even compiled. I just added it for demonstration purposes. As Russell expected, there is a lot of consolidation potential. The changelog of that patch is: 1 file changed, 29 insertions(+), 71 deletions(-) The preparing series has 4 files changed, 294 insertions(+), 50 deletions(-) So for removing 42 lines in a single driver the core grows 244 lines including header changes and comments. Convert 6 drivers and we are more than even because we get the benefit of sharing and therefor exposing the same code to broader testing and utilization. We have already 11 of those candidates in drivers/irqchips and new ones are knocking on the door. There might be even more consolidation potential, but I leave that to the DT/irq domain experts. WARNING: It's compile tested only. So if you find bugs you can keep them and fix them yourself :) Thanks, tglx --- drivers/irqchip/irq-sun4i.c | 100 ++++----------- include/linux/irq.h | 45 ++++++- include/linux/irqdomain.h | 12 + kernel/irq/generic-chip.c | 281 +++++++++++++++++++++++++++++++++++++------- kernel/irq/irqdomain.c | 6 5 files changed, 323 insertions(+), 121 deletions(-)
Hello, On Fri, May 03, 2013 at 09:50:43PM -0000, Thomas Gleixner wrote: > The ongoing device tree support for ARM is creating new irq chip > drivers in drivers/irqchip/ in a frenzy. Quite some of them are > ripping out the generic irq chip implementation from arch/arm/* and > just creating the same mess of duplicated code again, which was > cleaned up with the generic irq chip implementation with a lot of > effort. Sigh! > > I already prodded a few people in reviews to tackle that issue with no > outcome. Even more sigh! > > Poor Sebastian triggered me into rant mode, but he ad hoc > volunteered to give it a try. YAY! > > Though he asked for a bit of kickstart help. So I squeezed out a few > spare cycles and implemented the basics as far as I think that they > should work. > > The following series contains the missing bits and pieces including a > somehow forgotten and now slightly modified series from Gerlando > adding support for irq chips which need separate mask caches for > different chip (control flow) types. > > At the moment this supports only linear irq domains, but it could be > extended to other types as well if the need arises. Though the ARM > chips are pretty much all about linear domains AFAICT. Is there a tree/set of patches that have already fixed the issues pointed out by Russell and Sebastian? I'd like to use it to get forward with my nvic patch and want to avert double work and merging different approaches. Best regards Uwe
Changes vs. V1:
- Fixed the generic chip pointer thinko (Sebastian Hesselbarth)
- Proper support for mask cache
- Read mask hardware only for the first map of an generic chip
instance
- sun4i prefix irq functions proper
Thanks,
tglx
Hi Thomas, Sebastian, I see these changes made it to 3.11. AFAICT though, 3.10.9 still has the original bug (the one that got me to write the patch for handling separate mask registers) and I am bit confused as to how to integrate that back into 3.10 (or any previous affected kernels, as they deserve a fix as well!). The way I understand it, any mainstream patch would be based on this work, which is not available on previous kernels. And I guess backporting the whole thing would be overkill. So I believe the only way to fix it on older kernels would be to write one (or more) minimal version-specific patch series. But then I wonder: would that be acceptable material for linux-stable? Please correct me if I'm totally wrong here. I'm willing to help & test but I need directions. Thanks, Gerlando On 05/06/2013 04:30 PM, Thomas Gleixner wrote: > Changes vs. V1: > > - Fixed the generic chip pointer thinko (Sebastian Hesselbarth) > > - Proper support for mask cache > > - Read mask hardware only for the first map of an generic chip > instance > > - sun4i prefix irq functions proper > > Thanks, > > tglx > >
Index: linux-2.6/include/linux/irq.h =================================================================== --- linux-2.6.orig/include/linux/irq.h +++ linux-2.6/include/linux/irq.h @@ -119,6 +119,7 @@ struct irq_domain; /** * struct irq_data - per irq and irq chip data passed down to chip functions + * @mask: precomputed bitmask for accessing the chip registers * @irq: interrupt number * @hwirq: hardware interrupt number, local to the interrupt domain * @node: node index useful for balancing @@ -138,6 +139,7 @@ struct irq_domain; * irq_data. */ struct irq_data { + u32 mask; unsigned int irq; unsigned long hwirq; unsigned int node; @@ -700,10 +702,14 @@ struct irq_chip_generic { * @IRQ_GC_INIT_NESTED_LOCK: Set the lock class of the irqs to nested for * irq chips which need to call irq_set_wake() on * the parent irq. Usually GPIO implementations + * @IRQ_GC_NO_MASK: Do not calculate irq_data->mask + * @IRQ_GC_MASK_FROM_HWIRQ: Calculate irq_data->mask from the hwirq number */ enum irq_gc_flags { IRQ_GC_INIT_MASK_CACHE = 1 << 0, IRQ_GC_INIT_NESTED_LOCK = 1 << 1, + IRQ_GC_NO_MASK = 1 << 2, + IRQ_GC_MASK_FROM_HWIRQ = 1 << 4, }; /* Generic chip callback functions */ Index: linux-2.6/kernel/irq/generic-chip.c =================================================================== --- linux-2.6.orig/kernel/irq/generic-chip.c +++ linux-2.6/kernel/irq/generic-chip.c @@ -39,7 +39,7 @@ void irq_gc_noop(struct irq_data *d) void irq_gc_mask_disable_reg(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable); @@ -57,7 +57,7 @@ void irq_gc_mask_disable_reg(struct irq_ void irq_gc_mask_set_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); gc->mask_cache |= mask; @@ -75,7 +75,7 @@ void irq_gc_mask_set_bit(struct irq_data void irq_gc_mask_clr_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); gc->mask_cache &= ~mask; @@ -93,7 +93,7 @@ void irq_gc_mask_clr_bit(struct irq_data void irq_gc_unmask_enable_reg(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable); @@ -108,7 +108,7 @@ void irq_gc_unmask_enable_reg(struct irq void irq_gc_ack_set_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack); @@ -122,7 +122,7 @@ void irq_gc_ack_set_bit(struct irq_data void irq_gc_ack_clr_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = ~(1 << (d->irq - gc->irq_base)); + u32 mask = ~d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack); @@ -136,7 +136,7 @@ void irq_gc_ack_clr_bit(struct irq_data void irq_gc_mask_disable_reg_and_ack(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask); @@ -151,7 +151,7 @@ void irq_gc_mask_disable_reg_and_ack(str void irq_gc_eoi(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi); @@ -169,7 +169,7 @@ void irq_gc_eoi(struct irq_data *d) int irq_gc_set_wake(struct irq_data *d, unsigned int on) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; if (!(mask & gc->wake_enabled)) return -EINVAL; @@ -254,6 +254,15 @@ void irq_setup_generic_chip(struct irq_c if (flags & IRQ_GC_INIT_NESTED_LOCK) irq_set_lockdep_class(i, &irq_nested_lock_class); + if (!(flags & IRQ_GC_NO_MASK)) { + struct irq_data *d = irq_get_irq_data(i); + u32 mask; + + if (flags & IRQ_GC_MASK_FROM_HWIRQ) + d->mask = 1 << (d->hwirq % 32); + else + d->mask = 1 << (i - gc->irq_base); + } irq_set_chip_and_handler(i, &ct->chip, ct->handler); irq_set_chip_data(i, gc); irq_modify_status(i, clr, set);