Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld <rosenfeld at grumpf dot hope-2000 dot org>]

On Wed, Aug 22, 2007 at 10:42:44AM +0200, Andrew Lunn wrote:
> On Tue, Aug 21, 2007 at 08:10:59AM +0200, cetoni GmbH - Uwe Kindler wrote:
> > Btw. the assertion I put into the code is wrong:
> >
> > CYG_ASSERT(vector <= CYGNUM_HAL_INTERRUPT_EINT3 &&
> >            vector >= CYGNUM_HAL_INTERRUPT_EINT0 , "Invalid vector");
> >
> > should be:
> >
> > CYG_ASSERT(vector <= CYGNUM_HAL_INTERRUPT_EINT3 ||
> >            vector >= CYGNUM_HAL_INTERRUPT_EINT0 , "Invalid vector");
> 
> Please could you submit a patch.

May I suggest that you think about this again? 

The only configurable interrupts are EINT0 to EINT3. So this assertion
makes sure that the vector passed to hal_interrupt configure is less or
equal to EINT3 _and_ greater or equal to EINT0. And if this is not true
the assertion fails. If you change the assertion the way suggested you
could also just remove it, since _all_ vectors are either less or equal
to EINT3, or greater or equal to EINT0, or both.

> Should there also be another check like:
> 
>  CYG_ASSERT(vector <= CYGNUM_HAL_ISR_MAX &&
>             vector >= CYGNUM_HAL_ISR_MIN , "Invalid vector");
> 
> If you pass it an invalid vector, such as 33, you end up shifting 1 by
> 33 which i guess results in 0. The code will then set/reset the level
> on every single interrupt!

This assertion cannot fail if the previous assertion did not fail, and
if the previous assertion failed it is never reached. Any vector less or
equal to EINT3 is also less than ISR_MAX, and any vector greater or equal
to EINT0 is also greater than ISR_MIN.
 
> If the hardware is not capable of setting priority and level for these
> vectors, we should check for this. I suggest we make the test case a
> little bit more intelligent so that it only tries to do things which
> the hardware is capable of doing. In some respect, the test failing is
> correct!

This could probably be done by introducing something like
CYGNUM_HAL_ISR_CONF_MAX and _MIN to the HALs.


-- 
%SYSTEM-F-ANARCHISM, The operating system has been overthrown