This was exactly what I needed to get stuff working...
irq_request has some very strange behavior if
it is nested within a save_flags /restore_flags
construction.
Consider the snippet:
.....
save_flags(flags); cli();
request_irq(SERIAL, dz_interrupt, SA_INTERRUPT, "DZ", lines[0]);
restore_flags(flags);
......
It turns out that this manages to leave the state of the status register
unchanged.
Move the restore_flags before the call to request_irq and you make
progress - and indeed the dz.c driver code actually generates characters
across the serial line.
-Tom
-----------------------------------------------------------------------
Given enough eyeballs all bugs seem shallow.
On Fri, 16 Oct 1998, Harald Koerfgen wrote:
> Hi,
>
> On 16-Oct-98 Thomas Riemer wrote:
> > Looking at int-handler.S the code testing the CAUSE register($12) against
> > the STATUS register($13). It only processes if both bits in the
> > two registers are set. Somehow the clock is being set in the STATUS
> > register and I am getting hardware interrupts - which is a good sign.
> >
> > After having looked for about an hour for where the STATUS register is
> > set, I seem to be unable to find where that happens. In the case
> > of the dz driver, I believe that the kernel is generating interrupts, but
> > because the STATUS register is not set, the kernel is ignoring these
> > interrupts.
> >
> > I would think the STATUS register should be set in request_irq - but
> > it isn't.
> >
> > Where is the code that sets the STATUS bit for the clock? and in general
> > for irqs?
>
> This is indeed somewhat tricky. I had to find a way to manipulate the status
> register within a
>
> save_and_cli(flags);
> ...
> restore_flags(flags);
>
> code sequence. save_flags saves the content of the status register in flags
> and
> restore_flags restores it, so enabling or disabling interrupts within a
> save/restore
> pair wouldn't work.
>
> arch/mips/dec/irq.c:
>
> request_irq() -> setup_dec_irq()
>
> setup_dec_irq:
>
> ...
> save_and_cli(flags);
> *p = new;
>
> if (!shared) {
> unmask_irq_flags(irq, &flags);
> }
> restore_flags(flags);
> ....
>
> and unmask_irq_flags looks basicly like (leaving the IOASIC stuff aside):
>
> static inline void unmask_irq_flags(unsigned int irq_nr, unsigned long *flags)
> {
> ...
> *flags |= dec_interrupt[irq_nr].cpu_mask;
> }
>
> That means, the bits in the status register are set (or reset) by setting (or
> resetting) bits in flags and restore_flags(flags) does the trick.
>
> I _know_ this is dirty and if you have a better idea without adding a
> performance
> penalty, I'd love to implement it.
>
> People, when writing device drivers for DECstations, DO NEVER EVER use code
> like:
>
> save_flags(flags);
> [disable or enable interrupts with en/disable_irq(irq)]
> restore(flags);
>
> That simply doesn't work and will cause endless headache.
> ---
> Regards,
> Harald
>
|