-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
arch: sw_isr: store device info in the table and add funtions to access #63172
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should NOT add a property for has-multi-instance
to the devicetree, but scan the tree in some way to get the count of instances to determine its greater than 1.
f66a225
to
9d7918d
Compare
@cfriedt @galak the This implementation doesn't support zephyr/dts/riscv/openisa/rv32m1.dtsi Lines 88 to 177 in b4da11f
I've updated the filter to grab the interrupt controller that is the direct child of |
76262c9
to
c455987
Compare
c455987
to
2bc94ef
Compare
2bc94ef
to
d6701ed
Compare
Change the internal function to `get_parent_entry`, which returns the entire entry of table. Store the parent interrupt controller device in the `irq_parent_offset` table, and added 2 helper functions to: 1. determine the parent interrupt controller based on the IRQ 2. determine the IRQ of the parent interrupt controller Declare the `struct _irq_parent_entry` in the header and added `-` suffix to the struct so that it can be used to test the functions in testsuites. Signed-off-by: Yong Cong Sin <[email protected]>
Validate the following functions in the sw_isr_table: - z_get_sw_isr_table_idx - z_get_sw_isr_device_from_irq - z_get_sw_isr_irq_from_device Signed-off-by: Yong Cong Sin <[email protected]>
Relocate new and existing internal software-managed table access functions from the public `sw_isr_table.h` into a private header that should only be accessed internally. Signed-off-by: Yong Cong Sin <[email protected]>
398e0ad
to
07dff11
Compare
Refactor multi-level IRQ related code from `sw_isr_common.c` to `multilevel_irq.c` to simplify `sw_isr_common` & macrologies. Signed-off-by: Yong Cong Sin <[email protected]>
Instead of using a macro guard to prevent functions in `sw_isr_common.c` from getting compiled when `CONFIG_DYNAMIC_INTERRUPTS` isn't enabled, do that in `CMakeLists.txt` instead. Signed-off-by: Yong Cong Sin <[email protected]>
07dff11
to
156a642
Compare
Convert `#if defined CONFIG_*` to use `if (IS_ENABLED(CONFIG_*))` and removed some macro guards since they are safe to be compile. Signed-off-by: Yong Cong Sin <[email protected]>
aab4338
to
dd604b2
Compare
|
||
#ifdef CONFIG_3RD_LEVEL_INTERRUPTS | ||
|
||
const struct _irq_parent_entry _lvl3_irq_list[CONFIG_NUM_3RD_LEVEL_AGGREGATORS] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be conditionally defined?
E.g. can we remove the ifdef above, use if (IS_ENABLED())
elsewhere, and let the linker discard unused code and data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it fails to compile as the initialization used some undefined macros when 3rd level isn't enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from the perspective of #62907.
@dcpleung can you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH... we should probably start looking into DAG or tree-like hierarchy for multi-level interrupts so we can avoid making this even complicated in the future.
Where can I read more about this? Any existing implementation? |
Maybe the Linux kernel? The idea is (probably) simple, where each interrupt controller is a node in a tree. For multi-level interrupts, each node is a child of some other node (except the root node). So you can have arbitrary levels of interrupts, and each controller can trace back to the root controller. Though, this is an overhaul of interrupt handling and is going to be a huge project... for soul(s) brave enough to tackle it. |
Yep. Linux would suffice. Of course, it's mostly about the model. The model will also drastically simplify shared interrupts with a marginal increase in complexity. It's also more stateful. Zephyr's multi-level model (maybe surprisingly) started out simple too, of course 🤪 |
Change the internal function to
get_parent_entry
, which returns the entire entry of table.Store the parent interrupt controller device in the
irq_parent_offset
table, and added 2 helper functions to:Declare the
struct _irq_parent_entry
in the header and added so that it can be used to test the functions in testsuites.Note
This change will increase the LUT size for build with multi-level enabled by
CONFIG_NUM_x_LEVEL_AGGREGATORS * sizeof(struct device)
Note
This PR is spined off from #62907, to hopefully make it easier to review, and is required to fix #63465
Important
Blocking #62907