Skip to content

Conversation

@bentheredonethat
Copy link
Contributor

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.

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>
@tnmysh
Copy link
Collaborator

tnmysh commented Jan 30, 2026

@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 ?
I remember we had AMD_GENERATED macro passed previously. Can we use it?

@bentheredonethat
Copy link
Contributor Author

bentheredonethat commented Jan 30, 2026

Hi @tnmysh

yes its the AMD_GENERATED header file symbol, no?

In other words:
https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/CMakeLists.txt#L40

If we want some extra awareness without Lopper, why not push that to BSP logic? e.g. metal_xlnx_extension

@tnmysh
Copy link
Collaborator

tnmysh commented Jan 30, 2026

Hi @tnmysh

yes its the AMD_GENERATED header file symbol, no?

In other words: https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/CMakeLists.txt#L40

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.
Here:

metal_irq_register(irq_vect, zynqmp_linux_r5_proc_irq_handler, rproc);

If we move irq conversion logic to BSP, then irq_info above is not passing correct irq information, and interrupt handler is not called.

@bentheredonethat
Copy link
Contributor Author

Hi @tnmysh thanks for the comment -- I might be missing something but I dont think 'zynqmp' area applies.

  • The routine xlnx_hw_to_bsp_irq () is not called in the 'zynqmp' area.
  • The 'zynqmp' section is for Linux.
  • The Lopper generated header is only used for baremetal/generic and FreeRTOS.

So it seems to me like this commit does not conflict at all but again I might be missing something here.

@tnmysh
Copy link
Collaborator

tnmysh commented Jan 30, 2026

Hi @tnmysh thanks for the comment -- I might be missing something but I dont think 'zynqmp' area applies.

* The routine xlnx_hw_to_bsp_irq () is not called in the 'zynqmp' area.

* The 'zynqmp' section is for Linux.

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.

* The Lopper generated header is only used for baremetal/generic and FreeRTOS.

So it seems to me like this commit does not conflict at all but again I might be missing something here.

@bentheredonethat
Copy link
Contributor Author

@tnmysh Im still confused. Historically the 'zynqmp' subdir is for linux. you can see that a few ways:

  1. it has a reference DTSI
  2. i nthe file you reference it says 'Linux r5 remoteproc' where Linux is first https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/legacy_apps/machine/xlnx/zynqmp/zynqmp_linux_r5_proc.c#L142
  3. it is using printf (not xil_printf)
  4. also we have used 'zynqmp' as convention for linux side in libmetal, openamp demos historically - i can provide pointers there.

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants