-
Notifications
You must be signed in to change notification settings - Fork 19
examples: legacy_apps: xlnx: Remove xlnx_hw_to_bsp_irq() #94
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
base: main
Are you sure you want to change the base?
examples: legacy_apps: xlnx: Remove xlnx_hw_to_bsp_irq() #94
Conversation
The Lopper-generated amd_platform_info.h already accounts for BSP. So remove the unneeded function xlnx_hw_to_bsp_irq() which tries to account for IRQ system number difference from baremetal (generic) and FreeRTOS BSPs. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
|
@bentheredonethat lopper is not widely used yet. So we should handle both cases i.e. with and without lopper. Can we implement a way to know if generated header file is used or hard-code definition from the header file ? |
|
Hi @tnmysh yes its the AMD_GENERATED header file symbol, no? In other words: If we want some extra awareness without Lopper, why not push that to BSP logic? e.g. metal_xlnx_extension |
@bentheredonethat good point. Yes I had tried it. But that doesn't work because the zynqmp driver ops is also using the irq_info. openamp-system-reference/examples/legacy_apps/machine/xlnx/zynqmp/zynqmp_linux_r5_proc.c Line 138 in 4bdabbb
If we move irq conversion logic to BSP, then irq_info above is not passing correct irq information, and interrupt handler is not called. |
|
Hi @tnmysh thanks for the comment -- I might be missing something but I dont think 'zynqmp' area applies.
So it seems to me like this commit does not conflict at all but again I might be missing something here. |
This file is for r5: https://github.com/OpenAMP/openamp-system-reference/blob/4bdabbb5b6aaaa902f56d408c091dbefc11e62ca/examples/legacy_apps/machine/xlnx/zynqmp/zynqmp_linux_r5_proc.c It maps irq_info interrupt number to isr routine in the *_init function.
|
|
@tnmysh Im still confused. Historically the 'zynqmp' subdir is for linux. you can see that a few ways:
also as further reasoning (said this in earlier comment) - there is no mention of the routine in the init function for this subdir https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp/zynqmp_linux_r5_proc.c#L66 can you please clarify how the routine xlnx_hw_to_bsp_irq() is being called in this subdir or why this commit causes an issue? I understand that xlnx_hw_to_bsp_irq() was setting up the IRQ INFO based on BSP. But for the zynqmp/linux subdir - this is not called. The routine xlnx_hw_to_bsp_irq() is presently called only from xlnx_machine_init(). The routine xlnx_machine_init() is only called in https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp_r5/platform_info.c. So from my looking through this - there is no link to the 'zynqmp' subdir that affects the removal of the xlnx_hw_to_bsp_irq() call. Also the Lopper-flow is fully upstream and the README at https://github.com/OpenAMP/openamp-system-reference/tree/main/examples/legacy_apps/machine/xlnx shows how to generate the Lopper header file. |
The Lopper-generated amd_platform_info.h already accounts for BSP. So remove the unneeded function xlnx_hw_to_bsp_irq() which tries to account for IRQ system number difference from baremetal (generic) and FreeRTOS BSPs.