zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.96k stars 6.67k forks source link

Resolve inconsistent cell name for IRQ type in device trees #79356

Open jan-kiszka opened 1 month ago

jan-kiszka commented 1 month ago

Introduction

Let's consolidate over a single name to describe the IRQ type in interrupt controller bindings.

Problem description

There are currently 7 interrupt controller bindings using "sense" as cell name to describe the IRQ type and 6 using "flags". Drivers that shall be used with different controllers may be forced to look after both cell names - or fail to do that, see e.g.

https://github.com/zephyrproject-rtos/zephyr/blob/c710f8892beef3e486dded2f0ad4394e4a4d9aa0/drivers/serial/uart_ns16550.c#L1791

This is a source of subtle problems.

Proposed change

I have no strong preference about a unified name. We can either pick one of the two or maybe even rename them all to type so that using IRQ_TYPE_* constants for it feels more natural. I'm happy to write a PR for any of such renamings.

Future bindings should then be forced to use the new name for the flags/sense/type cell.

Dependencies

None.

Concerns and Unresolved Questions

See above: decision about the name is needed.

Alternatives

Applying lots of changes like this:

diff --git a/drivers/serial/uart_ns16550.c b/drivers/serial/uart_ns16550.c
index a46dc34aad7..8e1b39e436e 100644
--- a/drivers/serial/uart_ns16550.c
+++ b/drivers/serial/uart_ns16550.c
@@ -1787,9 +1787,12 @@ static const struct uart_driver_api uart_ns16550_driver_api = {
 };

 #define UART_NS16550_IRQ_FLAGS(n) \
-   COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, sense),                           \
-           (DT_INST_IRQ(n, sense)),                                  \
-           (0))
+   (COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, flags),                          \
+            (DT_INST_IRQ(n, flags)),                                 \
+            (0)) |                                                   \
+    COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, sense),                          \
+            (DT_INST_IRQ(n, sense)),                                 \
+            (0)))

 /* IO-port or MMIO based UART */
 #define UART_NS16550_IRQ_CONFIG(n)                                            \

One could also provide some common macro for that, of course, but all that would look strange.

github-actions[bot] commented 1 month ago

Hi @jan-kiszka! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. πŸ€–πŸ’™

jan-kiszka commented 1 month ago

@glneo