[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MiNT] [PATCH] EHCI driver interrupt checking.



It's good that more people are looking at these patches even if
it was right all along. ;)

Peter



On Wed, 10 Dec 2014 11:38:31 -0800, Hrafnkell Hlodversson <keli@keli.dk> wrote:
>
> Hi me again. (Nice conversation I?m having with myself here.)
>
> I realized that I misread the original comment, since d0 is the return
> value, it should only be restored when our handler does not use it. I
> see that the _ehci_interrupt_handle will preserve the value of d0 when
> needed by returning the original value, and the original code was
> correct.
>
> Just forget the whole thing happened ;)
>
> // Keli
>
>
> > On 10 Dec 2014, at 11:23, Hrafnkell Hlodversson <keli@keli.dk> wrote:
> >
> > Whoops! I just learned that the Coldfire can?t update the
> stack pointer in the movem instruction, so it would be (although *still*
> not tested):
> >> _ehci_int_handle_asm:
> >>     subq.l #12, sp
> >>     movem.l d0-d1/a1,(sp) // save d0, d1 and a1 to stack
> >>     move.l a0,-(sp)        // PCI_BIOS specification: parameter is
> in a0
> >>     jsr _ehci_interrupt_handle
> >>     addq.l #4,sp // Clean up argument from stack as per C calling
> conventions
> >>     movem.l (sp),d0-d1/a1 // restore old values of d0, d1 and a1
> >>     addq.l #12, sp
> >>     rts
> >
> >> On 10 Dec 2014, at 11:16, Hrafnkell Hlodversson <keli@keli.dk> wrote:
> >>
> >> Hello everyone, I?ve been lurking this list (and watching the
> CVS commits for a while)
> >>
> >> I did a quick read through this diff, and it seems there is a problem
> with how you?re saving d0.
> >>> + move.l d0,-(sp)
> >> The above does save it to the stack, but after returning from
> ehci_interrupt_handle:
> >>> -    addq.l #4,sp
> >>> +    addq.l #8,sp
> >> you simply add 4 more bytes to the stack pointer instead of restoring
> it.
> >> Also there is a movem.l instruction already there, maybe you should
> just add the d0 to the lists of registers to be pushed to that one?
> >>
> >> Something like this (First 68k assembly I?ve written in this
> century and completely untested, so don?t shoot me ;P )
> >>>
> >>> _ehci_int_handle_asm:
> >>>     movem.l d0-d1/a1,-(sp) // save d0, d1 and a1 to stack
> >>>     move.l a0,-(sp)        // PCI_BIOS specification: parameter is
> in a0
> >>>     jsr _ehci_interrupt_handle
> >>>     addq.l #4,sp // Clean up argument from stack as per C calling
> conventions
> >>>     movem.l (sp)+,d0-d1/a1 // restore old values of d0, d1 and a1
> >>>     rts
> >>
> >>
> >> With kind regards
> >> Keli
> >>
> >>> On 02 Dec 2014, at 10:11, David Gálvez <dgalvez75@gmail.com>
> wrote:
> >>>
> >>> PCI interrupt handlers are put into chain.
> >>> Check if the interrupt was triggered by the device we're driving.
> >>> Return d0 register unmodified if the interrupt is not ours.
> >>> Contributed by David Galvez.
> >>> <ehci_check_interrupt_20141202.diff>
> >>
> >
>
>
>