On Fri, 13 Dec 2002, Jun Sun wrote:
> This patch adds support for those sorry boards which only have
> option 2) available.
>
> While I am here, I also promoted do_IRQ() declaration to a
> header file, as it is needed by all C-level interrupt dispatching
> code.
>
> Any feedbacks?
Yep -- a few random thoughts.
> +asmlinkage void dispatch_i8259_irq(struct pt_regs *regs)
> +{
> + int isr, irq=-1;
> +
> + isr = ~(int)inb(0x20) | cached_21;
OCW3 defaults to IRR in our setup (as it does for the chip itself after
writing ICWs) -- you need to select ISR explicitly before reading and
reset it afterwards to avoid surprises. Unless we change the default for
MIPS, which seems feasible -- we don't have to handle i386 oddities like
I/O APICs and weird chipset bugs. And we avoid the need to grab a
spinlock here. Alpha went this path.
> + if (isr != -1)
> + irq = ffz (isr);
> + if (irq == 2) {
> + isr = ~(int)inb(0xa0) | cached_A1;
> + if (isr != -1)
> + irq = 8 + ffz(isr);
> + }
> + if (irq == -1) {
> + printk("We got a spurious i8259 interrupt\n");
The wording is odd -- how about following the one from
arch/i386/kernel/i8259.c?
> + atomic_inc(&irq_err_count);
> + } else {
> + do_IRQ(irq,regs);
And how about using an offset passed from a user? We're not on a PC --
i8259 IRQs do not have to start from 0. E.g. I find cleaner allocating
CPU IRQs first if handled.
> --- ./include/asm-mips64/irq.h.orig Fri Dec 13 10:51:25 2002
> +++ ./include/asm-mips64/irq.h Fri Dec 13 14:50:46 2002
[...]
> +extern asmlinkage unsigned int do_IRQ(int irq, struct pt_regs *regs);
Hmm, <asm/hw_irq.h> might be a better alternative. I have no strong
preference, though. Both get included by <linux/irq.h> so there's no
difference for a user.
Maciej
--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: macro@ds2.pg.gda.pl, PGP key available +
|