[ECOS] AT91 bad IRQ/FIQ priority handling?

[ECOS] AT91 bad IRQ/FIQ priority handling?

[Andrea Michelotti]

Hi everybody,
looking at at91_misc.c :

int hal_IRQ_handler(void)

I think there is a bug in irq fiq priority when a FIQ and an IRQ are arised
together.
In this case IVR reading updates ISR but with the IRQ number not with the
FIQ one. Then
the irq is served instead of fiq (changing SMR priority doesn't change order
always IRQ before FIQ) .
Is this behavior considered a bug?
I did several trials with my jtst board a new Atmel board with an ARM7+DSP
embedded (where the DSP generates almost simulataneous FIQs and IRQs).
I did a simple patch that solve my problem that is a mix of old and new ecos
irq handling method.

thanks.
Andrea.

------------------------------------------------------------------------
Andrea Michelotti - HW/SW Co-Design Manager
ATMEL Rome -


-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrew Lunn]

On Tue, Sep 14, 2004 at 10:57:34AM +0200, Andrea Michelotti wrote:
> Hi everybody,
> looking at at91_misc.c :
> 
> int hal_IRQ_handler(void)
> 
> I think there is a bug in irq fiq priority when a FIQ and an IRQ are arised
> together.
> In this case IVR reading updates ISR but with the IRQ number not with the
> FIQ one. Then
> the irq is served instead of fiq (changing SMR priority doesn't change order
> always IRQ before FIQ) .
> Is this behavior considered a bug?
> I did several trials with my jtst board a new Atmel board with an ARM7+DSP
> embedded (where the DSP generates almost simulataneous FIQs and IRQs).
> I did a simple patch that solve my problem that is a mix of old and new ecos
> irq handling method.

I know when i last looked, FIQ was not really supported. I have used
the FIQ on an ebsa285 but i did that with my own interrupt handling
code one the FIQ vector using cyg_interrupt_set_vsr().

Probably the best thing to do is clean up you patch and then post it
so we can discuss it.

        Andrew

-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrea Michelotti]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Hi Andrew,
here the patch.

best regards
Andrea.

----- Original Message ----- 
From: "Andrew Lunn" <andrew@lunn.ch>
To: "Andrea Michelotti" <amichelotti@atmel.com>
Cc: <ecos-discuss@sources.redhat.com>
Sent: Tuesday, September 14, 2004 11:19 AM
Subject: Re: [ECOS] AT91 bad IRQ/FIQ priority handling?


> On Tue, Sep 14, 2004 at 10:57:34AM +0200, Andrea Michelotti wrote:
> > Hi everybody,
> > looking at at91_misc.c :
> >
> > int hal_IRQ_handler(void)
> >
> > I think there is a bug in irq fiq priority when a FIQ and an IRQ are
arised
> > together.
> > In this case IVR reading updates ISR but with the IRQ number not with
the
> > FIQ one. Then
> > the irq is served instead of fiq (changing SMR priority doesn't change
order
> > always IRQ before FIQ) .
> > Is this behavior considered a bug?
> > I did several trials with my jtst board a new Atmel board with an
ARM7+DSP
> > embedded (where the DSP generates almost simulataneous FIQs and IRQs).
> > I did a simple patch that solve my problem that is a mix of old and new
ecos
> > irq handling method.
>
> I know when i last looked, FIQ was not really supported. I have used
> the FIQ on an ebsa285 but i did that with my own interrupt handling
> code one the FIQ vector using cyg_interrupt_set_vsr().
>
> Probably the best thing to do is clean up you patch and then post it
> so we can discuss it.
>
>         Andrew
>
> -- 
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
>
>

[-- Attachment #2: at91_misc.c.patch --]
[-- Type: application/octet-stream, Size: 961 bytes --]

Index: at91_misc.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/at91/var/current/src/at91_misc.c,v
retrieving revision 1.8
diff -u -5 -p -r1.8 at91_misc.c
--- at91_misc.c	12 Aug 2004 13:02:24 -0000	1.8
+++ at91_misc.c	14 Sep 2004 09:35:58 -0000
@@ -177,11 +177,18 @@ void hal_hardware_init(void)
 // should interrogate the hardware and return the IRQ vector number.
 
 int hal_IRQ_handler(void)
 {
     cyg_uint32 irq_num;
-    cyg_uint32 ivr;
+    cyg_uint32 ivr,ipr,imr;
+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IPR, ipr);
+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IMR, imr);
+    
+    if(imr&ipr&0x1){
+      HAL_WRITE_UINT32(AT91_AIC+AT91_AIC_ICCR, 1); // we must clean pending fiq
+      return 0;
+    }
     
     // Calculate active interrupt (updates ISR)
     HAL_READ_UINT32(AT91_AIC+AT91_AIC_IVR, ivr);
 
     HAL_READ_UINT32(AT91_AIC+AT91_AIC_ISR, irq_num);


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrew Lunn]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Tue, Sep 14, 2004 at 11:48:05AM +0200, Andrea Michelotti wrote:
> int hal_IRQ_handler(void)
> {
>     cyg_uint32 irq_num;
>-    cyg_uint32 ivr;
>+    cyg_uint32 ivr,ipr,imr;
>+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IPR, ipr);
>+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IMR, imr);
>+    
>+    if(imr&ipr&0x1){
>+      HAL_WRITE_UINT32(AT91_AIC+AT91_AIC_ICCR, 1); // we must clean pending fiq
>+      return 0;
>+    }
>     
>     // Calculate active interrupt (updates ISR)
>     HAL_READ_UINT32(AT91_AIC+AT91_AIC_IVR, ivr);
> 
>     HAL_READ_UINT32(AT91_AIC+AT91_AIC_ISR, irq_num);

Returning 0 is wrong. You need to return the interrupt source as
defined in hal_platform_ints.h. 

        Andrew

[-- Attachment #2: at91_misc.c.patch --]
[-- Type: application/octet-stream, Size: 961 bytes --]

Index: at91_misc.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/at91/var/current/src/at91_misc.c,v
retrieving revision 1.8
diff -u -5 -p -r1.8 at91_misc.c
--- at91_misc.c	12 Aug 2004 13:02:24 -0000	1.8
+++ at91_misc.c	14 Sep 2004 09:35:58 -0000
@@ -177,11 +177,18 @@ void hal_hardware_init(void)
 // should interrogate the hardware and return the IRQ vector number.
 
 int hal_IRQ_handler(void)
 {
     cyg_uint32 irq_num;
-    cyg_uint32 ivr;
+    cyg_uint32 ivr,ipr,imr;
+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IPR, ipr);
+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IMR, imr);
+    
+    if(imr&ipr&0x1){
+      HAL_WRITE_UINT32(AT91_AIC+AT91_AIC_ICCR, 1); // we must clean pending fiq
+      return 0;
+    }
     
     // Calculate active interrupt (updates ISR)
     HAL_READ_UINT32(AT91_AIC+AT91_AIC_IVR, ivr);
 
     HAL_READ_UINT32(AT91_AIC+AT91_AIC_ISR, irq_num);


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrea Michelotti]

The code I added is related only to the FIQ interrupt source generally
linked to the line 0 of the AIC (atmel Advanced Interrupt Controller) .
In fact if the FIQ line is linked to another line (possible, but, as far as
I know,  in atmel AIC fiq line is for default to 0) this code is not correct
but should be modified introducing a constant that identify the FIQ line.
Does hal_platform_ints.h have a macro to identify FIQ interrupt sources?
If not, I propose to add one to handle this kind of situations in at91
targets.
what do you think?

thanks
Andrea.


> On Tue, Sep 14, 2004 at 11:48:05AM +0200, Andrea Michelotti wrote:
> > int hal_IRQ_handler(void)
> > {
> >     cyg_uint32 irq_num;
> >-    cyg_uint32 ivr;
> >+    cyg_uint32 ivr,ipr,imr;
> >+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IPR, ipr);
> >+    HAL_READ_UINT32(AT91_AIC+AT91_AIC_IMR, imr);
> >+
> >+    if(imr&ipr&0x1){
> >+      HAL_WRITE_UINT32(AT91_AIC+AT91_AIC_ICCR, 1); // we must clean
pending fiq
> >+      return 0;
> >+    }
> >
> >     // Calculate active interrupt (updates ISR)
> >     HAL_READ_UINT32(AT91_AIC+AT91_AIC_IVR, ivr);
> >
> >     HAL_READ_UINT32(AT91_AIC+AT91_AIC_ISR, irq_num);
>
> Returning 0 is wrong. You need to return the interrupt source as
> defined in hal_platform_ints.h.
>
>         Andrew
>


----------------------------------------------------------------------------
----


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


-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrew Lunn]

On Tue, Sep 14, 2004 at 05:00:57PM +0200, Andrea Michelotti wrote:
> The code I added is related only to the FIQ interrupt source generally
> linked to the line 0 of the AIC (atmel Advanced Interrupt Controller) .
> In fact if the FIQ line is linked to another line (possible, but, as far as
> I know,  in atmel AIC fiq line is for default to 0) this code is not correct
> but should be modified introducing a constant that identify the FIQ line.
> Does hal_platform_ints.h have a macro to identify FIQ interrupt sources?
> If not, I propose to add one to handle this kind of situations in at91
> targets.
> what do you think?

That sounds like a better solution. But since this is in the hal
varient directory you need to check if this is correct for all
platforms that use the varient, ie eb40, eb40a, eb52 and eb55. If this
is not generally true you will need to add some CDL so that this
feature is only enabled on platforms that support it.

        Andrew

-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrea Michelotti]

Hi Andrew,
I did the check: all at91 targets have an AIC line 0 dedicated to FIQ (in
these platforms FIQ is an external interrupt ).
Then I think we don't need a CDL entry, a constant should be enough, but I
don't know the appropriate header where to put a constant like
CYGNUM_HAL_INTERRUPT_FIQ 0 for all at91 targets.

thanks
Andrea.



> On Tue, Sep 14, 2004 at 05:00:57PM +0200, Andrea Michelotti wrote:
> > The code I added is related only to the FIQ interrupt source generally
> > linked to the line 0 of the AIC (atmel Advanced Interrupt Controller) .
> > In fact if the FIQ line is linked to another line (possible, but, as far
as
> > I know,  in atmel AIC fiq line is for default to 0) this code is not
correct
> > but should be modified introducing a constant that identify the FIQ
line.
> > Does hal_platform_ints.h have a macro to identify FIQ interrupt sources?
> > If not, I propose to add one to handle this kind of situations in at91
> > targets.
> > what do you think?
>
> That sounds like a better solution. But since this is in the hal
> varient directory you need to check if this is correct for all
> platforms that use the varient, ie eb40, eb40a, eb52 and eb55. If this
> is not generally true you will need to add some CDL so that this
> feature is only enabled on platforms that support it.
>
>         Andrew
>
> -- 
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
>
>


-- 
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] AT91 bad IRQ/FIQ priority handling?

[Andrew Lunn]

On Tue, Sep 14, 2004 at 06:43:18PM +0200, Andrea Michelotti wrote:
> Hi Andrew,
> I did the check: all at91 targets have an AIC line 0 dedicated to FIQ (in
> these platforms FIQ is an external interrupt ).
> Then I think we don't need a CDL entry, a constant should be enough, but I
> don't know the appropriate header where to put a constant like
> CYGNUM_HAL_INTERRUPT_FIQ 0 for all at91 targets.

Zero is probably not the correct value, it needs to fit in with the
other constants. The files you need to change are:

hal/arm/at91/eb40/current/include/hal_platform_ints.h
hal/arm/at91/eb40a/current/include/hal_platform_ints.h
hal/arm/at91/eb42/current/include/hal_platform_ints.h
hal/arm/at91/eb55/current/include/hal_platform_ints.h

        Andrew

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

[ECOS] Re: AT91 bad IRQ/FIQ priority handling?

[Daniel Néri]

"Andrea Michelotti" <amichelotti@atmel.com> writes:

> I think there is a bug in irq fiq priority when a FIQ and an IRQ are
> arised together. In this case IVR reading updates ISR but with the
> IRQ number not with the FIQ one. Then the irq is served instead of
> fiq (changing SMR priority doesn't change order always IRQ before
> FIQ) .

Yes, I have noticed this too. But since FIQ handling isn't really well
supported by the ARM architecture HAL, I didn't find it worthwhile to
try to make it work on the AT91 specifically.

> Is this behavior considered a bug? I did several trials with my jtst
> board a new Atmel board with an ARM7+DSP embedded (where the DSP
> generates almost simulataneous FIQs and IRQs). I did a simple patch
> that solve my problem that is a mix of old and new ecos irq handling
> method.

In our platform HAL, I solved it with a simple VSR, that triggers the
software interrupt in the AIC (see below).

The interrupt handler for the hardware device is then attached to the
SWIRQ interrupt source instead.


Best wishes,
  --Daniel



#include <pkgconf/hal_arm_at91.h>
#include <cyg/hal/hal_platform_ints.h>
#include <cyg/hal/var_io.h>

#include <cyg/hal/arch.inc>

        .text
        .code   32

FUNC_START(platform_fiq_vsr)
        ldr     r8,=AT91_AIC
        ldr     r9,[r8,#AT91_AIC_FVR]   // clear interrupt (if edge triggered)

        ldr     r9,=(1 << CYGNUM_HAL_INTERRUPT_SW)
        str     r9,[r8,#AT91_AIC_ISCR]

        subs    pc,lr,#4


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