Re: [Simulavr-devel] Interrupt Behavior

[Theodore A. Roth]

On Wed, 26 Nov 2003, Keith Gudger wrote:

> On Wed, 26 Nov 2003, Theodore A. Roth wrote:
> >
> > On Wed, 26 Nov 2003, Keith Gudger wrote:
> >
> > > Ted:
> > >
> > > In debuging the UARTs, I noticed a couple of things about the interrupt
> > > behavior.
> > >
> > > 1.  After executing the reti instruction, simulavr does not execute 2
> > > instructions (the AVR does).
> >
> > The mega128 data sheet says this:
> >
> >   When the AVR exits from an interrupt, it will always return to the
> >   main program and execute one more instruction before any pending
> >   interrupt is served.
> >
> > Where is the 2nd insn coming from?
> >
> > I'm not sure if simulavr is handling that extra insn correctly though.
> > A flag would need to be set when the RETI insn is exectuted which
> > inhibits the next call to avr_core_check_interrupts(). The flag would be
> > checked and cleared by either avr_core_step() or
> > avr_core_check_interrupts().
> >
>
> OK, I'm wrong, it's only 1 instruction.  Still, the avr_core_step doesn't
> handle this properly - it immediately vectors to the next interrupt in the
> queue.

Ok. Try the attached patch and see if it fixes things for you. I haven't
tested it, but it at least compiles in my tree.

>
> > The data sheet also says this:
> >
> >   If an interrupt occurs during execution of a multi-cycle instruction,
> >   this instruction is completed before the interrupt is served.
> >
> > This case looks to be handled properly by the avr_core_step() function.
> >
> > >
> > > 2.  I discovered this as part of trying to figure out why I get 2 UDRE
> > > interrupts back to back.  I guess I need to find some way to keep the
> > > interrupt callback from issuing a 2nd interrupt until the first gets
> > > processed?  Does this sound right?  Any suggestions?  Thanks.
> >
> > Interrupts should not be queued. If there is an irq already in the
> > pending list for a given irq, a new one should not be added to the list.
> > Right now, I think it's up to the VDev to make sure that it doesn't
> > install multiple irq's into the pending list.
> >
> > It wouldn't be too difficult to modify irq_list_add() to allow only one
> > irq per priority. It looks like it will mean creating a new dlist method
> > called dlist_add_unique(). I'm not convinced that this is the best
> > solution though, since it masks a problem in the VDev.
> >
> That sounds nice - the Vdev should handle it - but I'm not sure how.  It
> could save a state that an interrupt was queued, but since the vdev calls
> avr_core_irq_raise, and that routine returns void, AND
> avr_core_check_interrupts() doesn't know which callback set the interrupt,
> how could the Vdev ever clear its flag?  I think it has to be in
> irq_list_add.   Let me know your thoughts.

You'd would have to track the state. The callback would need to clear
the state before it returns with the REMOVE code.

Have a look at wdtcr_write() and wdtcr_timer_cb() in register.c. That is
one way of knowing if the callback is already in the pending list.

Ted Roth

sim-irq-ret.diff
Description: Text document