stm32: UART doesn't support IRQ sharing

[ycsin]

I'm trying to use the lpuart1 in my g0b1re custom board and it can't compile when usart5 is already enabled.

gen_isr_tables.py: error: multiple registrations at table_index 29 for irq 29 (0x1d)
Existing handler 0x80547a9, new handler 0x80547a9
Has IRQ_CONNECT or IRQ_DIRECT_CONNECT accidentally been invoked on the same irq multiple times?

I tried to enable usart3 and it also won't compile. It seems like the current STM32 UART driver doesn't support uarts sharing the same interrupt?

From the G0B1RE reference manual:

[ycsin]

@erwango any idea? Is this indeed an unsupported feature?

[erwango]

Indeed, driver can't support this configuration for now.
It would require some modification as proposed here for ADC:
#37585

[ycsin]

Indeed, driver can't support this configuration for now. It would require some modification as proposed here for ADC: #37585

Oh no..
Seems like something screwed up in that PR, can't load the file changes page. Nevermind, sorted that out

[ycsin]

I tried to understand and port what's been done in #37585 to the uart_stm32.c driver but I stumped upon a few problem that I don't know how to solve.

Correct me if I'm wrong, #37585 seems to address the scenario where all ADC instances share the same IRQ line, instead of some ADCs share a IRQ line while some others share the other IRQ line, like the UART:

For the shared IRQ handler to work in the uart driver, we will need static bool init_irq_##DT_INST_IRQN(index) for each of the shared IRQ lines to make sure that we don't initialize them multiple times in uart_stm32_irq_init.

We might be able declare these variable with:

#define STM32_UART_IRQ_IDX_INIT(index) static bool init_irq_##DT_INST_IRQN(index);
DT_INST_FOREACH_STATUS_OKAY(STM32_UART_IRQ_IDX_INIT)

, which will generate a up to 2 static bool init_irq_28 and 5 static bool init_irq_29 in build time but its fine as long as they are not initialized.

But since we have a few IRQ lines, I think we will need one uart_stm32_irq_init for each IRQ line (i.e. uart_stm32_irq_init_28 & uart_stm32_irq_init_29) and here is the problem which I don't know how to solve: it is not okay to declare the same function name multiple times, and I'm not sure how to solve it with compiler magic.

Any idea? @erwango @mcscholtz @mbolivar-nordic

---

To simplify the question

I have these uarts enabled in devicetree (example):

UART	IRQ Line
1	13
2	13
3	14
4	14
5	15

and I'd like to enable interrupt driven APIs, how can I do the IRQ_CONNECT for each of the IRQ line?
Is there anything like "foreach of the irq_line of all of the uarts that are okay"?

[ycsin]

@mbolivar may I please have your input on this, regarding the DT part?

[mbolivar-nordic]

@ycsin I didn't think about it for very long, but it should be possible to do this with some more help from the build system.

Here is a hacky proposal you might be able to use to get started. Maybe you and @erwango can improve the idea from here.

If you check cmake/extensions.cmake, you see we have several extension functions already which work using devicetree nodes:

$ git grep 'function.*dt_' cmake/extensions.cmake

One possible idea is to use CMake to loop over the nodes you care about, and define some macros that tell you which shared IRQ handlers you need to define. That lets you get the global view of the devicetree in CMake.

Here is a rough sketch of how that might work:

1. Add a new devicetree helper to extensions.cmake, named something like dt_get_all_status_okay(out_var compatible). This takes a compatible (like st,stm32-uart) and sets out_var to a list of all the paths of each status "okay" node with that compatible
2. In the CMakeLists.txt for your driver, call dt_get_all_status_okay(uart_nodes "st,stm32-uart")
3. Do something like this pseudocode:

foreach(uart_node ${uart_nodes})
  if(irq_is_shared_for_${uart_node})
    set(irq_num shared_irq_line_for_${uart_node})
    zephyr_library_compile_definitions(STM32_NEED_SHARED_IRQ_${irq_num})
  endif()
endforeach()

The goal is to make sure STM32_NEED_SHARED_IRQ_29 is defined if any of USART3, USART4, etc. are enabled.

4. Then in the driver, do

#ifdef STM32_NEED_SHARED_IRQ_29
/* define IRQ handler here */
#endif 

[ycsin]

@mbolivar-nordic I don't have much experience with CMake apart from the basic one to get my application to compile, but I'm interested to learn and to give it a try if time permits.

I didn't think about it for very long..
Here is a hacky proposal you might be able to use to get started.
...
The goal is to make sure STM32_NEED_SHARED_IRQ_29 is defined if any of USART3, USART4, etc. are enabled.

I think the IRQ lines vary among different STM32 socs, so if this method is used I think we will have a very long irq init function?

static void uart_stm32_irq_init(const struct device *dev)
{
	if (!irq_init) {
...
#ifdef STM32_NEED_SHARED_IRQ_27
		IRQ_CONNECT(27,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(27);
#endif
#ifdef STM32_NEED_SHARED_IRQ_28
		IRQ_CONNECT(28,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(28);
#endif
#ifdef STM32_NEED_SHARED_IRQ_29
		IRQ_CONNECT(29,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(29);
#endif
...
		irq_init = true;
	}
}

Would it be possible to have something like DT_COMPAT_IRQ_NUM_FOREACH_STATUS_OKAY(func, compat) which run a function for each IRQ line of the UART?

#define INIT_IRQ(num)							\
		IRQ_CONNECT(num,					\
			0,						\ ← need a way to get the priority
			uart_stm32_shared_irq_handler, NULL, 0);	\
		irq_enable(num);


static void uart_stm32_irq_init(const struct device *dev)
{
	if (!irq_init) {
		DT_COMPAT_IRQ_NUM_FOREACH_STATUS_OKAY(INIT_IRQ, st_stm32_uart);
		irq_init = true;
	}
}

[mbolivar-nordic]

I think the IRQ lines vary among different STM32 socs, so if this method is used I think we will have a very long irq init function?

Would you have one common IRQ init function like that? Wouldn't it be called again and again for each UART, which would make the IRQ_CONNECT() calls too many times?

Would it be possible to have something like DT_COMPAT_IRQ_NUM_FOREACH_STATUS_OKAY(func, compat) which run a function for each IRQ line of the UART?

I'm having trouble understanding what you're asking about, sorry. It looks like you want to know about every IRQ for a compatible, but that doesn't make sense to me.

Are you looking to have something like DT_FOREACH_IRQ(node_id, fn), which would call the macro fn for every interrupts value in the given node_id? In other words, a special-case of DT_FOREACH_PROP_ELEM, but for the interrupts property?

Note that DT_FOREACH_PROP_ELEM probably won't work for you, for the same reason that we have to have DT_FOREACH_RANGE -- the ranges and interrupts properties are both arrays, but they have special semantics which make a "logical" number of elements created by grouping cells inside the property.

[ycsin]

Would you have one common IRQ init function like that? Wouldn't it be called again and again for each UART, which would make the IRQ_CONNECT() calls too many times?

I'm basing on the methodology of #37585, so the IRQ init function will be called multiple times. The first caller will connect all IRQ lines and set the irq_init to true, so that the following ones will not be able to run the IRQ_CONNECT again.

I can't think of a better way to have separate IRQ init function for each UART, since some of them may share the same IRQ lines.

The goal is to make sure STM32_NEED_SHARED_IRQ_29 is defined if any of USART3, USART4, etc. are enabled.

Then in the driver, do

#ifdef STM32_NEED_SHARED_IRQ_29
/* define IRQ handler here */
#endif 

The issue with this that I can imagine is that different STM32 socs have different IRQ lines for UARTs, so it would be tedious to handle each of the STM32_NEED_SHARED_IRQ_* macro manually.

I'm having trouble understanding what you're asking about, sorry.

Please don't be. I'm sorry for not making my question clear enough.

Are you looking to have something like DT_FOREACH_IRQ(node_id, fn), which would call the macro fn for every interrupts value in the given node_id? In other words, a special-case of DT_FOREACH_PROP_ELEM, but for the interrupts property?

This macro loops though some array/property of a single node_id, I wanted to loop through distinct IRQ line of all enabled UARTs.

It looks like you want to know about every IRQ for a compatible, but that doesn't make sense to me.

For example:

UART	IRQ Line
1	27
2	28
3	28
4	29
5	29

(Based on the same methodology) If possible, for these 5 UARTs, I imagine that it would be nice if we have a DT macro that will loop through each distinct IRQ line once. For this example, it would be 3 iterations (27, 28, 29), so I can do this:

#define INIT_IRQ(num)							\
		IRQ_CONNECT(num,					\
			0,						\ ← need a way to get the priority
			uart_stm32_shared_irq_handler, NULL, 0);	\
		irq_enable(num);


static void uart_stm32_irq_init(const struct device *dev)
{
	if (!irq_init) {
		DT_COMPAT_IRQ_NUM_FOREACH_STATUS_OKAY(INIT_IRQ, st_stm32_uart);
		irq_init = true;
	}
}

which the compiler can resolve into:

static void uart_stm32_irq_init(const struct device *dev)
{
	if (!irq_init) {
		IRQ_CONNECT(27,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(27);

		IRQ_CONNECT(28,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(28);

		IRQ_CONNECT(29,
			0,
			uart_stm32_shared_irq_handler, NULL, 0);
		irq_enable(29);

		irq_init = true;
	}
}

[ycsin]

This probably can be done now using #61422