From 146f17ac732da4d2299ec5440d391522401f22bc Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Wed, 14 Jun 2023 21:35:48 +0200 Subject: [PATCH 1/5] core: interrupt: registering interrupt providers Adds interrupt chip framework API functions for an interrupt controller to register as an interrupt provider in the driver probing sequence based on device tree. This allows interrupt consumer to be deferred when a dependent interrupt controller is not yet initialized. Interrupt controller register driver in DT_DRIVER providers list with: dt_register_interrupt_provider(). Interrupt consumer can get their interrupt through DT data with dt_get_interrupt(), dt_get_interrupt_by_index() or dt_get_interrupt_by_name(). This change removes inclusion of interrupt.h from kernel/dt.h as it is not needed and conflicts with inclusion of kernel/dt.h from kernel/interrupt.h. Signed-off-by: Etienne Carriere --- core/arch/arm/kernel/thread.c | 1 + core/drivers/atmel_piobu.c | 1 + core/drivers/atmel_wdt.c | 1 + core/include/kernel/dt.h | 1 - core/include/kernel/interrupt.h | 128 ++++++++++++++++++++++++++++++++ core/kernel/interrupt.c | 119 +++++++++++++++++++++++++++++ 6 files changed, 250 insertions(+), 1 deletion(-) diff --git a/core/arch/arm/kernel/thread.c b/core/arch/arm/kernel/thread.c index 4487ef026df..3748d5bfeac 100644 --- a/core/arch/arm/kernel/thread.c +++ b/core/arch/arm/kernel/thread.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include diff --git a/core/drivers/atmel_piobu.c b/core/drivers/atmel_piobu.c index 1f6aa115f8f..de1da2039b7 100644 --- a/core/drivers/atmel_piobu.c +++ b/core/drivers/atmel_piobu.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include diff --git a/core/drivers/atmel_wdt.c b/core/drivers/atmel_wdt.c index b2aa5458d17..93bb1f97a9b 100644 --- a/core/drivers/atmel_wdt.c +++ b/core/drivers/atmel_wdt.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include diff --git a/core/include/kernel/dt.h b/core/include/kernel/dt.h index e6ca702a0b3..257fbd5b57e 100644 --- a/core/include/kernel/dt.h +++ b/core/include/kernel/dt.h @@ -7,7 +7,6 @@ #define KERNEL_DT_H #include -#include #include #include #include diff --git a/core/include/kernel/interrupt.h b/core/include/kernel/interrupt.h index 89fe8572486..f8ec9b5eb8b 100644 --- a/core/include/kernel/interrupt.h +++ b/core/include/kernel/interrupt.h @@ -6,6 +6,7 @@ #define __KERNEL_INTERRUPT_H #include +#include #include #include #include @@ -70,6 +71,20 @@ struct itr_ops { uint8_t cpu_mask); }; +/* + * struct itr_desc - Interrupt description + * @chip Interrupt controller reference + * @itr_num Interrupt number + * + * This struct is used for binding interrupt device data between + * drivers when using DT_DRIVERS means. See itr_dt_get_func type + * definition. + */ +struct itr_desc { + struct itr_chip *chip; + size_t itr_num; +}; + /* Interrupt handler return value */ enum itr_return { ITRR_NONE, @@ -343,4 +358,117 @@ TEE_Result interrupt_alloc_add_handler(struct itr_chip *chip, size_t it_num, * This function may panic on non-NULL invalid @hdl reference. */ void interrupt_remove_free_handler(struct itr_handler *hdl); + +/* + * itr_dt_get_func - Typedef of function to get an interrupt in DT node + * + * @args Reference to phandle arguments + * @data Pointer to data given at interrupt_register_provider() call + * @res Output result code of the operation: + * TEE_SUCCESS in case of success + * TEE_ERROR_DEFER_DRIVER_INIT if controller is not initialized + * Any TEE_Result compliant code in case of error. + * + * Returns an allocated struct itr_desc pointer referencing to an interrupt + * or NULL if invalid description in which case @res provides the error + * code. The interrupt matches the reference found with properties of the + * pointer FDT node. + * + * Upon success, the interrupt is configured and consumer can add a handler + * function to the interrupt. Yet, the interrupt is not enabled until consumer + * calls interrupt_enable(). + * Upon success, struct itr_desc must point to memory allocated with malloc() + * or like as it is freed prior returning to caller function. + */ +typedef struct itr_desc *(*itr_dt_get_func)(struct dt_pargs *args, void *data, + TEE_Result *res); + +#ifdef CFG_DT +/** + * interrupt_register_provider() - Register an interrupt provider + * + * @fdt Device tree to work on + * @node Node offset of the interrupt controller in the DT + * @dt_get_itr Callback to match the devicetree interrupt reference with + * @data Data which will be passed to the get_dt_its callback + */ +TEE_Result interrupt_register_provider(const void *fdt, int node, + itr_dt_get_func dt_get_itr, void *data); + +/** + * dt_get_interrupt_by_index() - Get an interrupt from DT by interrupt index + * + * Interrupt index (@index) refers to the index of the target interrupt to be + * retrieved as DT binding property "interrupts" may define several + * interrupts. + * + * @fdt Device tree to work on + * @node Node offset of the subnode containing interrupt(s) references + * @index Index in "interrupts" or "extended-interrupts" property list + * @chip Output interrupt controller reference upon success + * @itr_num Output interrupt number upon success + * + * Return TEE_SUCCESS in case of success + * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt controller is not yet inited + * Return TEE_ERROR_ITEM_NOT_FOUND if the DT does not reference target interrupt + * Return any other TEE_Result compliant code in case of error + */ +TEE_Result dt_get_interrupt_by_index(const void *fdt, int node, + unsigned int index, struct itr_chip **chip, + size_t *itr_num); + +/** + * dt_get_interrupt_by_name() - Get an interrupt from DT by interrupt name + * + * @fdt Device tree to work on + * @node Node offset of the subnode containing interrupt(s) references + * @name Name identifier used in "interrupt-names" property + * @chip Output interrupt controller reference upon success + * @itr_num Output interrupt number upon success + * + * Return TEE_SUCCESS in case of success + * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt controller is not yet inited + * Return TEE_ERROR_ITEM_NOT_FOUND if the DT does not reference target interrupt + * Return any other TEE_Result compliant code in case of error + */ +TEE_Result dt_get_interrupt_by_name(const void *fdt, int node, const char *name, + struct itr_chip **chip, size_t *itr_num); +#else +static inline TEE_Result interrupt_register_provider(const void *dt __unused, + int node __unused, + itr_dt_get_func f __unused, + void *data __unused) +{ + return TEE_ERROR_NOT_IMPLEMENTED; +} + +static inline TEE_Result dt_get_interrupt_by_index(const void *fdt __unused, + int node __unused, + unsigned int index __unused, + struct itr_chip **c __unused, + size_t *itr_num __unused) +{ + return TEE_ERROR_NOT_IMPLEMENTED; +} + +static inline TEE_Result dt_get_interrupt_by_name(const void *fdt __unused, + int node __unused, + const char *name __unused, + struct itr_chip **ch __unused, + size_t *itr_num __unused) +{ + return TEE_ERROR_NOT_IMPLEMENTED; +} +#endif /*CFG_DT*/ + +/* + * Helper function for when caller retrieves the first interrupt defined + * in "interrupts" or "extended-interrupts" DT binding property list. + */ +static inline TEE_Result dt_get_interrupt(const void *fdt, int node, + struct itr_chip **chip, + size_t *itr_num) +{ + return dt_get_interrupt_by_index(fdt, node, 0, chip, itr_num); +} #endif /*__KERNEL_INTERRUPT_H*/ diff --git a/core/kernel/interrupt.c b/core/kernel/interrupt.c index 7d10dd4ece0..ac83ad0b501 100644 --- a/core/kernel/interrupt.c +++ b/core/kernel/interrupt.c @@ -264,3 +264,122 @@ void interrupt_remove_free_handler(struct itr_handler *hdl) free(hdl); } } + +#ifdef CFG_DT +TEE_Result interrupt_register_provider(const void *fdt, int node, + itr_dt_get_func dt_get_itr, void *data) +{ + return dt_driver_register_provider(fdt, node, + (get_of_device_func)dt_get_itr, + data, DT_DRIVER_INTERRUPT); +} + +/* + * Provide an itr_desc reference based on "interrupts" property bindings. + * May return TEE_ERROR_DEFER_DRIVER_INIT if parent controller is found but + * not yet initialized. + */ +static TEE_Result get_legacy_interrupt_by_index(const void *fdt, int node, + unsigned int index, + struct itr_desc **desc) +{ + TEE_Result res = TEE_ERROR_GENERIC; + const uint32_t *prop = NULL; + uint32_t phandle = 0; + int pnode = 0; + int len = 0; + + prop = fdt_getprop(fdt, node, "interrupts", &len); + if (!prop) + return TEE_ERROR_ITEM_NOT_FOUND; + + /* Find "interrupt-parent" in node or its parents */ + pnode = node; + prop = fdt_getprop(fdt, pnode, "interrupt-parent", &len); + + while (!prop) { + pnode = fdt_parent_offset(fdt, pnode); + if (pnode < 0) + break; + + prop = fdt_getprop(fdt, pnode, "interrupt-parent", &len); + if (!prop && len != -FDT_ERR_NOTFOUND) + break; + } + if (!prop) { + DMSG("No interrupt parent for node %s", + fdt_get_name(fdt, node, NULL)); + return TEE_ERROR_GENERIC; + } + + /* "interrupt-parent" provides interrupt controller phandle */ + phandle = fdt32_to_cpu(prop[0]); + + /* Get interrupt chip/number from phandle and "interrupts" property */ + *desc = dt_driver_device_from_node_idx_prop_phandle("interrupts", fdt, + node, index, + DT_DRIVER_INTERRUPT, + phandle, &res); + return res; +} + +/* + * Provide an itr_desc based on "interrupts-extended" property bindings. + * May return TEE_ERROR_DEFER_DRIVER_INIT if parent controller is found + * but not yet initialized. + * With this function, provider is expected to have allocated itr_desc + * with malloc() or like. + */ +static TEE_Result get_extended_interrupt_by_index(const void *fdt, int node, + unsigned int index, + struct itr_desc **desc) +{ + TEE_Result res = TEE_ERROR_GENERIC; + + *desc = dt_driver_device_from_node_idx_prop("interrupts-extended", + fdt, node, index, + DT_DRIVER_INTERRUPT, &res); + + return res; +} + +TEE_Result dt_get_interrupt_by_index(const void *fdt, int node, + unsigned int index, struct itr_chip **chip, + size_t *itr_num) +{ + TEE_Result res = TEE_ERROR_GENERIC; + struct itr_desc *desc = NULL; + + assert(chip && itr_num); + + /* "interrupts-extended" takes precedence over "interrupts" */ + if (fdt_getprop(fdt, node, "interrupts-extended", NULL)) + res = get_extended_interrupt_by_index(fdt, node, index, &desc); + else + res = get_legacy_interrupt_by_index(fdt, node, index, &desc); + + assert((!res && desc) || (res && !desc)); + + if (!res) { + *chip = desc->chip; + *itr_num = desc->itr_num; + + /* Balance malloc() or like from itr_dt_get_func callback */ + free(desc); + } + + return res; +} + +TEE_Result dt_get_interrupt_by_name(const void *fdt, int node, const char *name, + struct itr_chip **chip, size_t *itr_num) +{ + int idx = 0; + + idx = fdt_stringlist_search(fdt, node, "interrupt-names", name); + if (idx < 0) + return TEE_ERROR_GENERIC; + + return dt_get_interrupt_by_index(fdt, node, idx, chip, itr_num); +} +#endif /*CFG_DT*/ From 25a8de3308e9fc10112b808feba4cd14beb5048d Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Sun, 5 Feb 2023 03:24:56 +0100 Subject: [PATCH 2/5] drivers: gic: register to dt_driver Registers GIC driver as an interrupt controller in DT_DRIVER providers when DT is supported. This change allows interrupt consumer nodes to leverage interrupts and interrupts-extended properties DT bindings for their device drivers to retrieve their interrupts. Signed-off-by: Etienne Carriere --- core/drivers/gic.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/core/drivers/gic.c b/core/drivers/gic.c index 302a47e6ccc..f08e85f9691 100644 --- a/core/drivers/gic.c +++ b/core/drivers/gic.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -585,3 +586,61 @@ static void gic_op_set_affinity(struct itr_chip *chip, size_t it, gic_it_set_cpu_mask(gd, it, cpu_mask); } + +#ifdef CFG_DT +/* Callback for "interrupt-extended" GIC interrupts in consumer DT nodes */ +static struct itr_desc *dt_get_gic_chip_cb(struct dt_pargs *arg, + void *priv_data, TEE_Result *res) +{ + int itr_num = DT_INFO_INVALID_INTERRUPT; + struct itr_chip *chip = priv_data; + struct itr_desc *desc = NULL; + uint32_t type = 0; + uint32_t prio = 0; + + itr_num = gic_dt_get_irq(arg->args, arg->args_count, &type, &prio); + if (itr_num == DT_INFO_INVALID_INTERRUPT) { + *res = TEE_ERROR_GENERIC; + return NULL; + } + + /* Allocate returned itr_desc as required by itr_dt_get_func type */ + desc = calloc(1, sizeof(*desc)); + if (!desc) { + *res = TEE_ERROR_OUT_OF_MEMORY; + return NULL; + } + + gic_op_add(chip, itr_num, type, prio); + + desc->chip = chip; + desc->itr_num = itr_num; + + *res = TEE_SUCCESS; + return desc; +} + +static TEE_Result gic_probe(const void *fdt, int offs, const void *cd __unused) +{ + if (interrupt_register_provider(fdt, offs, dt_get_gic_chip_cb, + &gic_data.chip)) + panic(); + + return TEE_SUCCESS; +} + +static const struct dt_device_match gic_match_table[] = { + { .compatible = "arm,cortex-a15-gic" }, + { .compatible = "arm,cortex-a7-gic" }, + { .compatible = "arm,cortex-a5-gic" }, + { .compatible = "arm,cortex-a9-gic" }, + { .compatible = "arm,gic-400" }, + { } +}; + +DEFINE_DT_DRIVER(gic_dt_driver) = { + .name = "gic", + .match_table = gic_match_table, + .probe = gic_probe, +}; +#endif /*CFG_DT*/ From b0f96d77b8a0d8a5baa53c3fb91249dd44efa4e4 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Sun, 11 Jun 2023 23:30:34 +0200 Subject: [PATCH 3/5] [review] core: interrupt: registering interrupt providers Should rename the commit "interrupt: provider and consumer nodes Fixes s/inited/initialized/ Changes s/Provide/Provides/g for local description. Makes sense; Signed-off-by: Etienne Carriere --- core/include/kernel/interrupt.h | 4 ++-- core/kernel/interrupt.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/include/kernel/interrupt.h b/core/include/kernel/interrupt.h index f8ec9b5eb8b..2d8f8ca6b3c 100644 --- a/core/include/kernel/interrupt.h +++ b/core/include/kernel/interrupt.h @@ -409,7 +409,7 @@ TEE_Result interrupt_register_provider(const void *fdt, int node, * @itr_num Output interrupt number upon success * * Return TEE_SUCCESS in case of success - * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt controller is not yet inited + * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt driver is not yet initialized * Return TEE_ERROR_ITEM_NOT_FOUND if the DT does not reference target interrupt * Return any other TEE_Result compliant code in case of error */ @@ -427,7 +427,7 @@ TEE_Result dt_get_interrupt_by_index(const void *fdt, int node, * @itr_num Output interrupt number upon success * * Return TEE_SUCCESS in case of success - * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt controller is not yet inited + * Return TEE_ERROR_DEFER_DRIVER_INIT if interrupt driver is not yet initialized * Return TEE_ERROR_ITEM_NOT_FOUND if the DT does not reference target interrupt * Return any other TEE_Result compliant code in case of error */ diff --git a/core/kernel/interrupt.c b/core/kernel/interrupt.c index ac83ad0b501..46a59b1d18a 100644 --- a/core/kernel/interrupt.c +++ b/core/kernel/interrupt.c @@ -275,7 +275,7 @@ TEE_Result interrupt_register_provider(const void *fdt, int node, } /* - * Provide an itr_desc reference based on "interrupts" property bindings. + * Provides an itr_desc reference based on "interrupts" property bindings. * May return TEE_ERROR_DEFER_DRIVER_INIT if parent controller is found but * not yet initialized. */ @@ -324,7 +324,7 @@ static TEE_Result get_legacy_interrupt_by_index(const void *fdt, int node, } /* - * Provide an itr_desc based on "interrupts-extended" property bindings. + * Provides an itr_desc based on "interrupts-extended" property bindings. * May return TEE_ERROR_DEFER_DRIVER_INIT if parent controller is found * but not yet initialized. * With this function, provider is expected to have allocated itr_desc From 0ce2fee9d7bec977c87b3d1bb3c6bcc777c322e7 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Wed, 14 Jun 2023 21:36:58 +0200 Subject: [PATCH 4/5] [review] core: interrupt: registering interrupt providers Updates defined type itr_dt_get_func. Signed-off-by: Etienne Carriere --- core/include/kernel/interrupt.h | 20 ++++++++------------ core/kernel/interrupt.c | 25 ++++++++++++++++--------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/core/include/kernel/interrupt.h b/core/include/kernel/interrupt.h index 2d8f8ca6b3c..7cd6659fb10 100644 --- a/core/include/kernel/interrupt.h +++ b/core/include/kernel/interrupt.h @@ -364,24 +364,20 @@ void interrupt_remove_free_handler(struct itr_handler *hdl); * * @args Reference to phandle arguments * @data Pointer to data given at interrupt_register_provider() call - * @res Output result code of the operation: - * TEE_SUCCESS in case of success - * TEE_ERROR_DEFER_DRIVER_INIT if controller is not initialized - * Any TEE_Result compliant code in case of error. + * @out_itr_desc Output pointer to an allocated struct itr_desc upon success + * Return TEE_SUCCESS in case of success. + * Return TEE_ERROR_DEFER_DRIVER_INIT if controller is not initialized. + * Return another TEE_Result code otherwise. * - * Returns an allocated struct itr_desc pointer referencing to an interrupt - * or NULL if invalid description in which case @res provides the error - * code. The interrupt matches the reference found with properties of the - * pointer FDT node. + * Upon success, struct itr_desc must point to memory allocated with malloc() + * or like as it is freed prior returning to caller function. * * Upon success, the interrupt is configured and consumer can add a handler * function to the interrupt. Yet, the interrupt is not enabled until consumer * calls interrupt_enable(). - * Upon success, struct itr_desc must point to memory allocated with malloc() - * or like as it is freed prior returning to caller function. */ -typedef struct itr_desc *(*itr_dt_get_func)(struct dt_pargs *args, void *data, - TEE_Result *res); +typedef TEE_Result (*itr_dt_get_func)(struct dt_pargs *args, void *data, + struct itr_desc **out_itr_desc); #ifdef CFG_DT /** diff --git a/core/kernel/interrupt.c b/core/kernel/interrupt.c index 46a59b1d18a..358f0c56334 100644 --- a/core/kernel/interrupt.c +++ b/core/kernel/interrupt.c @@ -281,11 +281,12 @@ TEE_Result interrupt_register_provider(const void *fdt, int node, */ static TEE_Result get_legacy_interrupt_by_index(const void *fdt, int node, unsigned int index, - struct itr_desc **desc) + struct itr_desc **out_desc) { TEE_Result res = TEE_ERROR_GENERIC; const uint32_t *prop = NULL; uint32_t phandle = 0; + void *desc = NULL; int pnode = 0; int len = 0; @@ -316,10 +317,13 @@ static TEE_Result get_legacy_interrupt_by_index(const void *fdt, int node, phandle = fdt32_to_cpu(prop[0]); /* Get interrupt chip/number from phandle and "interrupts" property */ - *desc = dt_driver_device_from_node_idx_prop_phandle("interrupts", fdt, - node, index, - DT_DRIVER_INTERRUPT, - phandle, &res); + res = dt_driver_device_from_node_idx_prop_phandle("interrupts", fdt, + node, index, + DT_DRIVER_INTERRUPT, + phandle, desc); + if (!res) + *out_desc = desc; + return res; } @@ -332,13 +336,16 @@ static TEE_Result get_legacy_interrupt_by_index(const void *fdt, int node, */ static TEE_Result get_extended_interrupt_by_index(const void *fdt, int node, unsigned int index, - struct itr_desc **desc) + struct itr_desc **out_desc) { TEE_Result res = TEE_ERROR_GENERIC; + void *desc = NULL; - *desc = dt_driver_device_from_node_idx_prop("interrupts-extended", - fdt, node, index, - DT_DRIVER_INTERRUPT, &res); + res = dt_driver_device_from_node_idx_prop("interrupts-extended", + fdt, node, index, + DT_DRIVER_INTERRUPT, desc); + if (!res) + *out_desc = desc; return res; } From 3b5b1f6d8fb10b78531d20c24461adb3efa6aa43 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Wed, 14 Jun 2023 21:30:35 +0200 Subject: [PATCH 5/5] [review] drivers: gic: register to dt_driver Updates dt_get_gic_chip_cb(). Signed-off-by: Etienne Carriere --- core/drivers/gic.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/core/drivers/gic.c b/core/drivers/gic.c index f08e85f9691..72c2906f7b6 100644 --- a/core/drivers/gic.c +++ b/core/drivers/gic.c @@ -589,8 +589,8 @@ static void gic_op_set_affinity(struct itr_chip *chip, size_t it, #ifdef CFG_DT /* Callback for "interrupt-extended" GIC interrupts in consumer DT nodes */ -static struct itr_desc *dt_get_gic_chip_cb(struct dt_pargs *arg, - void *priv_data, TEE_Result *res) +static TEE_Result dt_get_gic_chip_cb(struct dt_pargs *arg, void *priv_data, + struct itr_desc **out_itr_desc) { int itr_num = DT_INFO_INVALID_INTERRUPT; struct itr_chip *chip = priv_data; @@ -599,25 +599,22 @@ static struct itr_desc *dt_get_gic_chip_cb(struct dt_pargs *arg, uint32_t prio = 0; itr_num = gic_dt_get_irq(arg->args, arg->args_count, &type, &prio); - if (itr_num == DT_INFO_INVALID_INTERRUPT) { - *res = TEE_ERROR_GENERIC; - return NULL; - } + if (itr_num == DT_INFO_INVALID_INTERRUPT) + return TEE_ERROR_GENERIC; /* Allocate returned itr_desc as required by itr_dt_get_func type */ desc = calloc(1, sizeof(*desc)); - if (!desc) { - *res = TEE_ERROR_OUT_OF_MEMORY; - return NULL; - } + if (!desc) + return TEE_ERROR_OUT_OF_MEMORY; gic_op_add(chip, itr_num, type, prio); desc->chip = chip; desc->itr_num = itr_num; - *res = TEE_SUCCESS; - return desc; + *out_itr_desc = desc; + + return TEE_SUCCESS; } static TEE_Result gic_probe(const void *fdt, int offs, const void *cd __unused)