On Mon, Apr 16, 2012 at 5:03 PM, Benjamin Herrenschmidt <b...@kernel.crashing.org> wrote: > On Mon, 2012-04-16 at 14:13 -0600, Grant Likely wrote: >> The mpc8xx driver uses a reference to NR_IRQS that is buggy. It uses >> NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but >> NR_IRQs could be smaller than the number of hardware irqs that >> ppc_cached_irq_mask tracks. >> >> Also, while fixing that problem, it became apparent that the interrupt >> controller only supports 32 interrupt numbers, but it is written as if >> it supports multiple register banks which is more complicated. >> >> This patch pulls out the buggy reference to NR_IRQs and fixes the size >> of the ppc_cached_irq_mask to match the number of HW irqs. It also >> drops the now-unnecessary code since ppc_cached_irq_mask is no longer >> an array. > > Can you rename ppc_cached_irq_mask while at it ? I think it was written > that way because it was all copy/pasted from powermac/pic.c which -does- > need it to be an array (and has the same bugs btw) :-) > > I wonder if we can get rid of the cached mask alltogether... I have the > feeling that we could just rely on the irq_desc fields nowadays for > that... this code is ancient.
The irq_desc fields aren't ideal because they are per-irq and would need to be &'ed together for each mask/unmask/ack operation. We could store something in the irq_domain I suppose, but I'm not convinced it is worth it for an interrupt controller that will only ever have one instance in the system. If I were to refactor any deeper than I have, then I'm move this driver over to use irq_generic_chip instead. g. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev