From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14971 invoked by alias); 22 Aug 2007 08:42:50 -0000 Received: (qmail 14584 invoked by uid 22791); 22 Aug 2007 08:42:49 -0000 X-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DK_POLICY_SIGNSOME,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from londo.lunn.ch (HELO londo.lunn.ch) (80.238.139.98) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 22 Aug 2007 08:42:47 +0000 Received: from lunn by londo.lunn.ch with local (Exim 3.36 #1 (Debian)) id 1INlnY-0001pR-00; Wed, 22 Aug 2007 10:42:44 +0200 Date: Wed, 22 Aug 2007 08:42:00 -0000 From: Andrew Lunn To: cetoni GmbH - Uwe Kindler Cc: ecos-patches@sourceware.org, Hans Rosenfeld Subject: Re: LPC2xxx patch for support of vectored interrupt controller Message-ID: <20070822084244.GG31057@lunn.ch> References: <46CA81F3.60109@cetoni.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46CA81F3.60109@cetoni.de> User-Agent: Mutt/1.5.16 (2007-06-11) X-IsSubscribed: yes Mailing-List: contact ecos-patches-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: ecos-patches-owner@ecos.sourceware.org X-SW-Source: 2007-08/txt/msg00040.txt.bz2 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