On Wed, 06 Dec 2006 16:17:29 +0300, Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx> wrote:
> > +static struct irq_chip i8259A_chip = {
> > + .name = "XT-PIC",
> > + .mask = disable_8259A_irq,
> > + .unmask = enable_8259A_irq,
> > + .mask_ack = mask_and_ack_8259A,
> > };
>
> I wonder whose idea was to call this device XT-PIC. XT never had dual
> 8259A PICs and so was capable of handling only 8 IRQs. Dual 8259A was first
> used in the AT class machines...
It has been called "XT-PIC" anyway so I'd like to keep unchanged.
> > {
> > - unsigned int mask = 1 << irq;
> > + unsigned int mask = 1<<irq;
>
> Unnecassary, to say the least.
>
> > @@ -109,7 +99,8 @@ int i8259A_irq_pending(unsigned int irq)
> > void make_8259A_irq(unsigned int irq)
> > {
> > disable_irq_nosync(irq);
> > - set_irq_chip(irq, &i8259A_irq_type);
> > + set_irq_chip_and_handler_name(irq, &i8259A_chip, handle_level_irq,
> > + "XT");
>
> No! Do not evoke the memory of XT anymore, let it rest in peace at last!
> Call it 8259A, please.
>
> > @@ -122,17 +113,17 @@ void make_8259A_irq(unsigned int irq)
> > static inline int i8259A_irq_real(unsigned int irq)
> > {
> > int value;
> > - int irqmask = 1 << irq;
> > + int irqmask = 1<<irq;
>
> Unnecessary too.
Well, these changes are due to synchronization with i386's code. I'll
drop them.
> > @@ -214,15 +207,52 @@ spurious_8259A_irq:
> > }
> > }
> >
> > +static char irq_trigger[2];
> > +/**
> > + * ELCR registers (0x4d0, 0x4d1) control edge/level of IRQ
> > + */
> > +static void restore_ELCR(char *trigger)
> > +{
> > + outb(trigger[0], 0x4d0);
> > + outb(trigger[1], 0x4d1);
> > +}
> > +
> > +static void save_ELCR(char *trigger)
> > +{
> > + /* IRQ 0,1,2,8,13 are marked as reserved */
> > + trigger[0] = inb(0x4d0) & 0xF8;
> > + trigger[1] = inb(0x4d1) & 0xDE;
>
> Erm, the bits should be zero, why mask them out I wonder...
These codes are also come from i386 ... while they seems not used on
MIPS now anyway I'll drop them.
Thank you for review. I'll post updated patch soon.
---
Atsushi Nemoto
