Interrupt controller DT macros and GPIO IRQ utilities #65779

bjarki-andreasen · 2023-11-26T13:00:40Z

Introduction

Implement interrupt-controller related macros to get the handle to an interrupt specifier's interrupt controller as described in Devicetree specification v0.4 section 2.4.1, and add the utilities and macros required to use the devicetree specification.

What will it solve

The following example shows how we currently configure GPIO routed interrupts in-tree for the ST LIS2DH, which generates two pin routed interrupts:

#include <zephyr/dt-bindings/gpio/gpio.h>
#include <zephyr/dt-bindings/sensor/lis2dh.h>

&i2c0 {
	sensor: lis2dh {
		compatible = "st,lis2dh";
		irq-gpios = <&gpio0 1 (GPIO_ACTIVE_HIGH | GPIO_PULL_UP)>,
		            <&gpio0 2 GPIO_ACTIVE_HIGH>;
		int1-gpio-config = LIS2DH_DT_GPIO_INT_EDGE_RISING;
		int2-gpio-config = LIS2DH_DT_GPIO_INT_EDGE_RISING;
	};
};

This solution is not optimal, as it is not generic, and does not follow the devicetree specification 2.4.1. This results in custom bindings required to configure the interrupt lines, and makes it near impossible to create subsystems and other utilities which interact with interrupt generating devices generically (like a wakeup source subsystem, or generic utilities for interacting with interrupt requests)

What does the solution look like

Using the properties for interrupt generating devices defined in the devicetree specification, the same sensor will take one of the following forms:

#include <zephyr/dt-bindings/gpio/gpio.h>
#include <zephyr/dt-bindings/interrupt-controller/irq.h>

&i2c0 {
	sensor: lis2dh {
		compatible = "st,lis2dh";
		interrupt-parent = <&gpio0>,
		interrupts = <1 (IRQ_TYPE_EDGE_RISING | GPIO_PULL_UP)>,
		             <2 IRQ_TYPE_EDGE_RISING>;
	};
};

or

#include <zephyr/dt-bindings/gpio/gpio.h>
#include <zephyr/dt-bindings/interrupt-controller/irq.h>

&i2c0 {
	sensor: lis2dh {
		compatible = "st,lis2dh";
		interrupts-extended = <&gpio0 1 (IRQ_TYPE_EDGE_RISING | GPIO_PULL_UP)>,
		                      <&gpio0 2 IRQ_TYPE_EDGE_RISING>;
	};
};

This is compatible with the Linux binding for the same sensor st-sensors.yaml

Note: this also allows us to exchange the GPIO controller for a programmable interrupt controller potentially in the future like:

	&i2c0 {
		sensor: lis2dh {
			compatible = "st,lis2dh";
			interrupt-parent = <&pic0>,
			interrupts = <1 (IRQ_TYPE_EDGE_RISING | GPIO_PULL_UP)>,
			             <2 IRQ_TYPE_EDGE_RISING>;
		};
	};

How will drivers use the new macros

Drivers will get a GPIO IRQ specification from the devicetree using a variation of GPIO_IRQ_DT_SPEC_GET() which initializes a struct gpio_irq_dt_spec, similar to how GPIO_DT_SPEC_GET() works.

struct foo_config {
	struct gpio_irq_dt_spec irq_spec;
};

const struct foo_config##inst = {
	.irq_spec = GPIO_IRQ_DT_SPEC_GET(inst),
};

Drivers will then utilize the gpio_irq utilities to configure a GPIO PIN as an interrupt as described by the spec.

struct foo_data {
	struct gpio_irq irq;
};

void foo_irq_handler(struct gpio_irq *irq)
{
	struct foo_data *data = CONTAINER_OF(irq, struct foo_data, irq);
	...
}

void foo_init(const struct device *dev)
{
	const struct foo_config *config = dev->config;
	struct foo_data *data = dev->data;

	/* Configure interrupt generating device to follow irq spec from devicetree */
	if (config->irq_spec.flags & IRQ_TYPE_EDGE_RISING) {
		...
	} else if (config->irq_spec.flags & IRQ_TYPE_EDGE_FALLING)
		...
	}

	/* Set pin interrupt and enable it according to irq spec from devicetree */
	gpio_irq_request_dt(&config->irq_spec, &data->irq, foo_irq_handler);
}

The struct gpio_irq instance is used to further interact with the GPIO pin interrupt

int foo_resume(const struct device *dev)
{
	struct foo_data *data = dev->data;

	return gpio_irq_enable(&data->irq);
}

int foo_suspend(const struct device *dev)
{
	struct foo_data *data = dev->data;

	return gpio_irq_disable(&data->irq);
}

Preemptive Q&A

What happens to `GPIO_ACTIVE_LOW` and `GPIO_INT_LEVELS_LOGICAL`

These flags are no longer useful as the IRQ_TYPE_* flags completely cover the information the GPIO flags where conveying.

Can we still use generic and vendor specific `GPIO_*` flags like `GPIO_PULL_UP`?

Yes! except for the first 4 flags which configure GPIO output and logical states, which are now illegal, and are replaced by the
new IRQ_TYPE_* flags.

Could we not just reuse `struct gpio_dt_spec`?

Technically yes, the layout of the structure is identical, but, this would be unsafe as it would indicate that the struct gpio_irq_dt_spec and struct gpio_dt_spec are interchangeable, which they are not. For struct gpio_irq_dt_spec, we enforce that the controller(port) is in fact also an interrupt controller, and we enforce that the irq(pin) is in fact from an interrupts property.

mbolivar-ampere

Looks like a great idea. I found some issues with the implementation.

mbolivar-ampere · 2023-11-30T00:36:48Z

scripts/dts/gen_defines.py

@@ -477,6 +477,13 @@ def map_arm_gic_irq_type(irq, irq_num):
                name_vals.append((name_macro, f"DT_{idx_macro}"))
                name_vals.append((name_macro + "_EXISTS", 1))

+        idx_controller_macro = f"{path_id}_IRQ_IDX_{i}_CONTROLLER"


Changes to the generated macros require corresponding doc/build/dts/macros.bnf updates. Please add them.

mbolivar-ampere · 2023-11-30T01:14:18Z

scripts/dts/gen_defines.py

+        idx_vals.append((idx_controller_macro, idx_controller_path))
+        if irq.name:
+            name_controller_macro = f"{path_id}_IRQ_NAME_{str2ident(irq.name)}_CONTROLLER"
+            name_vals.append((name_controller_macro, idx_controller_macro))


name_vals.append((name_controller_macro, f"DT_{idx_controller_macro}"))

missing DT_. The value of the macro is a node identifier.

mbolivar-ampere · 2023-11-30T01:22:08Z

include/zephyr/devicetree.h

+/**
+ * @brief Get an interrupt specifier's interrupt controller by name
+ *
+ * Example usage:


This is a nonblocking comment, but I try to keep the docstrings self-contained so that the user doesn't have to jump back and forth between the devicetree snippet in the DT_IRQ_INTC_BY_IDX() docstring and the example in the DT_IRQ_INTC_BY_NAME() docstring. You could make this happen with a little extra copy/pasting. That would also allow you to delete the interrupt-names properties from the DT_IRQ_INTC_BY_IDX() docstring, where they are not relevant.

mbolivar-ampere · 2023-11-30T01:31:59Z

include/zephyr/drivers/gpio/gpio_irq.h

+			DT_IRQ_INTC_BY_IDX(node_id, idx),			\
+			gpio_controller						\
+		),								\
+		DEVICE_DT_GET(DT_IRQ_INTC_BY_IDX(node_id, idx))			\


this is missing () around the second IF_ENABLED argument; missing test case?

mbolivar-ampere · 2023-11-30T01:32:39Z

include/zephyr/drivers/gpio/gpio_irq.h

+ */
+#define GPIO_IRQ_DT_INTC_BY_IDX(node_id, idx)					\
+	IF_ENABLED(								\
+		DT_HAS_PROP(							\


DT_NODE_HAS_PROP, not DT_HAS_PROP; missing test case?

mbolivar-ampere · 2023-11-30T01:33:08Z

include/zephyr/drivers/gpio/gpio_irq.h

+ * @brief Get interrupt controller device by name
+ * @note Validates interrupt controller is also a GPIO controller
+ */
+#define GPIO_IRQ_DT_INTC_BY_NAME(node_id, name)					\


similar comments here

mbolivar-ampere · 2023-11-30T01:34:37Z

include/zephyr/devicetree.h

+ * @return node_id of interrupt specifier's interrupt controller
+ * @see DT_IRQ_INTC_BY_IDX()
+ */
+#define DT_IRQ_INTC(node_id) \


All the new DT macros need test cases in tests/lib/devicetree/api

bjarki-andreasen · 2023-11-30T07:56:35Z

@mbolivar-ampere Thank you for reviewing, I'm currently writing the test suite :) just wanted to share the design a bit early :)

mbolivar-ampere · 2023-11-30T21:54:56Z

just wanted to share the design a bit early :)

OK! Please consider using a draft PR in the future for work that is not ready. This looked like you wanted it in, especially since you asked for a review on Discord.

bjarki-andreasen · 2023-12-01T12:55:15Z

just wanted to share the design a bit early :)

OK! Please consider using a draft PR in the future for work that is not ready. This looked like you wanted it in, especially since you asked for a review on Discord.

Moved back to draft :)

Add the following macros to devicetree.h to get an interrupt specifier's interrupt controller: DT_IRQ_INTC_BY_IDX(node_id, idx) DT_IRQ_INTC_BY_NAME(node_id, name) DT_IRQ_INTC(node_id) and their INST variants DT_INST_INTC_BY_IDX(inst, idx) DT_INST_INTC_BY_NAME(inst, name) DT_INST_IRQ(inst) which use the newly generated _CONTROLLER output from the previous commit. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2023-12-09T15:48:00Z

Added documentation for GPIO interrupt requests with examples and API overview

Extend api test suite to cover the new devicetree macros. This includes extending the devicetree overlay with two new bindings: - GPIO device which is also an interrupt controller - interrupt holder using interrupts-extended to point to both existing interrupt controller test_intc, and the newly added GPIO device Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2023-12-10T12:52:03Z

Update

Changed _OPTIONAL_ to _OPT_ to shorten macro names
Fixed erroneous IRQ_TYPE_* usage in documentation
Made functions in doc example driver static
Added comment to doc example driver to configure sensor interrupt lines before enabling interrupt requests

Add common definitions for interrupt requests to be used in devicetree and by various interrupt controllers and interrupt generating devices. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Extend bindings for gpio-emul to include interrupt-controller and the required interrupt-cells pin and flags. Then add the new required properties interrupt-controller and interrupt-cells to the gpio0 node on the native_sim board. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

The GPIO IRQ utility encapsulates the GPIO interrupt pin and callback APIs, providing a simple interrupt request centered API to be used by interrupt generating devices. The utility will replace existing boilerplate code for using GPIO pins as IRQs within drivers, while increasing usability by being compatible with the devicetree specification v0.4 section 2.4.1 interrupt specification. This addition increases readability, utility, and adds compatibility with Linux devicetree bindings, while removing boilerplate code. For example, in-tree drivers currently using the irq-gpios and similar bindings, with additional custom bindings like int1-gpio-config to add the missing functionality covered by the new IRQ_TYPE_* flags. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Test IRQ API and GPIO_IRQ_DT_* macros using a devicetree overlay and the emulated GPIO driver. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

Add documentation of GPIO IRQ with examples, and reference GPIO IRQ API. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2023-12-10T22:00:10Z

Update

Added helper macros to validate and test IRQ_TYPE_* flags
Implemented new helpers in GPIO IRQ utilities
Added error handling if gpio_irq_enable() fails when requesting GPIO IRQ using gpio_irq_request()

carlescufi · 2023-12-12T16:58:58Z

Architecture WG:

@henrikbrixandersen is worried that the current suggestion will still require clients using GPIO-based interrupts to handle it in the code with the GPIO API, instead of using the interrupt API. He mentions external interrupt controllers that have external pins available
@henrikbrixandersen asks for an interrupt controller API and then map this to the existing GPIO controller API

bjarki-andreasen · 2024-01-03T13:31:02Z

Closing this PR for now since it will probably be reworked based on the a new interrupt controller API (this one once its ready hopefully #66505)

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch 2 times, most recently from 4af08c9 to cce75e8 Compare November 26, 2023 13:23

bjarki-andreasen changed the title ~~Interrupt parent gpio controller~~ Interrupt controller DT macros Nov 26, 2023

bjarki-andreasen requested review from gmarull, mbolivar-ampere and vaishnavachath November 26, 2023 14:11

bjarki-andreasen changed the title ~~Interrupt controller DT macros~~ Interrupt controller macros and API Nov 26, 2023

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch 4 times, most recently from 4b5c10f to 798fed1 Compare November 26, 2023 17:20

bjarki-andreasen changed the title ~~Interrupt controller macros and API~~ Interrupt controller DT macros Nov 26, 2023

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from 798fed1 to 5713c24 Compare November 26, 2023 18:05

bjarki-andreasen requested a review from henrikbrixandersen November 26, 2023 18:53

bjarki-andreasen marked this pull request as ready for review November 26, 2023 19:54

zephyrbot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: native port Host native arch port (native_sim) area: Interrupt Controller labels Nov 26, 2023

zephyrbot requested review from aescolar and galak November 26, 2023 19:54

zephyrbot assigned galak Nov 26, 2023

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from 5713c24 to 5288554 Compare November 29, 2023 21:11

zephyrbot added the area: GPIO label Nov 29, 2023

zephyrbot requested a review from mnkp November 29, 2023 21:12

mbolivar-ampere requested changes Nov 30, 2023

View reviewed changes

bjarki-andreasen marked this pull request as draft December 1, 2023 12:54

zephyrbot requested review from jeremybettis, nashif, nordicjm and tejlmand December 8, 2023 19:54

bjarki-andreasen requested review from fabiobaltieri and MaureenHelm December 8, 2023 22:32

bjarki-andreasen changed the title ~~Interrupt controller DT macros~~ Interrupt controller DT macros and utilities Dec 8, 2023

bjarki-andreasen changed the title ~~Interrupt controller DT macros and utilities~~ Interrupt controller DT macros and GPIO IRQ utilities Dec 8, 2023

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from 923c9d7 to 4047ea2 Compare December 9, 2023 15:47

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from 4047ea2 to bfc9a5c Compare December 9, 2023 18:11

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from bfc9a5c to bc43216 Compare December 9, 2023 20:06

bjarki-andreasen requested a review from kartben December 10, 2023 12:14

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch 2 times, most recently from a66f114 to a86834f Compare December 10, 2023 12:49

bjarki-andreasen added 5 commits December 10, 2023 22:56

devicetree: dt-bindings: interrupt-controller: Add irq.h

fd39c4b

Add common definitions for interrupt requests to be used in devicetree and by various interrupt controllers and interrupt generating devices. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

tests: drivers: gpio: Add gpio_irq test suite

c793d28

Test IRQ API and GPIO_IRQ_DT_* macros using a devicetree overlay and the emulated GPIO driver. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

doc: hardware: Add gpio_irq documentation

8d4bb3c

Add documentation of GPIO IRQ with examples, and reference GPIO IRQ API. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen force-pushed the interrupt_parent_gpio_controller branch from a86834f to 8d4bb3c Compare December 10, 2023 21:57

bjarki-andreasen mentioned this pull request Dec 13, 2023

drivers: Add interrupt controller API #66505

Draft

bjarki-andreasen added the DNM This PR should not be merged (Do Not Merge) label Jan 3, 2024

bjarki-andreasen closed this Jan 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interrupt controller DT macros and GPIO IRQ utilities #65779

Interrupt controller DT macros and GPIO IRQ utilities #65779

bjarki-andreasen commented Nov 26, 2023 •

edited

Loading

mbolivar-ampere left a comment

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

mbolivar-ampere Nov 30, 2023

bjarki-andreasen commented Nov 30, 2023

mbolivar-ampere commented Nov 30, 2023

bjarki-andreasen commented Dec 1, 2023

bjarki-andreasen commented Dec 9, 2023

bjarki-andreasen commented Dec 10, 2023 •

edited

Loading

bjarki-andreasen commented Dec 10, 2023

carlescufi commented Dec 12, 2023

bjarki-andreasen commented Jan 3, 2024

Interrupt controller DT macros and GPIO IRQ utilities #65779

Interrupt controller DT macros and GPIO IRQ utilities #65779

Conversation

bjarki-andreasen commented Nov 26, 2023 • edited Loading

Introduction

What will it solve

What does the solution look like

How will drivers use the new macros

Preemptive Q&A

What happens to GPIO_ACTIVE_LOW and GPIO_INT_LEVELS_LOGICAL

Can we still use generic and vendor specific GPIO_* flags like GPIO_PULL_UP?

Could we not just reuse struct gpio_dt_spec?

mbolivar-ampere left a comment

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

mbolivar-ampere Nov 30, 2023

Choose a reason for hiding this comment

bjarki-andreasen commented Nov 30, 2023

mbolivar-ampere commented Nov 30, 2023

bjarki-andreasen commented Dec 1, 2023

bjarki-andreasen commented Dec 9, 2023

bjarki-andreasen commented Dec 10, 2023 • edited Loading

Update

bjarki-andreasen commented Dec 10, 2023

Update

carlescufi commented Dec 12, 2023

bjarki-andreasen commented Jan 3, 2024

bjarki-andreasen commented Nov 26, 2023 •

edited

Loading

What happens to `GPIO_ACTIVE_LOW` and `GPIO_INT_LEVELS_LOGICAL`

Can we still use generic and vendor specific `GPIO_*` flags like `GPIO_PULL_UP`?

Could we not just reuse `struct gpio_dt_spec`?

bjarki-andreasen commented Dec 10, 2023 •

edited

Loading