[ECOS] Spurious interrupt on ARM.

[ECOS] Spurious interrupt on ARM.

[Andrew Parlane]

Looking at hal/arm/arch/current/src/vectors.S in IRQ:

We increment the scheduler lock and decrement it again in interrupt_end.

In the case of there being a spurious interrupt, we don't call 
interrupt_end, and so the scheduler never gets decremented.

Am I missing something here?

Andrew Parlane
Carallon ltd.


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Nick Garnett]

On 31/10/13 17:15, Andrew Parlane wrote:
> Looking at hal/arm/arch/current/src/vectors.S in IRQ:
> 
> We increment the scheduler lock and decrement it again in interrupt_end.
> 
> In the case of there being a spurious interrupt, we don't call 
> interrupt_end, and so the scheduler never gets decremented.
> 
> Am I missing something here?

interrupt_end() does get called. A spurious interrupt only causes the
code to skip calling an ISR by jumping to the spurious_IRQ label. From
there it follows the same code path and will call interrupt_end() as normal.

-- 
Nick Garnett                                         Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com    The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.   Tel: +44 1223 245571
Registered in England and Wales:                      Reg No: 4422071

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Andrew Parlane]

Sorry, I should have been a bit more clear.
First we skip the ISR by jumping to the spurious_IRQ label, and then we 
switch stacks if necessary, then we have (line numbers may vary):

941        // The return value from the handler (in r0) will indicate 
whether a
942        // DSR is to be posted. Pass this together with a pointer to the
943        // interrupt object we have just used to the interrupt tidy 
up routine.
944
945                              // don't run this for spurious interrupts!
946        cmp     v1,#CYGNUM_HAL_INTERRUPT_NONE
947        beq     17f
948        ldr     r1,.hal_interrupt_objects
949        ldr     r1,[r1,v1,lsl #2]
950        mov     r2,v6           // register frame
951
952        THUMB_MODE(r3,10)
953
954        bl      interrupt_end   // post any bottom layer handler
955                                // threads and call scheduler
956        ARM_MODE(r1,10)
957 17:

So it compares the result of hal_IRQ_handler (stored in v1) with 
CYGNUM_HAL_INTERRUPT_NONE, and jumps forwards to label 17: which is 
after interrupt_end. if it was a spurious IRQ.

Andrew

On 01/11/2013 17:06, Nick Garnett wrote:
>
> On 31/10/13 17:15, Andrew Parlane wrote:
>> Looking at hal/arm/arch/current/src/vectors.S in IRQ:
>>
>> We increment the scheduler lock and decrement it again in interrupt_end.
>>
>> In the case of there being a spurious interrupt, we don't call
>> interrupt_end, and so the scheduler never gets decremented.
>>
>> Am I missing something here?
> interrupt_end() does get called. A spurious interrupt only causes the
> code to skip calling an ISR by jumping to the spurious_IRQ label. From
> there it follows the same code path and will call interrupt_end() as normal.
>



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Nick Garnett]

On 01/11/13 17:20, Andrew Parlane wrote:
> Sorry, I should have been a bit more clear.
> First we skip the ISR by jumping to the spurious_IRQ label, and then we 
> switch stacks if necessary, then we have (line numbers may vary):
> 
> 941        // The return value from the handler (in r0) will indicate 
> whether a
> 942        // DSR is to be posted. Pass this together with a pointer to the
> 943        // interrupt object we have just used to the interrupt tidy 
> up routine.
> 944
> 945                              // don't run this for spurious interrupts!
> 946        cmp     v1,#CYGNUM_HAL_INTERRUPT_NONE
> 947        beq     17f
> 948        ldr     r1,.hal_interrupt_objects
> 949        ldr     r1,[r1,v1,lsl #2]
> 950        mov     r2,v6           // register frame
> 951
> 952        THUMB_MODE(r3,10)
> 953
> 954        bl      interrupt_end   // post any bottom layer handler
> 955                                // threads and call scheduler
> 956        ARM_MODE(r1,10)
> 957 17:
> 
> So it compares the result of hal_IRQ_handler (stored in v1) with 
> CYGNUM_HAL_INTERRUPT_NONE, and jumps forwards to label 17: which is 
> after interrupt_end. if it was a spurious IRQ.


Hmm. You're right. That is clearly wrong. Our own sources have the
following code, which is slightly different:

        // The return value from the handler (in r0) will indicate
whether a
        // DSR is to be posted. Pass this together with a pointer to the
        // interrupt object we have just used to the interrupt tidy up
routine.

        // For a spurious interrupt, pass a NULL object. interrupt_end()
will
        // handle that and still unlock the scheduler.
        cmp     v1,#CYGNUM_HAL_INTERRUPT_NONE
        moveq   r1,#0
        beq     17f
        ldr     r1,.hal_interrupt_objects
        ldr     r1,[r1,v1,lsl #2]
17:
        mov     r2,v6           // register frame


So interrupt_end does get called, but with a NULL interrupt object pointer.


-- 
Nick Garnett                                         Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com    The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.   Tel: +44 1223 245571
Registered in England and Wales:                      Reg No: 4422071

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Andrew Parlane]

Excellent thanks. I guess we're a bit out of date.

Thanks again,
Andrew

On 01/11/2013 17:42, Nick Garnett wrote:
>
> On 01/11/13 17:20, Andrew Parlane wrote:
>> Sorry, I should have been a bit more clear.
>> First we skip the ISR by jumping to the spurious_IRQ label, and then we
>> switch stacks if necessary, then we have (line numbers may vary):
>>
>> 941        // The return value from the handler (in r0) will indicate
>> whether a
>> 942        // DSR is to be posted. Pass this together with a pointer to the
>> 943        // interrupt object we have just used to the interrupt tidy
>> up routine.
>> 944
>> 945                              // don't run this for spurious interrupts!
>> 946        cmp     v1,#CYGNUM_HAL_INTERRUPT_NONE
>> 947        beq     17f
>> 948        ldr     r1,.hal_interrupt_objects
>> 949        ldr     r1,[r1,v1,lsl #2]
>> 950        mov     r2,v6           // register frame
>> 951
>> 952        THUMB_MODE(r3,10)
>> 953
>> 954        bl      interrupt_end   // post any bottom layer handler
>> 955                                // threads and call scheduler
>> 956        ARM_MODE(r1,10)
>> 957 17:
>>
>> So it compares the result of hal_IRQ_handler (stored in v1) with
>> CYGNUM_HAL_INTERRUPT_NONE, and jumps forwards to label 17: which is
>> after interrupt_end. if it was a spurious IRQ.
>
> Hmm. You're right. That is clearly wrong. Our own sources have the
> following code, which is slightly different:
>
>          // The return value from the handler (in r0) will indicate
> whether a
>          // DSR is to be posted. Pass this together with a pointer to the
>          // interrupt object we have just used to the interrupt tidy up
> routine.
>
>          // For a spurious interrupt, pass a NULL object. interrupt_end()
> will
>          // handle that and still unlock the scheduler.
>          cmp     v1,#CYGNUM_HAL_INTERRUPT_NONE
>          moveq   r1,#0
>          beq     17f
>          ldr     r1,.hal_interrupt_objects
>          ldr     r1,[r1,v1,lsl #2]
> 17:
>          mov     r2,v6           // register frame
>
>
> So interrupt_end does get called, but with a NULL interrupt object pointer.
>
>



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Nick Garnett]

On 01/11/13 17:46, Andrew Parlane wrote:
> Excellent thanks. I guess we're a bit out of date.

Not really. I have an internal change here that is not in the public
sources. I'll take a look at pushing it out to the public repository. In
the meantime you can just update your sources locally.




-- 
Nick Garnett                                         Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com    The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.   Tel: +44 1223 245571
Registered in England and Wales:                      Reg No: 4422071

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Spurious interrupt on ARM.

[Andrew Parlane]

Ah, ok.

I already fixed it locally, I just wanted to make sure I wasn't missing 
something and also make others aware of the issue.

Thanks,
Andrew

On 01/11/2013 17:50, Nick Garnett wrote:
>
> On 01/11/13 17:46, Andrew Parlane wrote:
>> Excellent thanks. I guess we're a bit out of date.
> Not really. I have an internal change here that is not in the public
> sources. I'll take a look at pushing it out to the public repository. In
> the meantime you can just update your sources locally.
>
>
>
>



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss