Skip to content

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

Closed
etienne-lms wants to merge 12 commits into
OP-TEE:masterfrom
etienne-lms:itr-chip-1
Closed

core: interrupt: move to new interrupt API functions and remove the old ones#6113
etienne-lms wants to merge 12 commits into
OP-TEE:masterfrom
etienne-lms:itr-chip-1

Conversation

@etienne-lms

Copy link
Copy Markdown
Contributor

10 patches extracted from #5954 + a fixup patch (for series 1st commit) to finalise new interrupt management API functions that can be used on CPU main interrupt controller as well as on secondary interrupt controllers.

This series makes main controller to use generic resources, then it moves each interrupt consumer to the new API functions, last the series removes old interrupt API functions itr_*().

1st commit (("drivers: move to interrupt_call_handlers()") has a review tag applied but I appended a fixup commit for some rewording in inline description comments.

@etienne-lms etienne-lms changed the title Itr chip 1 core: interrupt: move to new interrupt API functions and remove the old ones Jun 14, 2023
@jenswikl

Copy link
Copy Markdown
Contributor

If you don't mind, please wait with "[review] drivers: move to interrupt_call_handlers" until we see how the bottom half handlers will play out.

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Ok, i'll drop the fixup commit.

Removes itr_handle() in favor to interrupt_call_handlers(). This
changes updates all implemented main interrupt controller drivers that
are the GIC driver, the HFIC driver and Atmel SAIC driver.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@etienne-lms

Copy link
Copy Markdown
Contributor Author

@borneoa, you may want to have a look at this series.

Comment thread core/drivers/atmel_wdt.c Outdated
Comment thread core/drivers/atmel_wdt.c Outdated
Comment thread core/kernel/notif.c Outdated
Comment thread core/kernel/notif.c
@etienne-lms

Copy link
Copy Markdown
Contributor Author

Updated with interrupt_raise_pi() and friends API functions defined in interrupt.h.

@jenswikl

Copy link
Copy Markdown
Contributor

For commit "core: interrupt: helper function for raise_pi, raise_sgi, set_affinity", please apply:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

@etienne-lms

Copy link
Copy Markdown
Contributor Author

May I squash the fixup commit and reorder the patches?

@jenswikl

Copy link
Copy Markdown
Contributor

May I squash the fixup commit and reorder the patches?

Yes, please.

@etienne-lms

etienne-lms commented Jun 22, 2023

Copy link
Copy Markdown
Contributor Author

Fixup commits squashed and commits reordered.
@tprrt, could you please test this change series?

Comment thread core/drivers/atmel_wdt.c Outdated
irq_type, irq_prio);
if (!it_hdlr)
goto err_free_wdt;
it_hdlr = calloc(1, sizeof(*it_hdlr));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice with an alternative to interrupt_alloc_add_handler() which also takes irq_type and irq_prio into account. I wouldn't mind an interrupt_alloc_add_handler2() to take care of that, or interrupt_alloc_add_conf_handler() if you don't mind even longer function names.

@jenswikl

jenswikl commented Jun 22, 2023

Copy link
Copy Markdown
Contributor

For the commit "core: notif: upgrade to new interrupt framework"
s/has old/as the old/
with that fixed:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

For the commits:

  • "drivers: atmel_wdt: upgrade to new interrupt framework"
  • "drivers: crypto: caam: upgrade to new interrupt framework"
  • "plat-vexpress: upgrade to new interrupt framework"
  • "plat-synquacer: upgrade to new interrupt framework"
  • "plat-stm32mp1: upgrade to new interrupt framework"
  • "core: interrupt: remove old API functions"

Please apply:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Comment for adding an interrupt_alloc_add_conf_handler() helper function addressed.

etienne-lms and others added 11 commits June 22, 2023 23:45
Adds interrupt API function interrupt_alloc_add_conf_handler() to
allocate, configure and register an interrupt handler, providing
interrupt type and priority.

Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Defines helper API functions to call .raise_pi, .raise_sgi and
.set_affinity handlers of a chip controller. Defines API function
to query support of these handlers in the interrupt controller.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Uses main controller ops to call .raise_sgi as the old API functions
itr_xxx() are deprecated.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves atmel_wdt watchdog driver to the new interrupt framework
API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves CAAM job ring driver to the new interrupt framework API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Upgrades sp805_wdt.c driver to the new interrupt API functions as
itr_alloc_add() and friends will be removed.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves atmel_piobu driver to the new interrupt framework API functions.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves plat-vexpress to the new interrupt framework API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves plat-synquacer to the new interrupt framework API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Moves plat-stm32mp1 to the new interrupt framework API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Remove old itr_xxx() API functions.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@etienne-lms

Copy link
Copy Markdown
Contributor Author

Fixed commit message (#6113 (comment)) and applied review tags to relevant patches (8/11).

@tprrt

tprrt commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Hello,

Fixup commits squashed and commits reordered. @tprrt, could you please test this change series?

I just tested there is a boot issue on sama5d27, with 3f99a0fa3a1d:

E/TC:0 0 assertion '!res' failed at core/drivers/clk/clk_dt.c:161 <parse_assigned_clock>
E/TC:0 0 Panic at core/kernel/assert.c:28 <_assert_break>
E/TC:0 0 TEE load address @ 0x20000000
E/TC:0 0 Call stack:
E/TC:0 0  0x20005b89 print_kernel_stack at core/arch/arm/kernel/unwind_arm32.c:109
E/TC:0 0  0x2001bfc5 __do_panic at core/kernel/panic.c:31
E/TC:0 0  0x20017ef9 _assert_break at core/kernel/assert.c:28
E/TC:0 0  0x200595f7 _assert_trap at lib/libutils/isoc/include/assert.h:20
E/TC:0 0  0x20012cc9 parse_assigned_clock at core/drivers/clk/clk_dt.c:161 (discriminator 1)
E/TC:0 0  0x20012e61 clk_probe_assigned at core/drivers/clk/clk_dt.c:198
E/TC:0 0  0x20012e33 clk_probe_assigned at core/drivers/clk/clk_dt.c:191
E/TC:0 0  0x20012e33 clk_probe_assigned at core/drivers/clk/clk_dt.c:191
E/TC:0 0  0x20012eef clk_dt_probe at core/drivers/clk/clk_dt.c:212
E/TC:0 0  0x2001dd19 call_initcalls at core/kernel/initcall.c:41
E/TC:0 0  0x20054233 init_tee_runtime at core/arch/arm/kernel/boot.c:1379
E/TC:0 0  0x200058d5 boot_init_primary_late at core/arch/arm/kernel/boot.c:1485
E/TC:0 0  0x200001ec clear_bss at core/arch/arm/kernel/entry_a32.S:565

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Thansk @tprrt for the tests. Is the boot issue you found related to this P-R? I find it strange that clock drivers probinding fail due to changes in the interrupt framework. Can you check that sama5d27 boots well on current OP-TEE OS master branch tip?

@tprrt

tprrt commented Jun 24, 2023

Copy link
Copy Markdown
Contributor

Hello

Thansk @tprrt for the tests. Is the boot issue you found related to this P-R? I find it strange that clock drivers probinding fail due to changes in the interrupt framework. Can you check that sama5d27 boots well on current OP-TEE OS master branch tip?

I agree, the issue isn’t related to this pull-request, I reproduced with the master branch (31a550c).

Kind regards,
Thomas

@etienne-lms

Copy link
Copy Markdown
Contributor Author

@tprrt, can you bisect into master branch commits to locate (and possibly fix) the faulty commit?

@tprrt

tprrt commented Jun 26, 2023

Copy link
Copy Markdown
Contributor

Hello,

@tprrt, can you bisect into master branch commits to locate (and possibly fix) the faulty commit?

Yes, I will.

@tprrt

tprrt commented Jul 6, 2023

Copy link
Copy Markdown
Contributor

Hello,

Sorry, I have a very busy schedule, I will do my best to apply required changes next week.

Kind regards,
Thomas

@github-actions

github-actions Bot commented Aug 6, 2023

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the Stale label Aug 6, 2023
@jforissier jforissier removed the Stale label Aug 6, 2023
@github-actions

github-actions Bot commented Sep 6, 2023

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants