Re: [PATCH 1/2] powerpc/8xx: Fix NR_IRQ bugs and refactor 8xxinterrupt controller

[Grant Likely]

On Mon, Apr 16, 2012 at 5:03 PM, Benjamin Herrenschmidt
<benh@xxxxxxxxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/