interrupt chip controller drivers by etienne-lms · Pull Request #5954 · OP-TEE/optee_os

etienne-lms · 2023-04-12T10:42:21Z

This P-R proposes a interrupt (controller) chip framework and moves GIC driver to that framework.

These change allows to integrate interrupt chip that multiplex interrupts on an interrupt of a parent interrupt controller (as the GIC or another interrupt chip). Interrupt consumer drivers can use generic DT bindings (properties "interrupts" and "interrupts-extended") to get their interrupt resources.

These changes preserve exiting itr_xxx() API functions and friends related to the unique root CPU interrupt controller (likely GIC based on Arm arch). They add interrupt_xxx() functions where interrupt controller is an struct itr_chip, not specifically the GIC.

Comments are welcome. Later addition should bring some dt_driver tests means for this interrupt chip framework.

jenswikl · 2023-04-18T10:10:51Z

This is interesting. It would help with a picture and some description in the documentation to better understand how fits together.

I don't think we should need two sets of functions, with a good abstraction layer we should be able to have only one set of functions.

etienne-lms · 2023-04-18T15:35:48Z

It would help with a picture and some description in the documentation

Ok

I don't think we should need two sets of functions, with a good abstraction layer we should be able to have only one set of functions.

This series preserves legacy API functions itr_xxx() for backward compat of the prototypes... but maybe that's useless.
I could change the functions to assume that when argument struct itr_chip * is NULL, it relates to the root CPU interrupt controller, unless one thinks it is an ugly shortcut.

jenswikl · 2023-04-18T15:58:57Z

I think we should make the API what we'd like it to be, that way it's easier to understand and use. If we need to change all the users in the tree, so be it.

etienne-lms · 2023-05-07T18:10:57Z

Rebased on matser tip.

I've created OP-TEE/optee_docs#192 to address #5954 (comment) about documentation.

etienne-lms · 2023-05-08T06:34:48Z

This series is not ready to use. I did not address deprecation of legacy function itr_xxx() and there interrupt configuration means must be refined. Please consider this is still work in progress.
I plan to squash all fixup commits in my next update.

etienne-lms · 2023-05-10T11:32:40Z

By the way, I think config switch CFG_DRIVERS_INTERRUPT is useless. Added code for interrupt binding through DT can be confiditionned by CFG_DT only.

etienne-lms · 2023-05-24T13:49:41Z

I have updated the series, squashing the fixup commits, changing all itr_core_ label prefixes to interrupt_main_ label prefixes. The 2 last commits remove old itr_xxx() functions, leaving only the new interrupt_xxx() API functions that can be used with main interrupt controller (as the GIC) or a secondary interrupt controller.

Note the serie introduces dt_driver support for description of interrupt controller chains from a DT blob.

The series is still long, at least it gives the big picture. If we agree on moving forward, i'll extract the first commits from this series and create dedicated P-Rs to ease step by step review.

etienne-lms · 2023-05-31T07:33:46Z

May I rebase on mater tip and squash the fixup commits?

Once done, as the series is quite long (19 patches), maybe I should create smaller P-Rs to ease review.

jenswikl · 2023-05-31T08:14:03Z

May I rebase on mater tip and squash the fixup commits?

Yes, please.

etienne-lms · 2023-05-31T10:44:31Z

Fixup commit squashed and series rebased on master branch tip.

etienne-lms · 2023-05-31T10:45:24Z

But I see I missed a lot of comments I posted.
I'll fix that and force push again...

etienne-lms · 2023-05-31T10:58:16Z

Series is now back ready for review.

jenswikl · 2023-05-31T14:59:57Z

For "core: interrupt: rename internal itr_chip to itr_main_chip" please apply:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

For "core: dt_driver: define interrupt controller drivers identifier" please apply:
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Comment on commit message for "drivers: move to interrupt_call_handlers()":
Replace "Deprecates itr_handle()" with "Removes itr_handle()"

For the entire PR:
s/interrupt line/interrupt/g where you can't find a better word than "line".

etienne-lms · 2023-05-31T16:14:23Z

s/interrupt line/interrupt/g where you can't find a better word than "line".

Okay, i'll go for 'interrupt'.

jenswikl · 2023-06-01T09:30:03Z

Feel free to squash in the updates.

etienne-lms · 2023-06-01T10:32:11Z

Sorry I missed your last comment.
Ok, i'll squash commits.

etienne-lms · 2023-06-05T20:44:44Z

I've squashed the fixup commits and applied review tag on 3 commits (#5954 (review), #5954 (review) and #5954 (review)). The previously reviewed series can be found here: branch itr-chip-prev.

For commit: "core: interrupt: interrupt chip framework", I updated the commit message according to #5954 (review) and #5954 (comment)) and I addressed review comments with an new appended 1st fixup commit:
"[review] core: interrupt: interrupt chip framework"

Fixed Atmel driver issues (#5954 (comment)) with 2 fixup commits:
"[review] drivers: atmel_piobu: upgrade to new interrupt framework"
"[review] drivers: atmel_wdt: upgrade to new interrupt framework"

I appended 3 new commits to remove the old interrupt API functions itr_xxx():
"[review] plat-stm32mp1: upgrade to new interrupt framework"
"core: notif: upgrade to new interrupt framework"
"core: interrupt: remove old API functions"

Last, I appendeded another fixup commit "[review] core: interrupt: interrupt chip framework" to mask unhandled interrupt instead of disabling them. This is needed as mask operation handler is expected to be atomic while disable operation handler does not.

@tprrt: cuold you test this updated series? Atmel SAIC controller shoulld be provided the expect configuration data now.

etienne-lms · 2023-06-05T20:46:17Z

Should I create smaller P-Rs to help this series (~22 commits) review?

Rename dt_register_interrupt_provider() to interrupt_register_provider() for consistency with other function. Rename type dt_get_itr_func to itr_dt_get_func for consistency with other dt driver callback function types. Fix buggy reference to clock in interrupt API function inline description comments. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Struct dt_get_itr_func is renamed struct itr_dt_get_func. dt_register_interrupt_provider() is renamed interrupt_register_provider(). Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

jenswikl

Comment for "core: dt_driver: add helper for old fashion interrupt bindings". With that addressed
please apply:
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

jenswikl

Comment for "core: dt_driver: add helper for old fashion interrupt bindings". With that addressed
please apply:
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

jenswikl · 2023-06-06T06:08:52Z

For commit "core: interrupt: interrupt_get_main_chip() returns main controller":
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

jenswikl · 2023-06-06T06:15:35Z

Should I create smaller P-Rs to help this series (~22 commits) review?

Sure, feel free to chip off a few patches. :-)

tprrt · 2023-06-06T08:09:44Z

I've squashed the fixup commits and applied review tag on 3 commits (#5954 (review), #5954 (review) and #5954 (review)). The previously reviewed series can be found here: branch itr-chip-prev.

For commit: "core: interrupt: interrupt chip framework", I updated the commit message according to #5954 (review) and #5954 (comment)) and I addressed review comments with an new appended 1st fixup commit: "[review] core: interrupt: interrupt chip framework"

Fixed Atmel driver issues (#5954 (comment)) with 2 fixup commits: "[review] drivers: atmel_piobu: upgrade to new interrupt framework" "[review] drivers: atmel_wdt: upgrade to new interrupt framework"

I appended 3 new commits to remove the old interrupt API functions itr_xxx(): "[review] plat-stm32mp1: upgrade to new interrupt framework" "core: notif: upgrade to new interrupt framework" "core: interrupt: remove old API functions"

Last, I appendeded another fixup commit "[review] core: interrupt: interrupt chip framework" to mask unhandled interrupt instead of disabling them. This is needed as mask operation handler is expected to be atomic while disable operation handler does not.

@tprrt: cuold you test this updated series? Atmel SAIC controller shoulld be provided the expect configuration data now.

Hello @etienne-lms,
It's good now, no more issue occurs on sama5.
Kind regards,
Thomas

Adds missing test of fdt_getprop() returned error code or property size. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

etienne-lms · 2023-06-06T09:27:18Z

thanks @jenswi-linaro for the review.
thanks @tprrt for the test.
I've added few more fixup commits. I'll create P-Rs for the 1st patches of this series.

etienne-lms · 2023-06-06T09:49:34Z

I have created #6087 and #6088 for the 5 first commits of the current series.

Return TEE_ERROR_ITEM_NOT_FOUND instead of TEE_ERROR_GENERIC when the interrupt index used by consumer is too high regarding the number of interrupts describe in the consumer node. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

Adds a debug trace when platform mixes shared and non-shared handlers on the same interrupt. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

etienne-lms · 2023-06-07T19:48:24Z

I created 2 P-Rs:
#6096: 5 commits for the new interrupt API functions interrupt_xxx()
#6097: 2 commits for the dt_driver registering/retrieving

Will remain the updates in few drivers and platform code to bump to the new API and a last cmmit to remove the old interrupt API functions itr_xxx().

etienne-lms · 2023-06-14T09:55:56Z

As #6096 has been merged, I created #6113 to finalize the new interrupt API.

The part related to use of DT means to register interrupt driver providers and for consumer to get their interrupt is still pending: once #6097 is merged, I will create a P-R for a last commit to make GIC driver to register as a DT_DRIVER_INTERRUPT.

Once, done, all change proposed in this P-R will have been addressed.

The next step regarding interrupt management wil likely be support for bottom half interrupt handlers, see OP-TEE/optee_docs#198.

ldebieve · 2023-06-20T07:20:41Z

+
+	assert(hdl && hdl->chip->ops);
+
+	SLIST_FOREACH(h, &hdl->chip->handlers, link) {


Should it be a SAFE loop has the remove will update the list?

This part was merged with #6096.
That said, I agree, theree should be some protection here in case interrupt handlers are added/removed at runtime.

github-actions · 2023-07-21T01:46:07Z

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

etienne-lms mentioned this pull request Apr 12, 2023

Support for multiple interrupts per device node #5831

Closed

etienne-lms mentioned this pull request May 7, 2023

core: using native interrupt handlers OP-TEE/optee_docs#192

Closed

etienne-lms changed the title ~~[RFC] interrupt chip controller drivers~~ interrupt chip controller drivers May 7, 2023

etienne-lms force-pushed the itr-chip branch from 093dff9 to 64d4012 Compare May 7, 2023 18:09

etienne-lms mentioned this pull request May 17, 2023

core: interrupt: factorization & renaming #6030

Merged

etienne-lms force-pushed the itr-chip branch from ac37b3e to a1555a2 Compare May 24, 2023 13:42

etienne-lms commented May 25, 2023

View reviewed changes

etienne-lms mentioned this pull request May 31, 2023

(take 2) NS Interrupt notification over async notif #5793

Closed

etienne-lms force-pushed the itr-chip branch 2 times, most recently from de8438c to bc63a33 Compare May 31, 2023 10:42

etienne-lms force-pushed the itr-chip branch from bc63a33 to 017ea65 Compare May 31, 2023 10:57

jenswikl reviewed May 31, 2023

View reviewed changes

etienne-lms commented Jun 1, 2023

View reviewed changes

Comment thread core/arch/arm/plat-vexpress/main.c Outdated

etienne-lms commented Jun 1, 2023

View reviewed changes

Comment thread core/include/kernel/interrupt.h Outdated

etienne-lms added 2 commits June 5, 2023 22:55

[review] drivers: gic: get gic interrupt from device tree

d5bc329

Struct dt_get_itr_func is renamed struct itr_dt_get_func. dt_register_interrupt_provider() is renamed interrupt_register_provider(). Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

jenswikl reviewed Jun 6, 2023

View reviewed changes

Comment thread core/kernel/dt_driver.c

jenswikl reviewed Jun 6, 2023

View reviewed changes

[review] core: dt_driver: add helper for old fashion interrupt bindings

17537c1

Adds missing test of fdt_getprop() returned error code or property size. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

etienne-lms commented Jun 6, 2023

View reviewed changes

Comment thread core/kernel/dt_driver.c Outdated

This was referenced Jun 6, 2023

itr_chip drivers: get main chip + main chip renaming + add inline descriptions #6087

Merged

itr_chip drivers: dt_driver resources for interrupts bindings #6088

Merged

etienne-lms added 2 commits June 6, 2023 13:53

[review] core: interrupt: interrupt chip framework

c6bec8e

Adds a debug trace when platform mixes shared and non-shared handlers on the same interrupt. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

etienne-lms force-pushed the itr-chip branch from e9faf20 to c6bec8e Compare June 6, 2023 11:53

This was referenced Jun 7, 2023

core: interrupt: new interrupt API functions #6096

Merged

core: interrupt: dt_driver providers/consumers #6097

Closed

This was referenced Jun 8, 2023

core: dt_driver: swap TEE_result and retrieved device reference #6098

Merged

core: interrupt: move to new interrupt API functions and remove the old ones #6113

Closed

etienne-lms marked this pull request as draft June 14, 2023 09:50

ldebieve reviewed Jun 20, 2023

View reviewed changes

github-actions Bot added the Stale label Jul 21, 2023

github-actions Bot closed this Jul 26, 2023

etienne-lms deleted the itr-chip branch January 8, 2025 07:53


		assert(hdl && hdl->chip->ops);

		SLIST_FOREACH(h, &hdl->chip->handlers, link) {

Uh oh!

Conversation

etienne-lms commented Apr 12, 2023

Uh oh!

jenswikl commented Apr 18, 2023

Uh oh!

etienne-lms commented Apr 18, 2023

Uh oh!

jenswikl commented Apr 18, 2023

Uh oh!

etienne-lms commented May 7, 2023

Uh oh!

etienne-lms commented May 8, 2023

Uh oh!

etienne-lms commented May 10, 2023

Uh oh!

etienne-lms commented May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

etienne-lms commented May 31, 2023

Uh oh!

jenswikl commented May 31, 2023

Uh oh!

etienne-lms commented May 31, 2023

Uh oh!

etienne-lms commented May 31, 2023

Uh oh!

etienne-lms commented May 31, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jenswikl commented May 31, 2023

Uh oh!

etienne-lms commented May 31, 2023

Uh oh!

Uh oh!

Uh oh!

jenswikl commented Jun 1, 2023

Uh oh!

etienne-lms commented Jun 1, 2023

Uh oh!

etienne-lms commented Jun 5, 2023

Uh oh!

etienne-lms commented Jun 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenswikl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jenswikl left a comment

Choose a reason for hiding this comment

Uh oh!

jenswikl commented Jun 6, 2023

Uh oh!

jenswikl commented Jun 6, 2023

Uh oh!

tprrt commented Jun 6, 2023

Uh oh!

etienne-lms commented Jun 6, 2023

Uh oh!

Uh oh!

etienne-lms commented Jun 6, 2023

Uh oh!

etienne-lms commented Jun 7, 2023

etienne-lms commented May 24, 2023 •

edited

Loading

etienne-lms commented Jun 5, 2023 •

edited

Loading