Topics

IRQ_CONNECT: enum based line number?


Erwan Gouriou
 

Hi all,

I've a question around IRQ_CONNECT macro for Cortex_m defined in
include/arch/arm/cortex_m/irq.h

In ST CMSIS, we define IRQ line as enum. For instance:
typedef enum
{
/****** Cortex-M3 Processor Exceptions Numbers
***************************************************/
...
USART1_IRQn = 37, /*!< USART1 global Interrupt
*/
USART2_IRQn = 38, /*!< USART2 global Interrupt
*/
USART3_IRQn = 39, /*!< USART3 global Interrupt
*/
...
} IRQn_Type;

Issue is that we cannot use directly these values in IRQ_CONNECT macro
since it
expects constant IRQ line number.

Note: It compiles without any warning though, since compilation trap does
not catch enums:
enum { IRQ = irq_p }; \ --> irq_p could be an enum

Actual issue is in following directive:
__attribute__ ((section(STRINGIFY(_CONCAT(.gnu.linkonce.isr_irq, irq_p)))))
If irq_p is not a number, compiler will not be able to understand it has to
overwrite
section _isr_irq38.

I've tried numerous ways to cast my enum value into an acceptable const
value for the macro,
but it remained unsuccessful.

Is there any way to cast ny enum into an acceptable const for this macro?
If not, would it be possible to get the macro accepting enum values without
breaking its
benefice?

Thanks
Erwan


Carles Cufi
 

Hi Erwan,

Sorry I can’t quote you but my email client doesn’t allow me to for some reason.

We had the exact same issue with the nRF5x from Nordic, and we resorted to redefining them altogether:
https://github.com/zephyrproject-rtos/zephyr/blob/master/arch/arm/soc/nordic_nrf5/include/nrf5_common.h#L29

That said, we’d be glad to be able to use the original values. I filed a report with the MDK team at Nordic to see if they could switch to #define from enum too, but I am not sure if they’ve implemented it yet.

Regards,

Carles

From: Erwan Gouriou [mailto:erwan.gouriou(a)linaro.org]
Sent: Friday, October 28, 2016 16:27
To: devel(a)lists.zephyrproject.org
Subject: [devel] IRQ_CONNECT: enum based line number?

Hi all,

I've a question around IRQ_CONNECT macro for Cortex_m defined in
include/arch/arm/cortex_m/irq.h

In ST CMSIS, we define IRQ line as enum. For instance:
typedef enum
{
/****** Cortex-M3 Processor Exceptions Numbers ***************************************************/
...
USART1_IRQn = 37, /*!< USART1 global Interrupt */
USART2_IRQn = 38, /*!< USART2 global Interrupt */
USART3_IRQn = 39, /*!< USART3 global Interrupt */
...
} IRQn_Type;

Issue is that we cannot use directly these values in IRQ_CONNECT macro since it
expects constant IRQ line number.

Note: It compiles without any warning though, since compilation trap does not catch enums:
enum { IRQ = irq_p }; \ --> irq_p could be an enum

Actual issue is in following directive:
__attribute__ ((section(STRINGIFY(_CONCAT(.gnu.linkonce.isr_irq, irq_p)))))
If irq_p is not a number, compiler will not be able to understand it has to overwrite
section _isr_irq38.

I've tried numerous ways to cast my enum value into an acceptable const value for the macro,
but it remained unsuccessful.

Is there any way to cast ny enum into an acceptable const for this macro?
If not, would it be possible to get the macro accepting enum values without breaking its
benefice?

Thanks
Erwan


Boie, Andrew P
 

On Fri, 2016-10-28 at 16:27 +0200, Erwan Gouriou wrote:
 
Issue is that we cannot use directly these values in IRQ_CONNECT
macro since it
expects constant IRQ line number.

Note: It compiles without any warning though, since compilation trap
does not catch enums:
enum { IRQ = irq_p }; \  --> irq_p could be an enum

Actual issue is in following directive:
__attribute__ ((section(STRINGIFY(_CONCAT(.gnu.linkonce.isr_irq,
irq_p)))))
If irq_p is not a number, compiler will not be able to understand it
has to overwrite 
section _isr_irq38.
Let me try to explain the preprocessor/linker magic going on which
forces the IRQ line parameter to be a numeric constant, or something
that expands (at preprocessing time) to a numeric constant.

Interrupts are set up statically at build time. Zephyr interrupts
provide a parameter to ISRs which is provided in the IRQ_CONNECT()
invocation. In the build for ARC/ARM/Nios II, we need to set up a
_sw_isr_table array [0...NUM_IRQS] where each element in this array is:

struct _IsrTableEntry {
void *arg;
void (*isr)(void *);
};

What we do in the build is first set up a dummy table (in an assembly
file) with pointers to the spurious IRQ handler. Each element in the
table in in a named section. Let's set NUM_IRQS=16:

_sw_isr_table:
.section .gnu.linkonce.isr_irq0:
{0xABAD1DEA, _irq_spurious}
.section .gnu.linkonce.isr_irq1:
{0xABAD1DEA, _irq_spurious}
.section .gnu.linkonce.isr_irq2:
{0xABAD1DEA, _irq_spurious}
...
.section .gnu.linkonce.isr_irq15:
{0xABAD1DEA, _irq_spurious}

When an IRQ fires, the interrupting IRQ line is looked up in a register
to fetch the correct entry in this table.

This is set up the build to be linked *last*. This is because we are
using .gnu.linkonce mechanism to only use the first occurrence of a
particular section and discard all others. So let's say we want to
connect a handler _my_isr and parameter 0xDEADBEEF to irq 2.
IRQ_CONNECT for Nios II looks like:

#define _ARCH_IRQ_CONNECT(irq_p, priority_p, isr_p, isr_param_p,
flags_p) \
({ \
enum { IRQ = irq_p }; \
static struct _IsrTableEntry _CONCAT(_isr_irq, irq_p) \
__attribute__ ((used))  \
__attribute__
((section(STRINGIFY(_CONCAT(.gnu.linkonce.isr_irq, irq_p))))) = \
{isr_param_p, isr_p}; \
irq_p; \
})

The struct declaration expands to:

static struct _IsrTableEntry _my_isr2
__atttribute__((used))
__attribute__((section(".gnu.linkonce_isr_irq2")) = {
{0xDEADBEEF, _my_isr};

We have overridden the default .gnu.linkonce_isr_irq2 with our desired
handler and parameter, resuling in a table:

_sw_isr_table:
.section .gnu.linkonce.isr_irq0:
{0xABAD1DEA,
_irq_spurious}
.section .gnu.linkonce.isr_irq1:
{0xABAD1DEA,
_irq_spurious}
.section .gnu.linkonce.isr_irq2:
{0xDEADBEEF, _my_isr}

...
.section .gnu.linkonce.isr_irq15:
{0xABAD1DEA, _irq_spurious}

Since the IRQ line parameter is used to create a string that gets passed to __attribute__((section)) it has to be something that the preprocessor can expand to a numeric constant otherwise the overriding mechanism doesn't work.

Hope this helps. Note that x86 has a different mechanism involving assembly language stubs and a special tool to generate the IDT, I need to check if it also forbids enumerations. If using enums is very desirable we could consider revising the ARM/ARC/Nios implementation.

Andrew


Piotr Mienkowski <piotr.mienkowski@...>
 

Hi Erwan,

That said, we’d be glad to be able to use the original values. I filed a report with the
MDK team at Nordic to see if they could switch to #define from enum too, but I am not sure
if they’ve implemented it yet.
I doubt Nordic MDK team would ever accept your proposal. The definition of IRQs as an enum is enforced by the CMSIS standard which comes from ARM. E.g. following is a declaration of one of many NVIC_* functions:
void NVIC_EnableIRQ(IRQn_Type IRQn);
So the vendors have no choice but to use an enum.

Using #define in place of enum does not cause any gcc warnings but it would generate lint warnings and I'm pretty sure it violates Misra-C standard. While this is an acceptable workaround for Zephyr it may not be for other projects out there and the Nordic MDK team has to please us all.

Cheers,
Piotr


Piotr Mienkowski <piotr.mienkowski@...>
 

Hi Andrew,

If using enums is very desirable we could consider revising the ARM/ARC/Nios
implementation.
If this is even possible that would be indeed very desirable! Header files from all vendors that support CMSIS (I believe most of the vendors do) use an enum implementation. It would save a lot of work if we did not have to convert it to #define.

Piotr


Erwan Gouriou
 

Hi Andrew,

Thanks for the detailled explanation.

Hope this helps. Note that x86 has a different mechanism involving
assembly language stubs and a special tool to generate the IDT, I need to
check if it also forbids enumerations. If using enums is very desirable we
could consider revising the ARM/ARC/Nios implementation.

As mentionned by Carles and Piotr, since enum implementation is part of
CMSIS standard and hence should be adopted by other ARM vendors, new
implemmentation le IRQ_CONNECT would be very much welcomed. Should we fill
a JIRA change for this?

Erwan


Carles Cufi
 

Hi Piotr,

-----Original Message-----
From: Piotr Mienkowski [mailto:piotr.mienkowski(a)schmid-telecom.ch]
Sent: Monday, October 31, 2016 11:54
To: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: IRQ_CONNECT: enum based line number?

Hi Erwan,

That said, we’d be glad to be able to use the original values. I filed
a report with the MDK team at Nordic to see if they could switch to
#define from enum too, but I am not sure if they’ve implemented it
yet.

I doubt Nordic MDK team would ever accept your proposal. The definition
of IRQs as an enum is enforced by the CMSIS standard which comes from
ARM. E.g. following is a declaration of one of many NVIC_* functions:
void NVIC_EnableIRQ(IRQn_Type IRQn);
So the vendors have no choice but to use an enum.

Using #define in place of enum does not cause any gcc warnings but it
would generate lint warnings and I'm pretty sure it violates Misra-C
standard. While this is an acceptable workaround for Zephyr it may not
be for other projects out there and the Nordic MDK team has to please us
all.

Yes, I rewrote the issue internally here at Nordic so that it's an addition instead of a replacement, so we don't break anything else.

Carles


Boie, Andrew P
 

On Mon, 2016-10-31 at 11:00 +0000, Piotr Mienkowski wrote:
Hi Andrew,


If using enums is very desirable we could consider revising the ARM/ARC/Nios
implementation.
If this is even possible that would be indeed very desirable! Header files
from all vendors that support CMSIS (I believe most of the vendors do) use an
enum implementation. It would save a lot of work if we did not have to convert
it to #define.
Sure. I have a pretty good idea how to do this and it shouldn't take me very
long. https://jira.zephyrproject.org/browse/ZEP-1165

Andrew