soc: renesas: ra: add interrupt map support for RA2A1

[thenguyenyf]

• Renesas RA has a dynamic interrupt mechanism consisting of m hardware interrupt events and n NVIC interrupt lines. At one time, just 1 hardware interrupt event can be routed to 1 NVIC line. The interrupt event was routed to NVIC controller with a m:n map by the Interrupt Controller Unit HWIP.
• This PR to add a partial support for interrupt event route to NVIC controller, by the Devicetree Interrupt mapping feature: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#interrupts-and-interrupt-mapping
• First target is RA2A1 MCU and the ek_ra2a1 board.

[bjarki-andreasen]

Would it not be simpler to just have one interrupt map, mapping from the ICU domain to the NVIC domain?

If a device is not used, it has no interrupts defined:

/ {
        soc {
                pwm1: pwm {
                                interrupt-parent = <&icu>; /* contains one big map of all ICU event to NVIC lines */
                                /* no interrupts defined */
                                status = "disabled";
                };
        }
};

then, if a user needs said device, set the status and define the IRQs

&pwm1 {
        status = "okay";
	interrupts = <RA_ICU_EVENT_GPT1_CAPTURE_COMPARE_A>,
		           <RA_ICU_EVENT_GPT1_COUNTER_OVERFLOW>;
        interrupt-names = "gtioca", "overflow";
};

I'm missing what these nodes are useful for: interrupt-parent = <&{/interrupt-controller-unit/pwm1}>;

[thenguyenyf]

Hi @bjarki-andreasen , nice to see you!

Would it not be simpler to just have one interrupt map, mapping from the ICU domain to the NVIC domain?

→ The big one interrupt map, that map all interrupt sources as default for enabled node, will have a NG point when someone would like to enable other node (etc. by an overlay file). They should redefine whole interrupt map, not only the interrupt map for their additional node, but also the default node - which should be handled before. So, I split the big map into many sub-units, then the user can modify a part but not necessary to overwrite the already mapped event.

If a device is not used, it has no interrupts defined

→ In my opinion, it is NG for user experience. The interrupts should be defined correctly: number of interrupts, the interrupts name, ..., because it is mandatory for driver functionality. It is very hard for user to investigate how to add the correct interrupt information for each shim driver. Currently, we are doing as your idea (not add interrupt prop for disabled node) to work around with the problem of not having enough NVIC interrupt lines, and the fact is, there is some feedback/question about how they can look up the information to add interrupt information for disabled node (in overlay or in their custom board dts).

[bjarki-andreasen]

Hi @bjarki-andreasen , nice to see you!

Would it not be simpler to just have one interrupt map, mapping from the ICU domain to the NVIC domain?

→ The big one interrupt map, that map all interrupt sources as default for enabled node, will have a NG point when someone would like to enable other node (etc. by an overlay file). They should redefine whole interrupt map, not only the interrupt map for their additional node, but also the default node - which should be handled before. So, I split the big map into many sub-units, then the user can modify a part but not necessary to overwrite the already mapped event.

Interrupt maps are not mutable, they should not be changed by an overlay. Interrupt maps contain every possible routing of any interrupt line between domains, this is restricted to physical hardware, in this case, which ICU event can be routed to which NVIC interrupt line.

If a device is not used, it has no interrupts defined

→ In my opinion, it is NG for user experience. The interrupts should be defined correctly: number of interrupts, the interrupts name, ..., because it is mandatory for driver functionality. It is very hard for user to investigate how to add the correct interrupt information for each shim driver.

Users have the option of looking to the interrupt map or reference manual to understand how interrupts can be routed. The devicetree is not a manual, its a description of hardware.

Currently, we are doing as your idea (not add interrupt prop for disabled node) to work around with the problem of not having enough NVIC interrupt lines, and the fact is, there is some feedback/question about how they can look up the information to add interrupt information for disabled node (in overlay or in their custom board dts).

Currently, this PR is working around the way interrupt maps work, which is not exactly user friendly for users who understand how interrupt maps work... I would have, and still have no clue how to use the novel approach to interrupt mapping presented in this PR, and that will apply to other users as well.

Please look to the devicetree specification, v0.4, section 2.4.4

Interrupt maps are well documented. This PR is deviating from the spec, which will leave users confused. If the hardware is complex, like the ICU inherently is, then the interrupt map will be complex. Its not a problem with the spec, it perfectly models the ICU, the problem seems to be an unwillingness to accurately model the hardware as is.

[thenguyenyf]

Let me summarize some points as my understanding, please correct me if there are any wrong:

• If there are 2 interrupt domains: ICU and NVIC, there should be 1 and only one interrupt nexus node to map all available event from ICU to NVIC to make it closest to HWM information.
• If there is some event that does not map into interrupt controller, it's interrupts prop should be remove because it may cause a DTC build error due to it doesn't route the event into the interrupt controller node
• The interrupt-map is not a configuration tool, something like hardware abstraction interface provided for user to config device node interrupt by event instead of NVIC IRQn and priority, it's just a visualization/modelling tool to describe exactly the hardware interrupt relationship by devicetree language. The interrupt-map is optional, beside directly adding the interrupt-controller specifier. So, someone else look at the devicetree can know about the hardware description (how many interrupts event this HWIP/node have, then which interrupt lines that this event was routed to).

[thenguyenyf]

So let me summarize some points as my understanding, please correct me if there are any wrong:

• If there are 2 interrupt domains: ICU and NVIC, there should be 1 and only one interrupt nexus node to map all available event from ICU to NVIC to make it closest to HWM information.
• If there is some event that does not map into interrupt controller, it's interrupts prop should be remove because it may cause a DTC build error due to it doesn't route the event into the interrupt controller node
• The interrupt-map is not a configuration tool, something like hardware abstraction interface provided for user to config device node interrupt by event instead of NVIC IRQn and priority, it's just a visualization/modelling tool to describe exactly the hardware interrupt relationship by devicetree language. The interrupt-map is optional, beside directly adding the interrupt-controller specifier. So, someone else look at the devicetree can know about the hardware description (how many interrupts event this HWIP/node have, then which interrupt lines that this event was routed to).

So, I'm afraid interrupt-map doesn't really solve my problem

• Number of ICU event >> number of NVIC line
• Complex scenario of interrupt routing from ICU to NVIC

[bjarki-andreasen]

Let me summarize some points as my understanding, please correct me if there are any wrong:

If there are 2 interrupt domains: ICU and NVIC, there should be 1 and only one interrupt nexus node
to map all available event from ICU to NVIC to make it closest to HWM information.

That I believe is correct.

If there is some event that does not map into interrupt controller, it's interrupts prop should be remove
because it may cause a DTC build error due to it doesn't route the event into the interrupt controller node

Yes, interrupt maps only contain possible mappings...

The interrupt-map is not a configuration tool, something like hardware abstraction interface provided for user to config device
node interrupt by event instead of NVIC IRQn and priority, it's just a visualization/modelling tool to describe exactly the
hardware interrupt relationship by devicetree language. The interrupt-map is optional, beside directly adding the
interrupt-controller specifier. So, someone else look at the devicetree can know about the hardware description (how many
interrupts event this HWIP/node have, then which interrupt lines that this event was routed to).

This is not correct. The interrupt-map is not optional. If interrupts are physically connected between two or more domains, all possible physical connections are defined in an interrupt map, and the subset of interrupts to connect are defined by devices which select the interrupts to connect their "events"/irq lines to in their domain (ICU domain in this case), which is then looked up in the interrupt-map to figure out which interrupt controller it routes to in any parent domain (NVIC in this case).

To step back a bit, the reason we use interrupt maps, is quite simply because it is a user choice, and I really want to emphasize: "user choice" here, which, if any, device event is actually routed to an interrupt line.

Device drivers can be written to be entirely polling in most cases, not using any interrupts which any hardware provides. Interrupts are an optional feature, which can be used for async hardware usage, if the user so desires. Think of a flash controller for example, it could have an interrupt line which triggers when its done erasing a page, a driver could choose to use this interrupt to wait, or it could just spin, checking a flag which is set when its done. This is the case for most hardware.

When one uses a platform where there are more events/interrupts than there are interrupt lines available to the CPU, as mentioned here as well

So, I'm afraid interrupt-map doesn't really solve my problem

• Number of ICU event >> number of NVIC line
• Complex scenario of interrupt routing from ICU to NVIC

the user must prioritize which hardware event should be routed to an interrupt, and which can simply be polled. There is no algorithm to choose this, its a user choice. The interrupt map makes this selection possible, along with the user being able to specify on a per device level which event to actually connect to which interrupt line, if any.

The interrupt-map, despite what you claim, does indeed solve the problem as good as I believe possible. It provides every possible interrupt configuration, which the user can then select from. Its a complex problem, with a complimentarily complex solution. It being "complex" in itself is not an argument, unless the complexity of the interrupt-map itself was more complex than the ICU to NVIC mapping, which it is not IMO, its near identical in complexity.

[thenguyenyf]

I'm still wondering about 2 points:

• If we have a fixed interrupt-map (maybe in the board layer), how about if someone enable a new device node in their overlay? I think they will not be happy to rewrite the whole interrupt-map to enable just 1 or 2 nodes.
• For each shim driver/dt-binding in SoC dtsi, I think we should keep at least 1 node with fully interrupts define for user reference. The Renesas RA HWIP have multiple interrupts for each HWIP event, and they are distinguished by interrupt-names. User should fill correct name for interrupt to make shim driver recognize it. There is no guarantee that the number of predefined interrupts in the SoC dtsi and board dts does not exceed the number of NVIC lines. It is the reason that I'm looking for a feasible solution to add the interrupt information (at least 1 node for each shim driver, better to add for all device node).

[bjarki-andreasen]

I'm still wondering about 2 points:

• If we have a fixed interrupt-map (maybe in the board layer), how about if someone enable a new device node in their overlay? I think they will not be happy to rewrite the whole interrupt-map to enable just 1 or 2 nodes.

Unless that someone also changes what SoC they are using, the interrupt-map will remain unchanged :) The interrupt-map is a lookup table, it contains only possibilities, not decisions.

• For each shim driver/dt-binding in SoC dtsi, I think we should keep at least 1 node with fully interrupts define for user reference. The Renesas RA HWIP have multiple interrupts for each HWIP event, and they are distinguished by interrupt-names. User should fill correct name for interrupt to make shim driver recognize it. There is no guarantee that the number of predefined interrupts in the SoC dtsi and board dts does not exceed the number of NVIC lines. It is the reason that I'm looking for a feasible solution to add the interrupt information (at least 1 node for each shim driver, better to add for all device node).

Devicetree bindings, the .yaml files in /dts/bindings, are where possible interrupts/events along with their names are described. See https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/serial/infineon%2Cxmc4xxx-uart.yaml for example. DMAs, ram sections, whatever hardware information specific to any device are in those.

When creating a board, define a subset of peripherals in a viable manner, which users can use for reference. Like an instance of a uart where you connect its interrupts. The user may need to change the interrupts or disable the peripheral in their overlays later, that is expected.

[thenguyenyf]

• For each shim driver/dt-binding in SoC dtsi, I think we should keep at least 1 node with fully interrupts define for user reference. The Renesas RA HWIP have multiple interrupts for each HWIP event, and they are distinguished by interrupt-names. User should fill correct name for interrupt to make shim driver recognize it. There is no guarantee that the number of predefined interrupts in the SoC dtsi and board dts does not exceed the number of NVIC lines. It is the reason that I'm looking for a feasible solution to add the interrupt information (at least 1 node for each shim driver, better to add for all device node).

Devicetree bindings, the .yaml files in /dts/bindings, are where possible interrupts/events along with their names are described. See https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/serial/infineon%2Cxmc4xxx-uart.yaml for example. DMAs, ram sections, whatever hardware information specific to any device are in those.

When creating a board, define a subset of peripherals in a viable manner, which users can use for reference. Like an instance of a uart where you connect its interrupts. The user may need to change the interrupts or disable the peripheral in their overlays later, that is expected.

It sounds good.

• If we have a fixed interrupt-map (maybe in the board layer), how about if someone enable a new device node in their overlay? I think they will not be happy to rewrite the whole interrupt-map to enable just 1 or 2 nodes.

Unless that someone also changes what SoC they are using, the interrupt-map will remain unchanged :) The interrupt-map is a lookup table, it contains only possibilities, not decisions.

Maybe I missed something. Do you mean the interrupt map can be defined as below (map the sci0 and spi0 to NVIC slot 0-7 as their possibilities)?

&icu {
	interrupt-map = <
		/* sci0 interrupts variant 1*/
		0 RA_ICU_EVENT_SCI0_RXI &nvic 0 0 1	/* NVIC: IRQn = 0, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_TXI &nvic 0 1 1	/* NVIC: IRQn = 1, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_TEI &nvic 0 2 1	/* NVIC: IRQn = 2, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_ERI &nvic 0 3 1	/* NVIC: IRQn = 3, IPL = 1 */

		/* sci0 interrupts variant 2*/
		0 RA_ICU_EVENT_SCI0_RXI &nvic 0 4 1	/* NVIC: IRQn = 4, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_TXI &nvic 0 5 1	/* NVIC: IRQn = 5, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_TEI &nvic 0 6 1	/* NVIC: IRQn = 6, IPL = 1 */
		0 RA_ICU_EVENT_SCI0_ERI &nvic 0 7 1	/* NVIC: IRQn = 7, IPL = 1 */
		
		/* spi1 interrupt variant 1 */
		0 RA_ICU_EVENT_SPI1_RXI &nvic 0 0 1	/* NVIC: IRQn = 0, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_TXI &nvic 0 1 1	/* NVIC: IRQn = 1, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_TEI &nvic 0 2 1	/* NVIC: IRQn = 2, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_ERI &nvic 0 3 1	/* NVIC: IRQn = 3, IPL = 1 */

		/* spi1 interrupt variant 2 */
		0 RA_ICU_EVENT_SPI1_RXI &nvic 0 4 1	/* NVIC: IRQn = 4, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_TXI &nvic 0 5 1	/* NVIC: IRQn = 5, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_TEI &nvic 0 6 1	/* NVIC: IRQn = 6, IPL = 1 */
		0 RA_ICU_EVENT_SPI1_ERI &nvic 0 7 1	/* NVIC: IRQn = 7, IPL = 1 */
		>;
};

I received a build error for multiple registration at an ISR slot. Because the ICU doesn't allow shared interrupt, let's take example with RA8M1 (have 296 ICU events and 96 NVIC lines) we can only map 96 events to 96 NVIC line. So, if user want to use a disabled device node (define in SoC dtsi but not enabled by default in board dts), they should remove/disabled some device node, then map its ICU event to the removed device node NVIC slots, right? And I think it's a common usage, because I see the samples/ztests overlays to enable a device node that not used by the board dts everywhere.

[bjarki-andreasen]

The interrupt-map is erroneous, the child specifiers have to be unique:

        0 RA_ICU_EVENT_SCI0_RXI &nvic 0 0 1	/* NVIC: IRQn = 0, IPL = 1 */

and

        0 RA_ICU_EVENT_SCI0_RXI &nvic 0 4 1	/* NVIC: IRQn = 4, IPL = 1 */

have identical child specifiers, so dts can not determine which to select. You need to add more child specifiers to distinguish which event "group" you use.

Have you seen #75946? See this comment for a viable interrupt map: #75946 (comment)

[thenguyenyf]

The interrupt-map is erroneous, the child specifiers have to be unique:

        0 RA_ICU_EVENT_SCI0_RXI &nvic 0 0 1	/* NVIC: IRQn = 0, IPL = 1 */

and

        0 RA_ICU_EVENT_SCI0_RXI &nvic 0 4 1	/* NVIC: IRQn = 4, IPL = 1 */

have identical child specifiers, so dts can not determine which to select. You need to add more child specifiers to distinguish which event "group" you use.

Have you seen #75946? See this comment for a viable interrupt map: #75946 (comment)

Okay. Let's me investigate on what we have discussed. Thanks for your help!