[ECOS] sa11x0 spurious interrupts

[ECOS] sa11x0 spurious interrupts

[Robin Farine]

Hello,

While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
interrupt sources, I noticed that when it does not find an interrupt source,
the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
platform. However, the common ARM code in "vectors.S" assumes that a spurious
interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!

Did I miss something?

Thanks,

Robin

Re: [ECOS] sa11x0 spurious interrupts

[Jonathan Larmour]

Robin Farine wrote:
> 
> Hello,
> 
> While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
> interrupt sources, I noticed that when it does not find an interrupt source,
> the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
> platform. However, the common ARM code in "vectors.S" assumes that a spurious
> interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
> 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
> 
> Did I miss something?

I don't think so. 0 can be a valid ISR. What's worse is that the default
ISR in hal/common also returns 0 to indicate a spurious interrupt.

Anyone got any opinions why this isn't simply all wrong?

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] sa11x0 spurious interrupts

[Gary Thomas]

On 13-Feb-2001 Jonathan Larmour wrote:
> Robin Farine wrote:
>> 
>> Hello,
>> 
>> While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
>> interrupt sources, I noticed that when it does not find an interrupt source,
>> the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
>> platform. However, the common ARM code in "vectors.S" assumes that a spurious
>> interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
>> 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
>> 
>> Did I miss something?
> 
> I don't think so. 0 can be a valid ISR. What's worse is that the default
> ISR in hal/common also returns 0 to indicate a spurious interrupt.

The default ISR handler returning 0 is a whole separate matter.

> 
> Anyone got any opinions why this isn't simply all wrong?

Actually, it does seem to be rather messed up.  It comes from having a large
number of ports and this particular behaviour is platform specific, thus 
we've somehow ended up with no less than 3 different answers here.

These should all be changed to return CYGNUM_HAL_INTERRUPT_NONE and have
the code in 'vectors.S' handle this as a special case.  However, what one
actually does when there is a spurious interrupt is tinder for a large
flame war :-)

Re: [ECOS] sa11x0 spurious interrupts

[Robin Farine]

Jonathan Larmour <jlarmour@redhat.com> writes:

> Robin Farine wrote:
> > 
> > Hello,
> > 
> > While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
> > interrupt sources, I noticed that when it does not find an interrupt source,
> > the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
> > platform. However, the common ARM code in "vectors.S" assumes that a spurious
> > interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
> > 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
> > 
> > Did I miss something?

> ...
> I don't think so. 0 can be a valid ISR. What's worse is that the default
> ISR in hal/common also returns 0 to indicate a spurious interrupt.
> 
> Anyone got any opinions why this isn't simply all wrong?

I think that the sa11x0 'hal_IRQ_handler()' routine does the right thing
(returning -1) and that "vector.S" should in any case check that the vector
returned is in range and if not, call the spurious interrupt handler if the
configuration requested it.

Robin

Re: [ECOS] sa11x0 spurious interrupts

[Jonathan Larmour]

Gary Thomas wrote:
> 
> On 13-Feb-2001 Jonathan Larmour wrote:
> > Robin Farine wrote:
> >>
> >> Hello,
> >>
> >> While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
> >> interrupt sources, I noticed that when it does not find an interrupt source,
> >> the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
> >> platform. However, the common ARM code in "vectors.S" assumes that a spurious
> >> interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
> >> 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
> >>
> >> Did I miss something?
> >
> > I don't think so. 0 can be a valid ISR. What's worse is that the default
> > ISR in hal/common also returns 0 to indicate a spurious interrupt.
> 
> The default ISR handler returning 0 is a whole separate matter.

Oops, I read too much into the name hal_IRQ_handler.
 
> > Anyone got any opinions why this isn't simply all wrong?
> 
> Actually, it does seem to be rather messed up.  It comes from having a large
> number of ports and this particular behaviour is platform specific, thus
> we've somehow ended up with no less than 3 different answers here.

Yes I noticed on the AEB that it returns 13!
 
> These should all be changed to return CYGNUM_HAL_INTERRUPT_NONE

where CYGNUM_HAL_INTERRUPT_NONE is defined on every platform to be -1 I
presume.

> and have
> the code in 'vectors.S' handle this as a special case.  However, what one
> actually does when there is a spurious interrupt is tinder for a large
> flame war :-)

It depends on how spurious is spurious :).

Robin would you care to have a go at fixing this for all the
hal_IRQ_handlers, i.e. in the aeb, pid, cma230, ebsa285, edb7xxx, and
sa11x0/var dirs? As well as vector.S of course. ChangeLog entries would be
superb :-).

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] sa11x0 spurious interrupts

[Gary Thomas]

On 13-Feb-2001 Jonathan Larmour wrote:
> Gary Thomas wrote:
>> 
>> On 13-Feb-2001 Jonathan Larmour wrote:
>> > Robin Farine wrote:
>> >>
>> >> Hello,
>> >>
>> >> While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
>> >> interrupt sources, I noticed that when it does not find an interrupt source,
>> >> the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
>> >> platform. However, the common ARM code in "vectors.S" assumes that a spurious
>> >> interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
>> >> 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
>> >>
>> >> Did I miss something?
>> >
>> > I don't think so. 0 can be a valid ISR. What's worse is that the default
>> > ISR in hal/common also returns 0 to indicate a spurious interrupt.
>> 
>> The default ISR handler returning 0 is a whole separate matter.
> 
> Oops, I read too much into the name hal_IRQ_handler.
>  
>> > Anyone got any opinions why this isn't simply all wrong?
>> 
>> Actually, it does seem to be rather messed up.  It comes from having a large
>> number of ports and this particular behaviour is platform specific, thus
>> we've somehow ended up with no less than 3 different answers here.
> 
> Yes I noticed on the AEB that it returns 13!
>  
>> These should all be changed to return CYGNUM_HAL_INTERRUPT_NONE
> 
> where CYGNUM_HAL_INTERRUPT_NONE is defined on every platform to be -1 I
> presume.
> 

Not necessarily.  The code in 'vectors.S' should just check for this [named] value.

>> and have
>> the code in 'vectors.S' handle this as a special case.  However, what one
>> actually does when there is a spurious interrupt is tinder for a large
>> flame war :-)
> 
> It depends on how spurious is spurious :).
> 
> Robin would you care to have a go at fixing this for all the
> hal_IRQ_handlers, i.e. in the aeb, pid, cma230, ebsa285, edb7xxx, and
> sa11x0/var dirs? As well as vector.S of course. ChangeLog entries would be
> superb :-).

If not, I can take care of it as well.

Re: [ECOS] sa11x0 spurious interrupts

[Robin Farine]

Gary Thomas <gthomas@cambridge.redhat.com> writes:

> On 13-Feb-2001 Jonathan Larmour wrote:
> > Gary Thomas wrote:
> >> 
> >> On 13-Feb-2001 Jonathan Larmour wrote:
> >> > Robin Farine wrote:
> >> >>
> >> >> Hello,
> >> >>
> >> >> While looking at the way the sa11x0 hal's routine 'hal_IRQ_handler()' decodes
> >> >> interrupt sources, I noticed that when it does not find an interrupt source,
> >> >> the routine returns CYGNUM_HAL_INTERRUPT_NONE, which equals to -1 for this
> >> >> platform. However, the common ARM code in "vectors.S" assumes that a spurious
> >> >> interrupt always have the vector #0. And worse, 'handle_IRQ_or_FIQ' will call
> >> >> 'hal_interrupt_handlers[-1]' which contains 0 and thus reboot!
> >> >>
> >> >> Did I miss something?
> >> >
> >> > I don't think so. 0 can be a valid ISR. What's worse is that the default
> >> > ISR in hal/common also returns 0 to indicate a spurious interrupt.
> >> 
> >> The default ISR handler returning 0 is a whole separate matter.
> > 
> > Oops, I read too much into the name hal_IRQ_handler.
> >  
> >> > Anyone got any opinions why this isn't simply all wrong?
> >> 
> >> Actually, it does seem to be rather messed up.  It comes from having a large
> >> number of ports and this particular behaviour is platform specific, thus
> >> we've somehow ended up with no less than 3 different answers here.
> > 
> > Yes I noticed on the AEB that it returns 13!
> >  
> >> These should all be changed to return CYGNUM_HAL_INTERRUPT_NONE
> > 
> > where CYGNUM_HAL_INTERRUPT_NONE is defined on every platform to be -1 I
> > presume.
> > 
> 
> Not necessarily.  The code in 'vectors.S' should just check for this [named] value.
> 
> >> and have
> >> the code in 'vectors.S' handle this as a special case.  However, what one
> >> actually does when there is a spurious interrupt is tinder for a large
> >> flame war :-)
> > 
> > It depends on how spurious is spurious :).
> > 
> > Robin would you care to have a go at fixing this for all the
> > hal_IRQ_handlers, i.e. in the aeb, pid, cma230, ebsa285, edb7xxx, and
> > sa11x0/var dirs? As well as vector.S of course. ChangeLog entries would be
> > superb :-).

I'd *love* the ChangeLog entries but, you know, with all the other things I still
have to do ...
 
> If not, I can take care of it as well.

... yes, thanks Gary! And so you can immediately incorporate these changes to
the beasts in your farm so that I get a brand new working eCos for my plc2 ;-].

Robin