enable irqsteer for m33 of imx943 #93760
Conversation
zephyrbot
added
area: Interrupt Controller
platform: NXP
zephyrbot
requested review from
bperseghetti,
dbaluta,
dcpleung,
decsny,
dleach02,
EmilioCBen,
iuliana-prodan,
JiafeiPan,
manuargue,
mmahadevan108,
nashif,
PetervdPerk-NXP,
yangbolu1991,
ycsin and
Zhiqiang-Hou
| @@ -44,6 +44,7 @@ zephyr_library_sources_ifdef(CONFIG_NXP_PINT intc_nxp_pint.c) | |||
| zephyr_library_sources_ifdef(CONFIG_RENESAS_RX_ICU intc_renesas_rx_icu.c) | |||
| zephyr_library_sources_ifdef(CONFIG_RENESAS_RZ_EXT_IRQ intc_renesas_rz_ext_irq.c) | |||
| zephyr_library_sources_ifdef(CONFIG_NXP_IRQSTEER intc_nxp_irqsteer.c) | |||
| zephyr_library_sources_ifdef(CONFIG_NXP_IRQSTEER1 intc_nxp_irqsteer1.c) | |||
There was a problem hiding this comment.
Please explain why do you need a new driver?
Why the intc_nxp_irqsteer.c driver is not enough?
In the commit message you state that the "old" intc_nxp_irqsteer driver can be replaced.
In this case, why don't you enhance the intc_nxp_irqsteer driver with the support that you need for i.MX943?
There was a problem hiding this comment.
Yes, it's a good question.
Because of the new structure of irqsteer(https://github.com/zephyrproject-rtos/hal_nxp/blob/master/mcux/mcux-sdk-ng/devices/i.MX/i.MX943/periph/PERI_IRQSTEER.h#L244) is different with the old structure of irqsteer(https://github.com/zephyrproject-rtos/hal_nxp/blob/master/mcux/mcux-sdk-ng/devices/i.MX/i.MX95/periph/PERI_IRQSTEER.h#L145),
The new driver doesn't depend on the structure IRQSTEER_TYPE. It can be used in all of platforms.(i.mx8, i.mx95, i.mx943. Include irqsteer instance dpu_irqsteer, m33_irqsteer, m7_irqsteer, a55_irqsteer, dsp_irqsteer, hdmi_irqsteer)
There was a problem hiding this comment.
@biwenli so your proposal is to replace the "old" irqsteer driver with the new one?
Usually, when such a thing is happening you need to update all the references to the old driver with the new one. And as last point remove the old driver.
There was a problem hiding this comment.
@biwenli so your proposal is to replace the "old" irqsteer driver with the new one?
Usually, when such a thing is happening you need to update all the references to the old driver with the new one. And as last point remove the old driver.
Yes, the old driver will be phased out over time. New product will use the new driver directly.
There was a problem hiding this comment.
@biwenli Could you please add a note indicating that the old irqsteer will be deprecated and should no longer be used in new samples or products?
Also, it would be helpful to include at least one commit where the old irqsteer is replaced with the new implementation, so others can refer to it as an example.
Thanks.
There was a problem hiding this comment.
@biwenli Could you please add a note indicating that the old
irqsteerwill be deprecated and should no longer be used in new samples or products? Also, it would be helpful to include at least one commit where the oldirqsteeris replaced with the new implementation, so others can refer to it as an example. Thanks.
Sure, np. Thanks.
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
from
2112635 to
1f02269
Compare
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
|
@biwenli you will also need to convert at least 1 user of the old driver to test your new implemenation. Ideally would be to convert all users, but we can do it in phases as we don't want to slow you down. |
The test case can be used to test the new irqsteer1 driver, |
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
from
0d4c5c6 to
f71c690
Compare
| @@ -22,23 +22,23 @@ | |||
| compatible = "nxp,irqsteer-master"; | |||
| reg = <0>; | |||
| interrupt-controller; | |||
There was a problem hiding this comment.
Please provide more context in your commit messages. One year later a poor soul who tries to understand this patch wont find any useful info :)
A commit message should always explain why the changes was done, with a little bit of context of how if the code is complicated.
What is done like update interrrupt-cells should be obvious from the code.
There was a problem hiding this comment.
Please provide more context in your commit messages. One year later a poor soul who tries to understand this patch wont find any useful info :)
A commit message should always explain why the changes was done, with a little bit of context of how if the code is complicated.
What is done like
update interrrupt-cellsshould be obvious from the code.
sure, np.
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
2 times, most recently
from
d6a8381 to
0ebc560
Compare
|
zephyr/zephyr/samples/net/sockets/http_server/src/main.c:22: fatal error: sample_usbd.h: No such file or directory This build issue is blocked by the commit id, 
@jfischer-no |
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
from
0ebc560 to
6ef6236
Compare
Let's try to reuse the code. |
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
from
6ef6236 to
8ca2ad2
Compare
biwenli
changed the title
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
2 times, most recently
from
45230fc to
bd989b7
Compare
Enable irqsteer for m33 of imx943 Signed-off-by: Biwen Li <[email protected]>
Enable edma4 by default for m33(in NETCMIX) of imx943_evk Signed-off-by: Biwen Li <[email protected]>
Update interrupt-cells of irqsteer master
- DT_INST_IRQ_BY_IDX(n, idx, priority) is used to
get priority attribute of interrupt in some drivers,
so add priority for irqsteer master to align priority
attribute.
Signed-off-by: Biwen Li <[email protected]>
Update interrupt-cells of imx8, imx8m, imx8qm, imx8qxp. - add priority attribute to fix build issue Signed-off-by: Biwen Li <[email protected]>
Update interrupt-cells for imx95_m7 - Add priority attribute to fix build issue. Signed-off-by: Biwen Li <[email protected]>
Update interrupt-cells of imx8mp_evk_mimx8ml8_adsp - Add priority attribute to fix build issue. Signed-off-by: Biwen Li <[email protected]>
Correct irq number for multi level interrupts Signed-off-by: Biwen Li <[email protected]>
a. Convert address for dma, unless dma will access reserved address. It will cause lockup issue. b. Print SADDR, DADDR for debugging. Signed-off-by: Biwen Li <[email protected]>
Add config for imx943_evk_mimx94398_m33 - Use the sample to test the irqsteer driver Signed-off-by: Biwen Li <[email protected]>
This commit fixes build issue when multi level interrupts is enabled,
define TEST_IRQ_NUM as (CONFIG_2ND_LVL_ISR_TBL_OFFSET - 1)
- CMSIS/Core/Include/core_cm33.h:2438:15: error: array subscript 24
is above array bounds of volatile uint32_t[16]
Signed-off-by: Biwen Li <[email protected]>
This commit fixes build issue when multi level interrupts feature
is enabled, define new macro TEST_1ST_LEVEL_INTERRUPTS_MAX to
support multi level interrupts case.
- error: array subscript 24 is above array bounds of 'volatile
uint32_t[16]' {aka 'volatile unsigned int[16]'}
[-Werror=array-bounds])
Signed-off-by: Biwen Li <[email protected]>
Fix build issue when multi level interrupts is enabled,
define new macro TEST_1ST_LEVEL_INTERRUPTS_MAX to support multi
level interrupts case.
- core_cm33.h:2559:47: error: iteration 496 invokes undefined behavior
[-Werror=aggressive-loop-optimizations]
Signed-off-by: Biwen Li <[email protected]>
Exclude imx943,
- multi level interrupts feature is enabled defaultly,
so irqsteer1 driver is enabled, then the test case
report the build issue:
multiple definition of z_soc_irq_enable,
tests/arch/arm/arm_custom_interrupt/src/arm_custom_interrupt.c:47
first defined here.
Signed-off-by: Biwen Li <[email protected]>
biwenli
force-pushed
the
imx943-add-new-irqsteer1-driver
branch
from
bd989b7 to
f33b470
Compare
|
enable irqsteer for imx943.