Re: [RFC PATCH] microblaze/irq: Change NO_IRQ to 0

From: Rob Herring
Date: Wed Dec 07 2011 - 11:39:56 EST


Linus,

On 12/07/2011 10:05 AM, Linus Torvalds wrote:
> On Wed, Dec 7, 2011 at 6:41 AM, Rob Herring <robherring2@xxxxxxxxx> wrote:
>>
>> This patch changes the Microblaze NO_IRQ setting from -1 to 0 to bring
>> it in line with most of the rest of the kernel. It also prepares for
>> Microblaze eventually supporting multiple interrupt controllers by
>> breaking the assumption that hwirq# == Linux IRQ#. The Linux IRQ
>> number is just a cookie with no guarantee of a direct relationship
>> with the hardware irq arrangement.
>
> This looks really nice and would probably be a great example for other
> architectures with the same issue.
>
> My only (small) nit is that just from a debuggability standpoint I'd
> actually suggest keeping the "translated" and "hardware" interrupt
> numbers totally disjoint, which would seem to be trivial - make the
> offset be 32 instead of 1. Sure, some NR_IRQ arrays would end up being
> a bit bigger that way, but it would allow for trivially seeing whether
> anybody reports the raw or translated irq numbers by just looking at
> the number. That could reduce lots of confusion if somebody ends up
> having the old non-translated number hardcoded etc.
>
> It would also potentially allow for actual debug checks - having code
> like "WARN_ON_ONCE(irq < 32)" in some arch-specific irq controller
> registration path etc, which is much harder to do with the overlapping
> numbers.

Agreed. I debated making it 16 just to skip the legacy interrupts as
we're doing on ARM. So there's 2 benefits to skipping to 32.

Anyway, I'll leave it to someone that can actually build and test this.

> But it's nice to see how small the patch *can* be.

If only ARM was this easy...

Rob
--
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/