Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Aug 21, 2007 at 08:10:59AM +0200, cetoni GmbH - Uwe Kindler wrote:
> Hello Hans,
>
> ------>
> I think it would be better to change this assertion into
> a simple test, so that calling hal_interrupt_configure() with an
> un-configurable interrupt vector just does nothing.
> <-----
>
> I dont't agree with this change. If I configure a system and do a mistake 
> setting up interrupt priorities then my only chance and a very good way to 
> catch this error is this assertion. If you silently drop this failure just 
> to make a test case happy then you may pass the test but you will run into 
> trouble with your real application.
>
> 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. 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!

> Hello Andrew, what do you think about the patch. Is it okay to remove the 
> assertion just to make test cases happy?

I would prefer to keep the assertion. Interrupt code is never easy to
debug and get right. When it does not work you have little idea why it
does not work. So i like tests like this which will prevent some of
the silly mistakes.

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!

       Andrew