LPC2xxx patch for support of vectored interrupt controller

LPC2xxx patch for support of vectored interrupt controller

[uwe.kindler]

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

Hello,

the following patch adds support for the vectored interrupt controller to the LPC2xxx HAL. Now the LPC2xxx HAL supports up to 17 interrupt priorities. This patch improves interrupt processing times for vectored IRQs and the speed of the whole system because also the system clock interrupt processing time is improved.

Regards, Uwe

[-- Attachment #2: lpc2xxx.patch --]
[-- Type: application/octet-stream, Size: 6933 bytes --]

diff -ru ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog ecos/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog
--- ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog	2007-07-02 07:47:58.000000000 +0200
+++ ecos/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog	2007-07-10 08:06:09.000000000 +0200
@@ -1,3 +1,15 @@
+2007_07-10  Uwe Kindler <uwe_kindler@web.de>
+
+	* cdl/hal_arm_lpc2xxx.cdl: Added option
+	CYGNUM_HAL_KERNEL_COUNTERS_CLOCK_ISR_DEFAULT_PRIORITY for
+	configuration of priority of system clock interrupts.
+	
+	* src/lpc2xxx_misc.c: Added support for vectored interrupt
+	controller and up to 17 interrupt priorities. This improves
+	interrupt processing time and makes processing of vectored
+	interrupts more determenistic because no for loop is required
+	for detection of interrupt source.
+
 2007-06-04  Alexey Shusharin <mrfinch@mail.ru>
 
 	* src/hal_diag.c (cyg_hal_plf_serial_isr): Fixed issue with UART
diff -ru ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl ecos/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl
--- ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl	2007-03-23 10:42:35.000000000 +0100
+++ ecos/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl	2007-07-10 08:04:45.000000000 +0200
@@ -206,4 +206,20 @@
            debugging via JTAG, as stopping the clock can prevent the
            debugger getting control of the system."
     }
+    
+    cdl_option CYGNUM_HAL_KERNEL_COUNTERS_CLOCK_ISR_DEFAULT_PRIORITY {
+	    display		"Default priority for the system clock interrupts"
+	    flavor		data
+	    legal_values  { 0 to 16 }
+        default_value 0
+	    description "
+            The LPC2xxx eCos HAL supports up to 17 interrupt levels.
+            Interrupt levels 0 - 15 are vectored IRQs. Vectored IRQs 
+            have a higher priority then non vectored IRQs and they 
+            are processed faster. Non vectored  IRQs are all chained together 
+            into one single slot and the ISR need to  find out which interrupt 
+            occured. The default value for the system clock interrupts is 0 - 
+            this is the highest priority IRQ."
+    }
+
 }
diff -ru ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/src/lpc2xxx_misc.c ecos/ecos/packages/hal/arm/lpc2xxx/var/current/src/lpc2xxx_misc.c
--- ecos_web_cvs/ecos/packages/hal/arm/lpc2xxx/var/current/src/lpc2xxx_misc.c	2006-08-01 08:02:58.000000000 +0200
+++ ecos/ecos/packages/hal/arm/lpc2xxx/var/current/src/lpc2xxx_misc.c	2007-07-10 08:05:30.000000000 +0200
@@ -240,6 +240,13 @@
     lpc_set_vpbdiv(CYGNUM_HAL_ARM_LPC2XXX_VPBDIV, 1);
 #endif
 
+    //
+    // 0xFFFFFFFF indicates that this is a non vectored ISR
+    // This is the default setting for all  interrupts
+    //
+    HAL_WRITE_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
+                     CYGARC_HAL_LPC2XXX_REG_VICDEFVECTADDR, 0xFFFFFFFF);
+
 #ifdef HAL_PLF_HARDWARE_INIT
     // Perform any platform specific initializations
     HAL_PLF_HARDWARE_INIT();
@@ -254,22 +261,35 @@
 // should interrogate the hardware and return the IRQ vector number.
 int hal_IRQ_handler(void)
 {
-    cyg_uint32 irq_num, irq_stat;
-
-    // Find out which interrupt caused the IRQ. This picks the lowest
-    // if there are more than 1.
-    // FIXME:try to make use of the VIC for better latency.
-    //       That will probably need changes to vectors.S and 
-    //       other int-related code
+    cyg_uint32 irq_num;
+    
     HAL_READ_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
-                    CYGARC_HAL_LPC2XXX_REG_VICIRQSTAT, irq_stat);
-    for (irq_num = 0; irq_num < 32; irq_num++)
-      if (irq_stat & (1 << irq_num))
-        break;
-    
-    // If not a valid interrrupt source, treat as spurious interrupt    
-    if (irq_num < CYGNUM_HAL_ISR_MIN || irq_num > CYGNUM_HAL_ISR_MAX)
-      irq_num = CYGNUM_HAL_INTERRUPT_NONE;
+                    CYGARC_HAL_LPC2XXX_REG_VICVECTADDR, irq_num);
+    //
+    // if this is a non vectored ISR then we need to find out which interrupt 
+    // caused the IRQ
+    //      
+    if (0xFFFFFFFF == irq_num)
+    {
+        cyg_uint32 irq_stat;
+        
+        // Find out which interrupt caused the IRQ. This picks the lowest
+        // if there are more than 1.
+        HAL_READ_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
+                        CYGARC_HAL_LPC2XXX_REG_VICIRQSTAT, irq_stat);
+        irq_num = 0;
+        while (!(irq_stat & 0x01))
+        {
+            irq_stat >>= 1;	
+            irq_num++;
+        }
+        
+        // If not a valid interrrupt source, treat as spurious interrupt    
+        if (irq_num < CYGNUM_HAL_ISR_MIN || irq_num > CYGNUM_HAL_ISR_MAX)
+        {
+            irq_num = CYGNUM_HAL_INTERRUPT_NONE;
+        }
+    } // if (0xFFFFFFFF == irq_num)
     
     return (irq_num);
 }
@@ -420,12 +440,43 @@
                      CYGARC_HAL_LPC2XXX_REG_EXTINT, vector);
 }
 
-// Change interrupt level. This is a non-operation on the LPC2XXX
+//
+// We support up to 17 interrupt levels
+// Interrupts 0 - 15 are vectored interrupt requests. Vectored IRQs have 
+// the higher priority then non vectored IRQs, but only 16 of the 32 requests 
+// can be assigned to this category. Any of the 32 requests can be assigned 
+// to any of the 16 vectored IRQ slots, among which slot 0 has the highest 
+// priority and slot 15 has the lowest. Priority 16 indicates a non vectored
+// IRQ.
+//
 void hal_interrupt_set_level(int vector, int level)
 {
     CYG_ASSERT(vector <= CYGNUM_HAL_ISR_MAX &&
                vector >= CYGNUM_HAL_ISR_MIN , "Invalid vector");
-    CYG_ASSERT(level >= 0 && level <= 15, "Invalid level");
+    CYG_ASSERT(level >= 0 && level <= 16, "Invalid level");
+    
+    //
+    // If level is < 16 then this is a vectored ISR and we try to write
+    // the vector number of this ISR in the right slot of the vectored 
+    // interrupt controller
+    //
+    if (level < 16)
+    {
+        cyg_uint32 addr_offset =  level << 2;
+        cyg_uint32 reg_val;
+        
+        HAL_READ_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
+                        CYGARC_HAL_LPC2XXX_REG_VICVECTCNTL0 + addr_offset, reg_val);
+        CYG_ASSERT((reg_val == 0) || (reg_val == (vector | 0x20)), "Priority already used by another vector");
+        HAL_WRITE_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
+                         CYGARC_HAL_LPC2XXX_REG_VICVECTCNTL0 + addr_offset, vector | 0x20);
+        //
+        // We do not store the adress of the ISR here but we store the vector number
+        // The hal_IRQ_handler then can faster determine the right vector number
+        //
+        HAL_WRITE_UINT32(CYGARC_HAL_LPC2XXX_REG_VIC_BASE + 
+                         CYGARC_HAL_LPC2XXX_REG_VICVECTADDR0 + addr_offset, vector);
+    }     
 }
 
 // Use the watchdog to generate a reset

Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn]

On Tue, Jul 10, 2007 at 08:11:01AM +0200, uwe.kindler@cetoni.de wrote:
> Hello,
> 
> the following patch adds support for the vectored interrupt
> controller to the LPC2xxx HAL. Now the LPC2xxx HAL supports up to 17
> interrupt priorities. This patch improves interrupt processing times
> for vectored IRQs and the speed of the whole system because also the
> system clock interrupt processing time is improved.

Thanks,
        Committed,
                Andrew

Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

On Tue, Jul 10, 2007 at 08:11:01AM +0200, uwe.kindler@cetoni.de wrote:
> the following patch adds support for the vectored interrupt controller
> to the LPC2xxx HAL. Now the LPC2xxx HAL supports up to 17 interrupt
> priorities. This patch improves interrupt processing times for
> vectored IRQs and the speed of the whole system because also the
> system clock interrupt processing time is improved.

I'm a bit confused about that VIC support.

There is an assertion in hal_interrupt_set_level() that makes sure that
no priority < 16 is used more than once.

The default clock interrupt priority is 0, as are the interrupt
priorities for CAN, SPI and I2C, and the driver for the RTL8019 ethernet
chip I use doesn't even support anything else (but that could be changed).

The kintr0 test creates one interrupt of priority 0 and two of priority 1.

I doubt that this will just work, and I'm a bit unsure what the right
solution would be.

One very simple solution would be to just use the next higher free
priority if the chosen priority is already used. This will be
problematic if some code relies on the correct priority.

A less simple solution would be to fix the drivers and the kintr0 test
to use configurable priorities. If it wouldn't involve kintr0 I would
just do that.


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

Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

On Fri, Aug 17, 2007 at 04:02:09PM +0200, Hans Rosenfeld wrote:
> A less simple solution would be to fix the drivers and the kintr0 test
> to use configurable priorities. If it wouldn't involve kintr0 I would
> just do that.

I decided to do that now, but ignored kintr0 for now.

Attached are patches to the CAN, I2C, SPI, serial and the NS DP83902A
drivers to add options to change the interrupt priorities, and a patch
to the LPC2xxx HAL to add a VIC component which works as parent for
these options.

The changes to the generic serial and the NS DP83902A drivers are
minimal, other code using those drivers should not be affected.


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

[-- Attachment #2: priorities-can.diff --]
[-- Type: text/plain, Size: 10129 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/can/arm/lpc2xxx/current/ChangeLog,v
retrieving revision 1.5
diff -u -r1.5 ChangeLog
--- ChangeLog	17 Aug 2007 08:48:57 -0000	1.5
+++ ChangeLog	17 Aug 2007 15:25:04 -0000
@@ -1,3 +1,8 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/can_lpc2xxx.cdl, src/can_lpc2xxx.src: add options to set the
+          interrupt priorities
+
 2007-08-17 Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
 	
 	* src/can_lpc2xxx.c: The definition of "info" is missing when only
Index: cdl/can_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/can/arm/lpc2xxx/current/cdl/can_lpc2xxx.cdl,v
retrieving revision 1.2
diff -u -r1.2 can_lpc2xxx.cdl
--- cdl/can_lpc2xxx.cdl	31 Jul 2007 07:53:36 -0000	1.2
+++ cdl/can_lpc2xxx.cdl	17 Aug 2007 15:25:04 -0000
@@ -143,7 +143,19 @@
                 Check this box to turn ON debug options for LPC2XXXX 
                 CAN device driver."
     } 
-    
+
+    cdl_option CYGNUM_DEVS_CAN_LPC2XXX_PRIO {
+        display       "Interrupt priority for CAN and Acceptance Filter"
+        parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+        active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+        flavor        data
+        default_value 7
+        legal_values  0 to 16
+        description   "The interrupt priority corresponds to the vector
+                       number in the Vectored Interrupt Controller. Lower
+                       numbers designate higher priorities."
+    }
+
     # Support up to 4 on-chip CAN modules. The number may vary between
     # processor variants so it is easy to update this here
     for { set ::channel 0 } { $::channel < 4 } { incr ::channel } {
@@ -229,6 +241,32 @@
                     identifier."
              }
 
+            cdl_option CYGNUM_DEVS_CAN_LPC2XXX_CAN[set ::channel]_TX_PRIO {
+                display       "TX interrupt priority for CAN module [set ::channel]"
+                parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+                active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+                active_if       CYGINT_DEVS_CAN_LPC2XXX_CAN[set ::channel]
+                flavor        data
+                default_value [set ::channel] * 2 + 8
+                legal_values  0 to 16
+                description   "The interrupt priority corresponds to the vector
+                               number in the Vectored Interrupt Controller. Lower
+                               numbers designate higher priorities."
+            }
+
+            cdl_option CYGNUM_DEVS_CAN_LPC2XXX_CAN[set ::channel]_RX_PRIO {
+                display       "RX interrupt priority for CAN module [set ::channel]"
+                parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+                active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+                active_if       CYGINT_DEVS_CAN_LPC2XXX_CAN[set ::channel]
+                flavor        data
+                default_value [set ::channel] * 2 + 9
+                legal_values  0 to 16
+                description   "The interrupt priority corresponds to the vector
+                               number in the Vectored Interrupt Controller. Lower
+                               numbers designate higher priorities."
+            }
+
         }    
     } 
     
Index: src/can_lpc2xxx.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/can/arm/lpc2xxx/current/src/can_lpc2xxx.c,v
retrieving revision 1.4
diff -u -r1.4 can_lpc2xxx.c
--- src/can_lpc2xxx.c	17 Aug 2007 08:48:57 -0000	1.4
+++ src/can_lpc2xxx.c	17 Aug 2007 15:25:05 -0000
@@ -348,11 +348,15 @@
 #if CYGINT_IO_CAN_CHANNELS == 1
 #define CAN_CTRL_BASE(_extra_)   CAN_CTRL_SINGLETON_BASE
 #define CAN_ISRVEC(_extra_)      CAN_SINGLETON_ISRVEC
+#define CAN_TXPRIO(_extra_)      CAN_SINGLETON_TXPRIO
+#define CAN_RXPRIO(_extra_)      CAN_SINGLETON_RXPRIO
 #define CAN_DECLARE_INFO(_chan_)
 #define CAN_DECLARE_CHAN(_data_)
 #else
 #define CAN_CTRL_BASE(_extra_)   ((_extra_)->base)
 #define CAN_ISRVEC(_extra_)      ((_extra_)->isrvec)
+#define CAN_TXPRIO(_extra_)      ((_extra_)->txprio)
+#define CAN_RXPRIO(_extra_)      ((_extra_)->rxprio)
 #define CAN_DECLARE_INFO(_chan_) lpc2xxx_can_info_t *info = (lpc2xxx_can_info_t *)chan->dev_priv;
 #define CAN_DECLARE_CHAN(_data_) can_channel  *chan = (can_channel *)data;
 #endif // CYGINT_IO_CAN_CHANNELS == 1 
@@ -425,6 +429,8 @@
 #if CYGINT_IO_CAN_CHANNELS > 1
     cyg_uint32         base;             // Per-bus h/w details
     cyg_uint8          isrvec;           // ISR vector (peripheral id)
+    cyg_uint8          txprio;           // TX ISR priority
+    cyg_uint8          rxprio;           // RX ISR priority
 #endif
 } lpc2xxx_can_info_t;
 
@@ -438,11 +444,13 @@
 //
 #define LPC2XXX_CTRL_NOT_INITIALIZED 0xFF
 #if CYGINT_IO_CAN_CHANNELS > 1
-#define LPC2XXX_CAN_INFO(_l, _base, _isrvec, _flags)                             \
+#define LPC2XXX_CAN_INFO(_l, _base, _isrvec, _txprio, _rxprio, _flags)           \
 lpc2xxx_can_info_t _l  = {                                                       \
     state             :  LPC2XXX_CTRL_NOT_INITIALIZED,                           \
     base              : (_base),                                                 \
     isrvec            : (_isrvec),                                               \
+    txprio            : (_txprio),                                               \
+    rxprio            : (_rxprio),                                               \
     flags             : (_flags),                                                \
     icr               : 0,                                                       \
     LPC2XXX_CAN_INFO_LAST_TX_ID_INIT                                             \
@@ -517,6 +525,8 @@
 LPC2XXX_CAN_INFO(lpc2xxx_can0_info,
                  CAN_CTRL_1_REG_BASE,
                  CYGNUM_HAL_INTERRUPT_CAN1_TX,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN0_TX_PRIO,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN0_RX_PRIO,
                  CAN0_FLAG_STARTUP_ACCFILT_SETUP);
 #endif
 
@@ -524,6 +534,8 @@
 LPC2XXX_CAN_INFO(lpc2xxx_can1_info, 
                  CAN_CTRL_2_REG_BASE, 
                  CYGNUM_HAL_INTERRUPT_CAN2_TX,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN1_TX_PRIO,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN1_RX_PRIO,
                  CAN1_FLAG_STARTUP_ACCFILT_SETUP);
 #endif
 
@@ -531,6 +543,8 @@
 LPC2XXX_CAN_INFO(lpc2xxx_can2_info, 
                  CAN_CTRL_3_REG_BASE, 
                  CYGNUM_HAL_INTERRUPT_CAN3_TX,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN2_TX_PRIO,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN2_RX_PRIO,
                  CAN2_FLAG_STARTUP_ACCFILT_SETUP);
 #endif
 
@@ -538,6 +552,8 @@
 LPC2XXX_CAN_INFO(lpc2xxx_can3_info, 
                  CAN_CTRL_4_REG_BASE, 
                  CYGNUM_HAL_INTERRUPT_CAN4_TX,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN3_TX_PRIO,
+                 CYGNUM_DEVS_CAN_LPC2XXX_CAN3_RX_PRIO,
                  CAN3_FLAG_STARTUP_ACCFILT_SETUP);
 #endif
 #else // CYGINT_IO_CAN_CHANNELS == 1
@@ -545,24 +561,32 @@
 LPC2XXX_CAN_INFO(lpc2xxx_can0_info, CAN0_FLAG_STARTUP_ACCFILT_SETUP);
 #define CAN_CTRL_SINGLETON_BASE   CAN_CTRL_1_REG_BASE
 #define CAN_SINGLETON_ISRVEC      CYGNUM_HAL_INTERRUPT_CAN1_TX
+#define CAN_SINGLETON_TXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN0_TX_PRIO
+#define CAN_SINGLETON_RXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN0_RX_PRIO
 #endif
 
 #ifdef CYGINT_DEVS_CAN_LPC2XXX_CAN1
 LPC2XXX_CAN_INFO(lpc2xxx_can1_info, CAN1_FLAG_STARTUP_ACCFILT_SETUP);
 #define CAN_CTRL_SINGLETON_BASE   CAN_CTRL_2_REG_BASE
 #define CAN_SINGLETON_ISRVEC      CYGNUM_HAL_INTERRUPT_CAN2_TX
+#define CAN_SINGLETON_TXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN1_TX_PRIO
+#define CAN_SINGLETON_RXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN1_RX_PRIO
 #endif
 
 #ifdef CYGINT_DEVS_CAN_LPC2XXX_CAN2
 LPC2XXX_CAN_INFO(lpc2xxx_can2_info, CAN2_FLAG_STARTUP_ACCFILT_SETUP);
 #define CAN_CTRL_SINGLETON_BASE   CAN_CTRL_3_REG_BASE
 #define CAN_SINGLETON_ISRVEC      CYGNUM_HAL_INTERRUPT_CAN3_TX
+#define CAN_SINGLETON_TXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN2_TX_PRIO
+#define CAN_SINGLETON_RXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN2_RX_PRIO
 #endif
 
 #ifdef CYGINT_DEVS_CAN_LPC2XXX_CAN3
 LPC2XXX_CAN_INFO(lpc2xxx_can3_info, CAN3_FLAG_STARTUP_ACCFILT_SETUP);
 #define CAN_CTRL_SINGLETON_BASE   CAN_CTRL_4_REG_BASE
 #define CAN_SINGLETON_ISRVEC      CYGNUM_HAL_INTERRUPT_CAN4_TX
+#define CAN_SINGLETON_TXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN3_TX_PRIO
+#define CAN_SINGLETON_RXPRIO      CYGNUM_DEVS_CAN_LPC2XXX_CAN3_RX_PRIO
 #endif
 
 #endif // #if CYGINT_IO_CAN_CHANNELS > 1
@@ -767,7 +791,7 @@
     // Create TX interrupt
     //
     cyg_drv_interrupt_create(CAN_ISRVEC(info),
-                             0,                        // Priority does not matter LPC2xxx
+                             CAN_TXPRIO(info),
                              (cyg_addrword_t)chan,     // Data item passed to interrupt handler
                              lpc2xxx_can_tx_ISR,
                              lpc2xxx_can_tx_DSR,
@@ -780,7 +804,7 @@
     // Create RX interrupt
     //
     cyg_drv_interrupt_create(CAN_ISRVEC(info) + 6,
-                             0,                        // Priority does not matter for LPC2xxx
+                             CAN_RXPRIO(info),
                              (cyg_addrword_t)chan,     // Data item passed to interrupt handler
                              lpc2xxx_can_rx_ISR,
                              lpc2xxx_can_rx_DSR,
@@ -800,7 +824,7 @@
         // Create err interrupt
         //
         cyg_drv_interrupt_create(CYGNUM_HAL_INTERRUPT_CAN,
-                                 0,                        // Priority does not matter for LPX2xxx
+                                 CYGNUM_DEVS_CAN_LPC2XXX_PRIO,
                                  0,                        // Data item passed to interrupt handler
                                  lpc2xxx_can_err_ISR,
                                  lpc2xxx_can_err_DSR,

[-- Attachment #3: priorities-i2c.diff --]
[-- Type: text/plain, Size: 1972 bytes --]

--- ChangeLog.orig	2007-07-12 15:19:18.000000000 +0200
+++ ChangeLog	2007-08-17 17:26:56.000000000 +0200
@@ -1,3 +1,8 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/i2c_lpc2xxx.cdl, src/i2c_lpc2xxx.c: added option to set
+	  interrupt priority
+
 2007-07-12  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
 
 	* lpc2xxx: driver for on-chip I2C unit
--- cdl/i2c_lpc2xxx.cdl.orig	2007-07-12 15:45:19.000000000 +0200
+++ cdl/i2c_lpc2xxx.cdl	2007-08-17 16:44:46.000000000 +0200
@@ -57,4 +57,16 @@
                  Philips LPC2xxx controllers."
 
     compile     i2c_lpc2xxx.c
+
+    cdl_option CYGNUM_DEVS_I2C_ARM_LPC2XXX_PRIO {
+        display       "interrupt priority for I2C interface"
+        parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+        active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+        flavor        data
+        default_value 3
+        legal_values  0 to 16
+        description   "The interrupt priority corresponds to the vector
+                       number in the Vectored Interrupt Controller. Lower
+                       numbers designate higher priorities."
+    }
 }
\ No newline at end of file
--- src/i2c_lpc2xxx.c.orig	2007-07-12 15:45:03.000000000 +0200
+++ src/i2c_lpc2xxx.c	2007-08-17 15:10:38.000000000 +0200
@@ -71,6 +71,7 @@
 
 #define I2C_XFER        8
 #define I2C_INTR        CYGNUM_HAL_INTERRUPT_I2C
+#define I2C_PRIO        CYGNUM_DEVS_I2C_ARM_LPC2XXX_PRIO
 #define I2C_FREQ        (CYGNUM_HAL_ARM_LPC2XXX_CLOCK_SPEED / CYGNUM_HAL_ARM_LPC2XXX_VPBDIV)
 #define I2C_BASE        CYGARC_HAL_LPC2XXX_REG_I2_BASE
 
@@ -260,7 +261,7 @@
         cyg_drv_mutex_init(&i2c_lock);
         cyg_drv_cond_init(&i2c_wait, &i2c_lock);
         cyg_drv_interrupt_create(
-                I2C_INTR, 0, (cyg_addrword_t) 0, &i2c_lpc2xxx_isr,
+                I2C_INTR, I2C_PRIO, (cyg_addrword_t) 0, &i2c_lpc2xxx_isr,
                 &i2c_lpc2xxx_dsr, &i2c_hand, &i2c_data);
         cyg_drv_interrupt_attach(i2c_hand);
 

[-- Attachment #4: priorities-spi.diff --]
[-- Type: text/plain, Size: 4401 bytes --]

--- ChangeLog.orig	2007-07-12 15:19:33.000000000 +0200
+++ ChangeLog	2007-08-17 17:32:01.000000000 +0200
@@ -1,3 +1,8 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/spi_lpc2xxx.cdl, src/spi_lpc2xxx.cxx: added option to set
+	  interrupt priorities
+
 2007-07-12  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
 
 	* lpc2xxx: driver for on-chip SPI units
--- cdl/spi_lpc2xxx.cdl.orig	2007-07-12 15:48:46.000000000 +0200
+++ cdl/spi_lpc2xxx.cdl	2007-08-17 16:44:40.000000000 +0200
@@ -53,22 +53,46 @@
     parent      CYGPKG_IO_SPI
     active_if   CYGPKG_IO_SPI
 
+    requires    CYGPKG_ERROR
     include_dir cyg/io
     compile     spi_lpc2xxx.cxx
 
-    cdl_option CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS0 {
+    cdl_component CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS0 {
         display       "Enable SPI interface 0"
         flavor        bool
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this option to get support for SPI interface 0."
+
+        cdl_option CYGNUM_DEVS_SPI_ARM_LPC2XXX_BUS0_PRIO {
+            display       "Interrupt priority for SPI interface 0"
+            parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+            active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+            flavor        data
+            default_value 1
+            legal_values  0 to 16
+            description   "The interrupt priority corresponds to the vector
+                           number in the Vectored Interrupt Controller. Lower
+                           numbers designate higher priorities."
+        }
     }
 
-    cdl_option CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS1 {
+    cdl_component CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS1 {
         display       "Enable SPI interface 1"
         flavor        bool
         default_value 1
         description   "The LPC2xxx controllers contain two SPI interfaces.
                        Enable this option to get support for SPI interface 1."
+        cdl_option CYGNUM_DEVS_SPI_ARM_LPC2XXX_BUS1_PRIO {
+            display       "Interrupt priority for SPI interface 1"
+            parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+            active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+            flavor        data
+            default_value 2
+            legal_values  0 to 16
+            description   "The interrupt priority corresponds to the vector
+                           number in the Vectored Interrupt Controller. Lower
+                           numbers designate higher priorities."
+        }
     }
 }
\ No newline at end of file
--- src/spi_lpc2xxx.cxx.orig	2007-07-12 15:49:33.000000000 +0200
+++ src/spi_lpc2xxx.cxx	2007-08-17 15:09:37.000000000 +0200
@@ -303,7 +303,8 @@
  */
 void spi_lpc2xxx_init_bus(cyg_spi_lpc2xxx_bus_t *bus, 
                           cyg_addrword_t dev,
-                          cyg_vector_t vec)
+                          cyg_vector_t vec,
+                          cyg_priority_t prio)
 {
         bus->spi_bus.spi_transaction_begin    = spi_lpc2xxx_begin;
         bus->spi_bus.spi_transaction_transfer = spi_lpc2xxx_transfer;
@@ -319,7 +320,7 @@
         bus->spi_dev = (struct spi_dev *) dev;
         bus->spi_vect = vec;
         cyg_drv_interrupt_create(
-                vec, 0, (cyg_addrword_t) bus,
+                vec, prio, (cyg_addrword_t) bus,
                 &spi_lpc2xxx_isr, &spi_lpc2xxx_dsr,
                 &bus->spi_hand, &bus->spi_intr);
         cyg_drv_interrupt_attach(bus->spi_hand);
@@ -342,7 +343,8 @@
 
                 spi_lpc2xxx_init_bus(&cyg_spi_lpc2xxx_bus0,
                                      CYGARC_HAL_LPC2XXX_REG_SPI0_BASE,
-                                     CYGNUM_HAL_INTERRUPT_SPI0);
+                                     CYGNUM_HAL_INTERRUPT_SPI0,
+                                     CYGNUM_DEVS_SPI_ARM_LPC2XXX_BUS0_PRIO);
 #endif
 #ifdef CYGPKG_DEVS_SPI_ARM_LPC2XXX_BUS1
                 addr = CYGARC_HAL_LPC2XXX_REG_PIN_BASE
@@ -352,7 +354,8 @@
                 HAL_WRITE_UINT32(addr, tmp);
                 spi_lpc2xxx_init_bus(&cyg_spi_lpc2xxx_bus1,
                                      CYGARC_HAL_LPC2XXX_REG_SPI1_BASE,
-                                     CYGNUM_HAL_INTERRUPT_SPI1);
+                                     CYGNUM_HAL_INTERRUPT_SPI1,
+                                     CYGNUM_DEVS_SPI_ARM_LPC2XXX_BUS1_PRIO);
 #endif
         }
 };

[-- Attachment #5: priorities-ser.diff --]
[-- Type: text/plain, Size: 5346 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/serial/generic/16x5x/current/ChangeLog,v
retrieving revision 1.16
diff -u -r1.16 ChangeLog
--- ChangeLog	22 Jun 2007 11:41:49 -0000	1.16
+++ ChangeLog	17 Aug 2007 17:00:03 -0000
@@ -1,3 +1,9 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* src/ser_16x5x.c: Added int_prio to pc_serial_info to enable
+	setting of interrupt priority. If unset the default priority is
+	used as before.
+
 2007-06-22  Alexander Aganichev  <aaganichev@gmail.com>
 
 	* cdl/ser_generic_16x5x.cdl
Index: src/ser_16x5x.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/serial/generic/16x5x/current/src/ser_16x5x.c,v
retrieving revision 1.15
diff -u -r1.15 ser_16x5x.c
--- src/ser_16x5x.c	22 Jun 2007 11:41:49 -0000	1.15
+++ src/ser_16x5x.c	17 Aug 2007 17:00:03 -0000
@@ -182,6 +182,7 @@
 typedef struct pc_serial_info {
     cyg_addrword_t base;
     int            int_num;
+    int            int_prio;
     cyg_interrupt  serial_interrupt;
     cyg_handle_t   serial_interrupt_handle;
 #ifdef CYGPKG_IO_SERIAL_GENERIC_16X5X_FIFO
@@ -375,6 +376,7 @@
 
     if (chan->out_cbuf.len != 0) {
         cyg_drv_interrupt_create(ser_chan->int_num,
+                                 ser_chan->int_prio > 0 ? ser_chan->int_prio :
                                  CYG_IO_SERIAL_GENERIC_16X5X_INT_PRIORITY,
                                  (cyg_addrword_t)chan,
                                  pc_serial_ISR,
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/serial/arm/lpc2xxx/current/ChangeLog,v
retrieving revision 1.3
diff -u -r1.3 ChangeLog
--- ChangeLog	22 Jun 2007 11:41:49 -0000	1.3
+++ ChangeLog	17 Aug 2007 17:02:26 -0000
@@ -1,3 +1,9 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/ser_arm_lpc2xxx.cdl, include/arm_lpc2xxx_ser.inl: Added
+	option to set interrupt priorities. Initialize int_prio field of
+	pc_serial_info.
+
 2007-06-22  Alexander Aganichev <aaganichev@gmail.com>
 
 	* cdl/ser_arm_lpc2xxx.cdl:
Index: cdl/ser_arm_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/serial/arm/lpc2xxx/current/cdl/ser_arm_lpc2xxx.cdl,v
retrieving revision 1.2
diff -u -r1.2 ser_arm_lpc2xxx.cdl
--- cdl/ser_arm_lpc2xxx.cdl	22 Jun 2007 11:41:49 -0000	1.2
+++ cdl/ser_arm_lpc2xxx.cdl	17 Aug 2007 17:02:26 -0000
@@ -91,6 +91,18 @@
             This option includes the serial device driver for the ARM
             LPC2XXX port 0."
 
+        cdl_option CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL0_PRIO {
+            display       "interrupt priority for serial port 0"
+            parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+            active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+            flavor        data
+            default_value 4
+            legal_values  0 to 16
+            description   "The interrupt priority corresponds to the vector
+                           number in the Vectored Interrupt Controller. Lower
+                           numbers designate higher priorities."
+        }
+
         cdl_option CYGDAT_IO_SERIAL_ARM_LPC2XXX_SERIAL0_NAME {
             display       "Device name for ARM LPC2XXX serial port 0 driver"
             flavor        data
@@ -136,6 +148,18 @@
             This option includes the serial device driver for the ARM
             LPC2XXX port 1."
 
+        cdl_option CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL1_PRIO {
+            display       "interrupt priority for serial port 1"
+            parent        CYGHWR_HAL_ARM_LPC2XXX_VIC
+            active_if     CYGHWR_HAL_ARM_LPC2XXX_VIC
+            flavor        data
+            default_value 5
+            legal_values  0 to 16
+            description   "The interrupt priority corresponds to the vector
+                           number in the Vectored Interrupt Controller. Lower
+                           numbers designate higher priorities."
+        }
+
         cdl_option CYGDAT_IO_SERIAL_ARM_LPC2XXX_SERIAL1_NAME {
             display       "Device name for ARM LPC2XXX serial port 1 driver"
             flavor        data
Index: include/arm_lpc2xxx_ser.inl
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/serial/arm/lpc2xxx/current/include/arm_lpc2xxx_ser.inl,v
retrieving revision 1.2
diff -u -r1.2 arm_lpc2xxx_ser.inl
--- include/arm_lpc2xxx_ser.inl	15 Nov 2004 09:20:27 -0000	1.2
+++ include/arm_lpc2xxx_ser.inl	17 Aug 2007 17:02:27 -0000
@@ -87,7 +87,8 @@
 #ifdef CYGPKG_IO_SERIAL_ARM_LPC2XXX_SERIAL0
 static pc_serial_info lpc2xxx_serial_info0 = 
   { CYGARC_HAL_LPC2XXX_REG_UART0_BASE,
-    CYGNUM_HAL_INTERRUPT_UART0
+    CYGNUM_HAL_INTERRUPT_UART0,
+    CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL0_PRIO
   };
 
 #if CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL0_BUFSIZE > 0
@@ -135,7 +136,8 @@
 #ifdef CYGPKG_IO_SERIAL_ARM_LPC2XXX_SERIAL1
 static pc_serial_info lpc2xxx_serial_info1 = 
   { CYGARC_HAL_LPC2XXX_REG_UART1_BASE,
-    CYGNUM_HAL_INTERRUPT_UART1
+    CYGNUM_HAL_INTERRUPT_UART1,
+    CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL1_PRIO
   };
 #if CYGNUM_IO_SERIAL_ARM_LPC2XXX_SERIAL1_BUFSIZE > 0
 static unsigned char 

[-- Attachment #6: priorities-eth.diff --]
[-- Type: text/plain, Size: 1986 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/ns/dp83902a/current/ChangeLog,v
retrieving revision 1.11
diff -u -r1.11 ChangeLog
--- ChangeLog	12 Aug 2004 13:01:17 -0000	1.11
+++ ChangeLog	17 Aug 2007 16:57:33 -0000
@@ -1,3 +1,8 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* src/if_dp83902a.c, include/dp83902a.h: HW-specific code can
+	define CYGNUM_DEVS_ETH_NS_DP83902A_PRIO to set interrupt priority.
+
 2004-08-12  Jani Monoses <jani@iv.ro>
 
 	* src/if_dp83902a.c: Fix builing with lwip.
Index: include/dp83902a.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/ns/dp83902a/current/include/dp83902a.h,v
retrieving revision 1.5
diff -u -r1.5 dp83902a.h
--- include/dp83902a.h	23 May 2002 23:00:46 -0000	1.5
+++ include/dp83902a.h	17 Aug 2007 16:57:34 -0000
@@ -185,6 +185,10 @@
 // ------------------------------------------------------------------------
 // Macros allowing platform to customize some of the driver details
 
+#ifndef CYGNUM_DEVS_ETH_NS_DP83902A_PRIO
+# define CYGNUM_DEVS_ETH_NS_DP83902A_PRIO 0
+#endif
+
 #ifndef CYGHWR_NS_DP83902A_PLF_RESET
 # define CYGHWR_NS_DP83902A_PLF_RESET(_b_) do { } while (0)
 #endif
Index: src/if_dp83902a.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/ns/dp83902a/current/src/if_dp83902a.c,v
retrieving revision 1.10
diff -u -r1.10 if_dp83902a.c
--- src/if_dp83902a.c	12 Aug 2004 13:01:17 -0000	1.10
+++ src/if_dp83902a.c	17 Aug 2007 16:57:34 -0000
@@ -178,7 +178,7 @@
 #ifdef CYGINT_IO_ETH_INT_SUPPORT_REQUIRED
     cyg_drv_interrupt_create(
         dp->interrupt,
-        0,                  // Priority - unused
+        CYGNUM_DEVS_ETH_NS_DP83902A_PRIO,
         (cyg_addrword_t)dp,// Data item passed to ISR & DSR
         dp83902a_isr,          // ISR
         dp83902a_dsr,          // DSR

[-- Attachment #7: priorities-hal.diff --]
[-- Type: text/plain, Size: 3061 bytes --]

Index: hal_arm_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl,v
retrieving revision 1.4
diff -u -r1.4 hal_arm_lpc2xxx.cdl
--- hal_arm_lpc2xxx.cdl	30 Jul 2007 18:09:47 -0000	1.4
+++ hal_arm_lpc2xxx.cdl	17 Aug 2007 15:40:10 -0000
@@ -157,6 +157,29 @@
             same as the processor clock."
     }
 
+    cdl_component CYGHWR_HAL_ARM_LPC2XXX_VIC {
+        display       "Vectored Interrupt Controller"
+        flavor        bool
+        calculated    1
+        description   "
+            This option enables or disables the Vectored Interrupt Controller.
+            The LPC2xxx eCos HAL supports up to 17 interrupt levels.
+            Interrupt levels 0 - 15 are vectored IRQs. Vectored IRQs 
+            have a higher priority then non vectored IRQs and they 
+            are processed faster. Non vectored IRQs are all chained together 
+            into one single slot and the ISR need to find out which interrupt 
+            occured."
+    
+        cdl_option CYGNUM_HAL_KERNEL_COUNTERS_CLOCK_ISR_DEFAULT_PRIORITY {
+	    display		"Default priority for system clock interrupts"
+	    flavor		data
+	    legal_values  { 0 to 16 }
+            default_value 0
+            description "The default value for the system clock interrupts is 0 - 
+                         this is the highest priority IRQ."
+        }
+    }
+
     cdl_component CYGNUM_HAL_RTC_CONSTANTS {
         display       "Real-time clock constants"
         flavor        none
@@ -206,20 +229,4 @@
            debugging via JTAG, as stopping the clock can prevent the
            debugger getting control of the system."
     }
-    
-    cdl_option CYGNUM_HAL_KERNEL_COUNTERS_CLOCK_ISR_DEFAULT_PRIORITY {
-	    display		"Default priority for system clock interrupts"
-	    flavor		data
-	    legal_values  { 0 to 16 }
-        default_value 0
-	    description "
-            The LPC2xxx eCos HAL supports up to 17 interrupt levels.
-            Interrupt levels 0 - 15 are vectored IRQs. Vectored IRQs 
-            have a higher priority then non vectored IRQs and they 
-            are processed faster. Non vectored  IRQs are all chained together 
-            into one single slot and the ISR need to  find out which interrupt 
-            occured. The default value for the system clock interrupts is 0 - 
-            this is the highest priority IRQ."
-    }
-
 }
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog,v
retrieving revision 1.7
diff -u -r1.7 ChangeLog
--- ChangeLog	30 Jul 2007 18:09:47 -0000	1.7
+++ ChangeLog	17 Aug 2007 15:58:09 -0000
@@ -1,3 +1,8 @@
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/hal_arm_lpc2xxx.cdl: added VIC component to support
+	configuration of individual interrupt priorities
+
 2007_07-10  Uwe Kindler <uwe_kindler@web.de>
 
 	* cdl/hal_arm_lpc2xxx.cdl: Added option

Re: LPC2xxx patch for support of vectored interrupt controller

[cetoni GmbH - Uwe Kindler]

Hello Hans,

I have to admit that I did not care about drivers that use interrupt 
priority 0 when I created the support for VIC controller in LPC2xxx 
variant. In current VIC support I decided to use 16 as the priority of 
the interrupts that do not use VIC - this is the formerly behaviour of 
the LPC2xxx interrupt support and of interrupts that use priority 0.

Another solution for this problem would be to invert interrupt 
priorities. That means interrupt priority 0 is the lowest interrupt 
priority and interrupt 16 is the highest priority. So if the drivers use 
interrupt level 0 they would fall back to the old (slower) interrupt 
handling. But that would not fix the test case that creates 2 interrupts 
of level 1.

I think your solution is cleaner - thank you for creating this patch

Best Regards,

Uwe


Dipl. Inf. (FH)
Uwe Kindler
Software Engineering

--

cetoni GmbH
Am Wiesenring 6
D-07554 Korbussen

Tel.: +49 (0) 36602 338 28
Fax:  +49 (0) 36602 338 11
uwe.kindler@cetoni.de
http://www.cetoni.de

Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

On Fri, Aug 17, 2007 at 07:13:23PM +0200, Hans Rosenfeld wrote:
> On Fri, Aug 17, 2007 at 04:02:09PM +0200, Hans Rosenfeld wrote:
> > A less simple solution would be to fix the drivers and the kintr0 test
> > to use configurable priorities. If it wouldn't involve kintr0 I would
> > just do that.

I did some minor changes intr0 and kintr0 to use a configurable
interrupt priority, which should be set to 16 for LPC2xxx systems.

There is another change required to the LPC2xxx HAL. The above tests
use hal_interrupt_configure() on some random interrupt vector, but this
function contains a assertion to make sure that only external interrupts
are configures. 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.


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

[-- Attachment #2: priorities-kernel.diff --]
[-- Type: text/plain, Size: 4969 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.143
diff -u -r1.143 ChangeLog
--- ChangeLog	2 Jul 2007 11:49:09 -0000	1.143
+++ ChangeLog	20 Aug 2007 14:54:14 -0000
@@ -1,3 +1,8 @@
+2007-08-20  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/kernel.cdl, tests/intr0.cxx, tests/kintr0.cxx: Added option
+	to set the interrupt priority used by the intr0 and kintr0 tests.
+
 2007-07-02  Gary Thomas  <gary@mlbassoc.com>
 
 	* src/debug/dbg_gdb.cxx: 
Index: cdl/kernel.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/cdl/kernel.cdl,v
retrieving revision 1.21
diff -u -r1.21 kernel.cdl
--- cdl/kernel.cdl	8 Jan 2007 16:20:13 -0000	1.21
+++ cdl/kernel.cdl	20 Aug 2007 14:54:15 -0000
@@ -317,7 +317,7 @@
                 the set of global flags if present."
         }
 
-        cdl_option CYGPKG_KERNEL_TESTS {
+        cdl_component CYGPKG_KERNEL_TESTS {
             display "Kernel tests"
             flavor  data
             no_define
@@ -330,6 +330,16 @@
             }
             description   "
                 This option specifies the set of tests for the eCos kernel."
+
+            cdl_option CYGNUM_KERNEL_INTR_TEST_PRIO {
+                display       "interrupt priority used by intr0/kintr0 test"
+                flavor        booldata
+                default_value 0
+                legal_values  0 to 16
+                description   "The intr0 and kintr0 tests create several interrupts.
+                               This option selects the interrupt priority to be used
+                               for these interrupts."
+            }
         }
     }
 }
Index: tests/intr0.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/intr0.cxx,v
retrieving revision 1.13
diff -u -r1.13 intr0.cxx
--- tests/intr0.cxx	11 Aug 2006 09:29:31 -0000	1.13
+++ tests/intr0.cxx	20 Aug 2007 14:54:15 -0000
@@ -60,6 +60,14 @@
 
 #include "testaux.hxx"
 
+#ifdef CYGNUM_KERNEL_INTR_TEST_PRIO
+#define PRIO_0 CYGNUM_KERNEL_INTR_TEST_PRIO
+#define PRIO_1 CYGNUM_KERNEL_INTR_TEST_PRIO
+#else
+#define PRIO_0 0
+#define PRIO_1 1
+#endif
+
 static cyg_ISR isr0, isr1;
 static cyg_DSR dsr0, dsr1;
 
@@ -97,7 +105,7 @@
 
 static bool flash( void )
 {
-    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN, 0, (CYG_ADDRWORD)333, isr0, dsr0 );
+    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN, PRIO_0, (CYG_ADDRWORD)333, isr0, dsr0 );
 
     return true;
 }
@@ -134,13 +142,13 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     Cyg_Interrupt* intr0 = NULL;
     if (!in_use)
-        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, 1, (CYG_ADDRWORD)777, isr0, dsr0 );
+        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, PRIO_1, (CYG_ADDRWORD)777, isr0, dsr0 );
      
     cyg_vector lvl2 = CYGNUM_HAL_ISR_MIN + ( 15 % CYGNUM_HAL_ISR_COUNT);
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     Cyg_Interrupt* intr1 = NULL;
     if (!in_use && lvl1 != lvl2)
-        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, 1, 888, isr1, dsr1 );
+        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, PRIO_1, 888, isr1, dsr1 );
 
     // Check these functions at least exist
     Cyg_Interrupt::disable_interrupts();
Index: tests/kintr0.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/kintr0.c,v
retrieving revision 1.12
diff -u -r1.12 kintr0.c
--- tests/kintr0.c	11 Aug 2006 09:29:31 -0000	1.12
+++ tests/kintr0.c	20 Aug 2007 14:54:15 -0000
@@ -61,6 +61,14 @@
 
 #include "testaux.h"
 
+#ifdef CYGNUM_KERNEL_INTR_TEST_PRIO
+#define PRIO_0 CYGNUM_KERNEL_INTR_TEST_PRIO
+#define PRIO_1 CYGNUM_KERNEL_INTR_TEST_PRIO
+#else
+#define PRIO_0 0
+#define PRIO_1 1
+#endif
+
 static cyg_interrupt intr_obj[2];
 
 static cyg_handle_t intr0, intr1;
@@ -103,7 +111,7 @@
     cyg_handle_t handle;
     cyg_interrupt intr;
 
-    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, 0, (cyg_addrword_t)333, 
+    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, PRIO_0, (cyg_addrword_t)333, 
                          isr0, dsr0, &handle, &intr );
     cyg_interrupt_delete(handle);
 
@@ -156,13 +164,13 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     intr0 = 0;
     if (!in_use)
-        cyg_interrupt_create(lvl1, 1, (cyg_addrword_t)777, isr0, dsr0, 
+        cyg_interrupt_create(lvl1, PRIO_1, (cyg_addrword_t)777, isr0, dsr0, 
                              &intr0, &intr_obj[0]);
     
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     intr1 = 0;
     if (!in_use && lvl1 != lvl2)
-        cyg_interrupt_create(lvl2, 1, 888, isr1, dsr1, &intr1, &intr_obj[1]);
+        cyg_interrupt_create(lvl2, PRIO_1, 888, isr1, dsr1, &intr1, &intr_obj[1]);
 
     // Check these functions at least exist
 

[-- Attachment #3: hal_interrupt_configure.diff --]
[-- Type: text/plain, Size: 787 bytes --]

Index: lpc2xxx_misc.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/src/lpc2xxx_misc.c,v
retrieving revision 1.5
diff -u -r1.5 lpc2xxx_misc.c
--- lpc2xxx_misc.c	30 Jul 2007 18:09:47 -0000	1.5
+++ lpc2xxx_misc.c	20 Aug 2007 14:46:32 -0000
@@ -355,8 +355,9 @@
     cyg_uint32 regval, saved_vpbdiv;
 
     // Only external interrupts are configurable	
-    CYG_ASSERT(vector <= CYGNUM_HAL_INTERRUPT_EINT3 &&
-               vector >= CYGNUM_HAL_INTERRUPT_EINT0 , "Invalid vector");
+    if(vector < CYGNUM_HAL_INTERRUPT_EINT0 ||
+       vector > CYGNUM_HAL_INTERRUPT_EINT3)
+            return;
 
     // Map int vector to corresponding bit (0..3)
     vector = 1 << (vector - CYGNUM_HAL_INTERRUPT_EINT0);

Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn]

> +#ifdef CYGNUM_KERNEL_INTR_TEST_PRIO
> +#define PRIO_0 CYGNUM_KERNEL_INTR_TEST_PRIO
> +#define PRIO_1 CYGNUM_KERNEL_INTR_TEST_PRIO
> +#else
> +#define PRIO_0 0
> +#define PRIO_1 1
> +#endif

Hi Hans

I've not spent time to really understand this change. But a first
glance suggests this is wrong. Shouldn't it be

#define PRIO_1 (CYGNUM_KERNEL_INTR_TEST_PRIO + 1)

in order to maintain the old behaviour. 

   Andrew

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

On Wed, Aug 22, 2007 at 01:12:16PM +0200, Andrew Lunn wrote:
> Could your produce a patch which contains all these changes and a
> ChangeLog entry in both the Kernel package and the HAL.

Done.


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

[-- Attachment #2: priorities-kernel.diff --]
[-- Type: text/plain, Size: 5374 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog,v
retrieving revision 1.7
diff -u -r1.7 ChangeLog
--- ChangeLog	30 Jul 2007 18:09:47 -0000	1.7
+++ ChangeLog	22 Aug 2007 13:34:11 -0000
@@ -1,3 +1,13 @@
+2007-08-22  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/hal_arm_lpc2xxx.cdl: require interrupt priority 16 for
+	kernel test intr0/kintr0 interrupts
+
 2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
 
 	* cdl/hal_arm_lpc2xxx.cdl: added VIC component to support
 	configuration of individual interrupt priorities
 
 2007_07-10  Uwe Kindler <uwe_kindler@web.de>
 
 	* cdl/hal_arm_lpc2xxx.cdl: Added option
Index: hal_arm_lpc2xxx.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/cdl/hal_arm_lpc2xxx.cdl,v
retrieving revision 1.4
diff -u -r1.4 hal_arm_lpc2xxx.cdl
--- hal_arm_lpc2xxx.cdl	30 Jul 2007 18:09:47 -0000	1.4
+++ hal_arm_lpc2xxx.cdl	22 Aug 2007 13:37:42 -0000
@@ -66,6 +66,15 @@
     implements    CYGINT_HAL_ARM_ARCH_ARM7
     implements    CYGINT_HAL_ARM_THUMB_ARCH
 
+    requires { is_active(CYGNUM_KERNEL_INTR_TEST_PRIO_A)
+               implies CYGNUM_KERNEL_INTR_TEST_PRIO_A == 16 }
+    
+    requires { is_active(CYGNUM_KERNEL_INTR_TEST_PRIO_B)
+               implies CYGNUM_KERNEL_INTR_TEST_PRIO_B == 16 }
+
+    requires { is_active(CYGNUM_KERNEL_INTR_TEST_PRIO_C)
+               implies CYGNUM_KERNEL_INTR_TEST_PRIO_C == 16 }
+
     # Let the architectural HAL see this variant's files
     define_proc {
         puts $::cdl_header "#define CYGBLD_HAL_VAR_INTS_H <cyg/hal/hal_var_ints.h>"
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.143
diff -u -r1.143 ChangeLog
--- ChangeLog	2 Jul 2007 11:49:09 -0000	1.143
+++ ChangeLog	22 Aug 2007 13:39:40 -0000
@@ -1,3 +1,8 @@
+2007-08-22  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/kernel.cdl, tests/intr0.cxx, tests/kintr0.c: added options
+	to configure priorities of the interrupts created by intr0 and kintr0
+
 2007-07-02  Gary Thomas  <gary@mlbassoc.com>
 
 	* src/debug/dbg_gdb.cxx: 
Index: intr0.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/intr0.cxx,v
retrieving revision 1.13
diff -u -r1.13 intr0.cxx
--- intr0.cxx	11 Aug 2006 09:29:31 -0000	1.13
+++ intr0.cxx	22 Aug 2007 13:40:08 -0000
@@ -97,7 +97,9 @@
 
 static bool flash( void )
 {
-    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN, 0, (CYG_ADDRWORD)333, isr0, dsr0 );
+    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN,
+                                        CYGNUM_KERNEL_INTR_TEST_PRIO_A,
+                                        (CYG_ADDRWORD)333, isr0, dsr0 );
 
     return true;
 }
@@ -134,13 +136,15 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     Cyg_Interrupt* intr0 = NULL;
     if (!in_use)
-        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, 1, (CYG_ADDRWORD)777, isr0, dsr0 );
+        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, CYGNUM_KERNEL_INTR_TEST_PRIO_B,
+                                                          (CYG_ADDRWORD)777, isr0, dsr0 );
      
     cyg_vector lvl2 = CYGNUM_HAL_ISR_MIN + ( 15 % CYGNUM_HAL_ISR_COUNT);
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     Cyg_Interrupt* intr1 = NULL;
     if (!in_use && lvl1 != lvl2)
-        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, 1, 888, isr1, dsr1 );
+        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, CYGNUM_KERNEL_INTR_TEST_PRIO_C,
+                                                          888, isr1, dsr1 );
 
     // Check these functions at least exist
     Cyg_Interrupt::disable_interrupts();
Index: kintr0.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/kintr0.c,v
retrieving revision 1.12
diff -u -r1.12 kintr0.c
--- kintr0.c	11 Aug 2006 09:29:31 -0000	1.12
+++ kintr0.c	22 Aug 2007 13:40:44 -0000
@@ -103,8 +103,8 @@
     cyg_handle_t handle;
     cyg_interrupt intr;
 
-    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, 0, (cyg_addrword_t)333, 
-                         isr0, dsr0, &handle, &intr );
+    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, CYGNUM_KERNEL_INTR_TEST_PRIO_A,
+                         (cyg_addrword_t)333, isr0, dsr0, &handle, &intr );
     cyg_interrupt_delete(handle);
 
     return true;
@@ -156,13 +156,14 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     intr0 = 0;
     if (!in_use)
-        cyg_interrupt_create(lvl1, 1, (cyg_addrword_t)777, isr0, dsr0, 
-                             &intr0, &intr_obj[0]);
+        cyg_interrupt_create(lvl1, CYGNUM_KERNEL_INTR_TEST_PRIO_B, 
+                             (cyg_addrword_t)777, isr0, dsr0, &intr0, &intr_obj[0]);
     
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     intr1 = 0;
     if (!in_use && lvl1 != lvl2)
-        cyg_interrupt_create(lvl2, 1, 888, isr1, dsr1, &intr1, &intr_obj[1]);
+        cyg_interrupt_create(lvl2, CYGNUM_KERNEL_INTR_TEST_PRIO_C,
+                             888, isr1, dsr1, &intr1, &intr_obj[1]);
 
     // Check these functions at least exist
 

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

On Wed, Aug 22, 2007 at 05:15:24PM +0200, Andrew Lunn wrote:
> However, you forgot the diff for the kernel cdl file.

Oops. Here it is.


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

[-- Attachment #2: priorities-kernel-cdl.diff --]
[-- Type: text/plain, Size: 2189 bytes --]

Index: kernel.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/cdl/kernel.cdl,v
retrieving revision 1.21
diff -u -r1.21 kernel.cdl
--- kernel.cdl	8 Jan 2007 16:20:13 -0000	1.21
+++ kernel.cdl	22 Aug 2007 17:54:55 -0000
@@ -317,7 +317,7 @@
                 the set of global flags if present."
         }
 
-        cdl_option CYGPKG_KERNEL_TESTS {
+        cdl_component CYGPKG_KERNEL_TESTS {
             display "Kernel tests"
             flavor  data
             no_define
@@ -330,6 +330,35 @@
             }
             description   "
                 This option specifies the set of tests for the eCos kernel."
+            cdl_option CYGNUM_KERNEL_INTR_TEST_PRIO_A {
+                display       "interrupt priority A used by intr0/kintr0 test"
+                flavor        data
+                default_value 0
+                legal_values  0 to 16
+                description   "The intr0 and kintr0 tests create several interrupts.
+                               This option selects the interrupt priority to be used
+                               for the first interrupt."
+            }
+
+            cdl_option CYGNUM_KERNEL_INTR_TEST_PRIO_B {
+                display       "interrupt priority B used by intr0/kintr0 test"
+                flavor        data
+                default_value 1
+                legal_values  0 to 16
+                description   "The intr0 and kintr0 tests create several interrupts.
+                               This option selects the interrupt priority to be used
+                               for the second interrupt."
+            }
+
+            cdl_option CYGNUM_KERNEL_INTR_TEST_PRIO_C {
+                display       "interrupt priority C used by intr0/kintr0 test"
+                flavor        data
+                default_value 1
+                legal_values  0 to 16
+                description   "The intr0 and kintr0 tests create several interrupts.
+                               This option selects the interrupt priority to be used
+                               for the third interrupt."
+            }
         }
     }
 }

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Bart Veer]

Hang on a sec - turning these interrupt priorities into CDL options
seems overkill. It would be simpler for the interrupt tests to contain
something like:

#ifdef HAL_INTR_TEST_PRIO_A
# define PRIO_A	HAL_INTR_TEST_PRIO_A
#else
# define PRIO_A 1
#endif
#ifdef HAL_INTR_TEST_PRIO_B
# define PRIO_B HAL_INTR_TEST_PRIO_B
#else
# define PRIO_B 1
#endif

  cyg_interrupt_create(lvl1, PRIO_A, ...);
  cyg_interrupt_create(lvl1, PRIO_B, ...);

and for the targets of interest arrange for HAL_INTR_TEST_PRIO_A and
HAL_INTR_TEST_PRIO_B to be defined when #include'ing
<cyg/hal/hal_intr.h>, e.g. via var_intr.h or proc_intr.h

Allowing HALs to override defaults in testcases sometimes makes sense.
However I cannot think of any circumstances where end users would want
to mess about with these settings, so exposing them in the config tool
would be inappropriate.

Bart

-- 
Bart Veer                                   eCos Configuration Architect
eCosCentric Limited    The eCos experts      http://www.ecoscentric.com/
Barnwell House, Barnwell Drive, Cambridge, UK.      Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

On Wed, Aug 22, 2007 at 07:30:54PM +0100, Bart Veer wrote:
> Allowing HALs to override defaults in testcases sometimes makes sense.
> However I cannot think of any circumstances where end users would want
> to mess about with these settings, so exposing them in the config tool
> would be inappropriate.

Makes sense, so here is yet another patch.


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

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

On Thu, Aug 23, 2007 at 12:56:11PM +0200, Hans Rosenfeld wrote:
> Makes sense, so here is yet another patch.

That is, it would be there if I hadn't forgotten to attach it.


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

[-- Attachment #2: priorities-kernel.diff --]
[-- Type: text/plain, Size: 5643 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.143
diff -u -r1.143 ChangeLog
--- ChangeLog	2 Jul 2007 11:49:09 -0000	1.143
+++ ChangeLog	23 Aug 2007 09:24:54 -0000
@@ -1,3 +1,9 @@
+2007-08-23  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* tests/intr0.cxx, tests/kintr0.c: As suggested by Bart Veer,
+	priorities of the interrupts created by intr0 and kintr0 can now
+	be overridden by the HAL through HAL_INTR_TEST_PRIO_x constants.
+
 2007-07-02  Gary Thomas  <gary@mlbassoc.com>
 
 	* src/debug/dbg_gdb.cxx: 
Index: tests/intr0.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/intr0.cxx,v
retrieving revision 1.13
diff -u -r1.13 intr0.cxx
--- tests/intr0.cxx	11 Aug 2006 09:29:31 -0000	1.13
+++ tests/intr0.cxx	23 Aug 2007 09:24:55 -0000
@@ -60,6 +60,24 @@
 
 #include "testaux.hxx"
 
+#ifdef HAL_INTR_TEST_PRIO_A
+# define PRIO_A HAL_INTR_TEST_PRIO_A
+#else
+# define PRIO_A 0
+#endif
+
+#ifdef HAL_INTR_TEST_PRIO_B
+# define PRIO_B HAL_INTR_TEST_PRIO_B
+#else
+# define PRIO_B 1
+#endif
+
+#ifdef HAL_INTR_TEST_PRIO_C
+# define PRIO_C HAL_INTR_TEST_PRIO_C
+#else
+# define PRIO_C 1
+#endif
+
 static cyg_ISR isr0, isr1;
 static cyg_DSR dsr0, dsr1;
 
@@ -97,7 +115,8 @@
 
 static bool flash( void )
 {
-    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN, 0, (CYG_ADDRWORD)333, isr0, dsr0 );
+    Cyg_Interrupt intr0 = Cyg_Interrupt(CYGNUM_HAL_ISR_MIN, PRIO_A,
+                                        (CYG_ADDRWORD)333, isr0, dsr0 );
 
     return true;
 }
@@ -134,13 +153,13 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     Cyg_Interrupt* intr0 = NULL;
     if (!in_use)
-        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, 1, (CYG_ADDRWORD)777, isr0, dsr0 );
+        intr0 = new((void *)&intr0_obj[0]) Cyg_Interrupt( lvl1, PRIO_B, (CYG_ADDRWORD)777, isr0, dsr0 );
      
     cyg_vector lvl2 = CYGNUM_HAL_ISR_MIN + ( 15 % CYGNUM_HAL_ISR_COUNT);
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     Cyg_Interrupt* intr1 = NULL;
     if (!in_use && lvl1 != lvl2)
-        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, 1, 888, isr1, dsr1 );
+        intr1 = new((void *)&intr1_obj[0]) Cyg_Interrupt( lvl2, PRIO_C, 888, isr1, dsr1 );
 
     // Check these functions at least exist
     Cyg_Interrupt::disable_interrupts();
Index: tests/kintr0.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/kintr0.c,v
retrieving revision 1.12
diff -u -r1.12 kintr0.c
--- tests/kintr0.c	11 Aug 2006 09:29:31 -0000	1.12
+++ tests/kintr0.c	23 Aug 2007 09:24:55 -0000
@@ -61,6 +61,24 @@
 
 #include "testaux.h"
 
+#ifdef HAL_INTR_TEST_PRIO_A
+# define PRIO_A HAL_INTR_TEST_PRIO_A
+#else
+# define PRIO_A 0
+#endif
+
+#ifdef HAL_INTR_TEST_PRIO_B
+# define PRIO_B HAL_INTR_TEST_PRIO_B
+#else
+# define PRIO_B 1
+#endif
+
+#ifdef HAL_INTR_TEST_PRIO_C
+# define PRIO_C HAL_INTR_TEST_PRIO_C
+#else
+# define PRIO_C 1
+#endif
+
 static cyg_interrupt intr_obj[2];
 
 static cyg_handle_t intr0, intr1;
@@ -103,7 +121,7 @@
     cyg_handle_t handle;
     cyg_interrupt intr;
 
-    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, 0, (cyg_addrword_t)333, 
+    cyg_interrupt_create(CYGNUM_HAL_ISR_MIN, PRIO_A, (cyg_addrword_t)333,
                          isr0, dsr0, &handle, &intr );
     cyg_interrupt_delete(handle);
 
@@ -156,13 +174,14 @@
     HAL_INTERRUPT_IN_USE( lvl1, in_use );
     intr0 = 0;
     if (!in_use)
-        cyg_interrupt_create(lvl1, 1, (cyg_addrword_t)777, isr0, dsr0, 
-                             &intr0, &intr_obj[0]);
+        cyg_interrupt_create(lvl1, PRIO_B, (cyg_addrword_t)777,
+                             isr0, dsr0, &intr0, &intr_obj[0]);
     
     HAL_INTERRUPT_IN_USE( lvl2, in_use );
     intr1 = 0;
     if (!in_use && lvl1 != lvl2)
-        cyg_interrupt_create(lvl2, 1, 888, isr1, dsr1, &intr1, &intr_obj[1]);
+        cyg_interrupt_create(lvl2, PRIO_C, 888,
+                             isr1, dsr1, &intr1, &intr_obj[1]);
 
     // Check these functions at least exist
 
Index: include/hal_var_ints.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/include/hal_var_ints.h,v
retrieving revision 1.2
diff -u -r1.2 hal_var_ints.h
--- include/hal_var_ints.h	7 May 2006 18:36:18 -0000	1.2
+++ include/hal_var_ints.h	23 Aug 2007 09:30:49 -0000
@@ -95,6 +95,11 @@
 
 #define CYGNUM_HAL_ISR_COUNT         (CYGNUM_HAL_ISR_MAX+1)
 
+/* use non-vectored interrupts in kernel tests intr0/kintr0 */
+#define HAL_INTR_TEST_PRIO_A 16
+#define HAL_INTR_TEST_PRIO_B 16
+#define HAL_INTR_TEST_PRIO_C 16
+
 //The vector used by the Real time clock
 #define CYGNUM_HAL_INTERRUPT_RTC     CYGNUM_HAL_INTERRUPT_TIMER0
 
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/arm/lpc2xxx/var/current/ChangeLog,v
retrieving revision 1.7
diff -u -r1.7 ChangeLog
--- ChangeLog	30 Jul 2007 18:09:47 -0000	1.7
+++ ChangeLog	23 Aug 2007 09:32:16 -0000
@@ -1,3 +1,13 @@
+2007-08-23  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* include/hal_var_ints.h: use interrupt priority 16 for
+	kernel test intr0/kintr0 interrupts
+
+2007-08-17  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* cdl/hal_arm_lpc2xxx.cdl: added VIC component to support
+	configuration of individual interrupt priorities
+
 2007_07-10  Uwe Kindler <uwe_kindler@web.de>
 
 	* cdl/hal_arm_lpc2xxx.cdl: Added option

Re: [ECOS] Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn]

On Thu, Aug 23, 2007 at 01:02:20PM +0200, Hans Rosenfeld wrote:
> On Thu, Aug 23, 2007 at 12:56:11PM +0200, Hans Rosenfeld wrote:
> > Makes sense, so here is yet another patch.
> 
> That is, it would be there if I hadn't forgotten to attach it.

In future please make the patch relative to the packages
directory. The first half of you patch was relative to kernel/current
and the second have to the LPC2xxx variant hal. So i had to chop the
patch up into two in order to apply it.

Anyway, thanks for the patch.

        Committed,
                Andrew

Re: LPC2xxx patch for support of vectored interrupt controller

[cetoni GmbH - Uwe Kindler]

Hello,

----->
May I suggest that you think about this again?
The only configurable interrupts are EINT0 to EINT3. So this assertion
<----

yes Hans you are right. This was my fault - sorry. But I still would 
leave this assertion in the code.

Regards, Uwe


> 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.
> 
> 

-- 

Mit freundlichen Grüßen,

Dipl. Inf. (FH)
Uwe Kindler
Software Engineering

--

cetoni GmbH
Am Wiesenring 6
D-07554 Korbussen

Tel.: +49 (0) 36602 338 28
Fax:  +49 (0) 36602 338 11
uwe.kindler@cetoni.de
www.cetoni.de

Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn]

> > 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

O.K. Fine. I just asked the question, is this needed. And the answer is now clear. No.

     Thanks
        Andrew

Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

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

Re: LPC2xxx patch for support of vectored interrupt controller

[Andrew Lunn]

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

Re: LPC2xxx patch for support of vectored interrupt controller

[Hans Rosenfeld]

On Tue, Aug 21, 2007 at 08:10:59AM +0200, cetoni GmbH - Uwe Kindler wrote:
> 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.

This function is not used for setting up priorities, but for interrupt
polarity and mode, which is only valid for the external interrupts.
I doubt anyone would even think about changing the polarity of an
internal interrupt or would try to use an internal interrupt for some
external devices by accident, except for code like the test case that
doesn't know and doesn't care. 

But, maybe the code should emit a warning if it is attempted.

> Btw. the assertion I put into the code is wrong:
> 
> CYG_ASSERT(vector <= CYGNUM_HAL_INTERRUPT_EINT3 &&
>            vector >= CYGNUM_HAL_INTERRUPT_EINT0 , "Invalid vector");
> 

I'm pretty sure this is correct.


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

Re: LPC2xxx patch for support of vectored interrupt controller

[cetoni GmbH - Uwe Kindler]

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");

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

Kind regards, Uwe

Mit freundlichen Grüßen,

Dipl. Inf. (FH)
Uwe Kindler
Software Engineering

--

cetoni GmbH
Am Wiesenring 6
D-07554 Korbussen

Tel.: +49 (0) 36602 338 28
Fax:  +49 (0) 36602 338 11
uwe.kindler@cetoni.de
www.cetoni.de