Messages in this thread | | | Date | Wed, 07 Dec 2011 10:39:46 -0600 | From | Rob Herring <> | Subject | Re: [RFC PATCH] microblaze/irq: Change NO_IRQ to 0 |
| |
Linus,
On 12/07/2011 10:05 AM, Linus Torvalds wrote: > On Wed, Dec 7, 2011 at 6:41 AM, Rob Herring <robherring2@gmail.com> 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
| |