Add xilinx a72 and a78 #252

bentheredonethat · 2023-09-11T23:17:25Z

Add support for xilinx platforms cortex a72 on Versal, cortex a78 on versal-net

both of these have support for standalone and baremetal

bentheredonethat · 2023-09-12T02:44:04Z

Hi @arnopo the failed check is due to Xilinx BSP. The failures are present in other platforms too.

We need the routines as part of enabling Xilinx BSP for these platforms.

Thanks,
Ben

arnopo · 2023-09-12T07:37:56Z

Hi @bentheredonethat,

Hi @arnopo the failed check is due to Xilinx BSP. The failures are present in other platforms too.

I can see two issues in CI one is linked to the format of one of your patch, the second is linked to the Zephyr CI, that I just fixed.

We need the routines as part of enabling Xilinx BSP for these platforms.

Thanks, Ben

bentheredonethat · 2023-09-12T13:55:32Z

@arnopo ok i can update commit message due to warning
"1: UC5 Title exceeds max length (98>72): "lib: generic: zynqmp_a78: Add support for versal_net a78 platform with System Device Tree workflow"

was there anything else?

bentheredonethat · 2023-09-12T14:56:27Z

@arnopo fixed the commit message issue

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.h

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.c

edmooring · 2023-09-26T03:56:10Z

@arnopo,
How did you handle this for the existing files like apps/system/generic/machine/zynqmp_r5/helper.c in your initial integration with checkpatch?

I could argue that checking for CamelCase in the platform-specific code is wrong in that this code may have to call down into BSP-specific code that may have a very different set of coding conventions.

arnopo · 2023-09-26T07:30:30Z

How did you handle this for the existing files like apps/system/generic/machine/zynqmp_r5/helper.c in your initial integration with checkpatch?

I could argue that checking for CamelCase in the platform-specific code is wrong in that this code may have to call down into BSP-specific code that may have a very different set of coding conventions.

I did not handle the exception, it could become difficult to manage.
For me, failing the test but with a justification is acceptable.

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.c

bentheredonethat · 2023-10-09T21:34:21Z

@tnmysh to be clear - is the only gap to fix right now the metal-io-mem-map() part? the cache part there is already convention of other platforms.

if so i will work on updating Xilinx BSP and then update the PR with that accordingly.

tnmysh · 2023-10-11T15:12:42Z

@arnopo we have refactored Xilinx platform specific files and added new platform support accordingly in this PR. I think it's ready to be merged, we can have this one as part of this release.

Do you have any extra comments? I see some failures with checkpatch. camelCasing can't be fixed. Other than that this PR looks good to me.

tnmysh

LGTM.

lib/system/generic/xlnx_common/sys.c

lib/system/generic/xlnx_common/zynqmp_aarch64/sys.c

lib/system/freertos/CMakeLists.txt

lib/system/freertos/sys.h

lib/system/freertos/CMakeLists.txt

lib/system/generic/sys.h

lib/system/freertos/sys.h

bentheredonethat · 2023-10-12T16:20:15Z

Hi @arnopo i just pushed another version. i testd zynq, zynqmp-a53 and zynqmp-r5 for freertos and generic. all of these passed.

That being said, if you will do not want the sys.c/sys.h xilinx common code consolidated i am fine with that.

arnopo

Just a minor clean-up to address, else LGTM

lib/system/generic/xlnx/CMakeLists.txt

bentheredonethat · 2023-10-13T15:46:03Z

Hi @arnopo on my end there is a lot of testing to cover. I would ask that this not be included in the upcoming release.

Thanks
CC @tnmysh

tnmysh · 2023-10-13T15:49:51Z

@arnopo I removed my ACK till Ben confirms this is working.
We can exclude this commit from this release and merge it later.

bentheredonethat · 2023-10-13T22:37:45Z

@arnopo i did testing for builds of apps and demos that use libmetal for xilinx paltforms. im ok t to proceed now
CC @tnmysh

tnmysh

This patch removes almost 1000 lines (~400 line of code) from libmetal library and we can achieve same platform support.

Thanks @bentheredonethat for efforts. When your side testing is done please let us know.
This will be good addition to release.

bentheredonethat · 2023-10-14T14:08:06Z

im ok with testing on my side

arnopo · 2023-10-16T08:40:36Z

@bentheredonethat
Please, could you rebase it on top of main branch that fixes CI test

@tnmysh : then ok to merge?

bentheredonethat · 2023-10-16T14:11:31Z

@arnopo i woke up sick today.. will try to do today. otherwise will hopefully do tomorrow

tnmysh · 2023-10-16T17:46:38Z

@bentheredonethat Please, could you rebase it on top of main branch that fixes CI test

@tnmysh : then ok to merge?

Yes I am ok to merge.

lib/utilities.h provided MB and GB macros so include from there instead of providing the definition again for a53 platforms. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Move common code in Xilinx area to consolidated location to remove redundant code. Enable A72 and A78 build in Standalone BSP. lib: generic: Add support for a78 with System Device Tree flow System Device Tree workflow is AMD-Xilinx workflow whereby the BSP, libraries and applications in software are derived from a hardware-design. The hardware-design is used to generate a system device tree (SDT) that describes information for Linux and other processing environments. The xreg/xcpu files are not generated for SDT workflow. Because of this do not include if SDT symbol is present in BSP. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Move common code in Xilinx area to consolidated location to remove redundant code. Enable A72 and A78 build in BSP for FreeRTOS OS. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

bentheredonethat · 2023-10-17T14:57:50Z

@arnopo rebased off latest

arnopo requested review from edmooring, arnopo and tnmysh September 12, 2023 07:40

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from 2f78e95 to 55a117b Compare September 12, 2023 14:39

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch 2 times, most recently from c2a1f59 to 5e536a0 Compare September 12, 2023 15:25

tnmysh reviewed Sep 22, 2023

View reviewed changes

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.h Outdated Show resolved Hide resolved

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from 5e536a0 to ee4d9d7 Compare September 25, 2023 15:54

tnmysh reviewed Sep 25, 2023

View reviewed changes

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.c Outdated Show resolved Hide resolved

tnmysh reviewed Oct 5, 2023

View reviewed changes

lib/system/freertos/xlnx_common/zynqmp_aarch64/sys.c Outdated Show resolved Hide resolved

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch 4 times, most recently from 0a6740f to fa4e21f Compare October 10, 2023 23:49

tnmysh approved these changes Oct 11, 2023

View reviewed changes

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from fa4e21f to a889df9 Compare October 11, 2023 15:17

arnopo requested changes Oct 12, 2023

View reviewed changes

lib/system/generic/xlnx_common/sys.c Outdated Show resolved Hide resolved

lib/system/generic/xlnx_common/zynqmp_aarch64/sys.c Outdated Show resolved Hide resolved

lib/system/freertos/CMakeLists.txt Outdated Show resolved Hide resolved

lib/system/freertos/sys.h Show resolved Hide resolved

arnopo requested changes Oct 12, 2023

View reviewed changes

lib/system/freertos/CMakeLists.txt Outdated Show resolved Hide resolved

lib/system/generic/sys.h Show resolved Hide resolved

lib/system/freertos/sys.h Show resolved Hide resolved

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from a889df9 to 3271579 Compare October 12, 2023 16:19

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch 2 times, most recently from 0fb9132 to 5dbbf87 Compare October 12, 2023 21:54

arnopo approved these changes Oct 13, 2023

View reviewed changes

lib/system/generic/xlnx/CMakeLists.txt Show resolved Hide resolved

arnopo added this to the Release V2023.10 milestone Oct 13, 2023

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from 5dbbf87 to f244596 Compare October 13, 2023 14:32

tnmysh self-requested a review October 13, 2023 15:49

arnopo modified the milestones: Release V2023.10, Release V2024.04 Oct 13, 2023

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from f244596 to 7c8b5ae Compare October 13, 2023 22:32

tnmysh approved these changes Oct 13, 2023

View reviewed changes

arnopo modified the milestones: Release V2024.04, Release V2023.10 Oct 16, 2023

bentheredonethat added 3 commits October 17, 2023 07:54

lib: a53: remove redundant MB and GB macros

ecdf507

lib/utilities.h provided MB and GB macros so include from there instead of providing the definition again for a53 platforms. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

lib: freertos: consolidate common Xilinx code

96d2bb8

Move common code in Xilinx area to consolidated location to remove redundant code. Enable A72 and A78 build in BSP for FreeRTOS OS. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

bentheredonethat force-pushed the add-xilinx-a72-and-a78 branch from 7c8b5ae to 96d2bb8 Compare October 17, 2023 14:57

arnopo merged commit f6a87a8 into OpenAMP:main Oct 17, 2023
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add xilinx a72 and a78 #252

Add xilinx a72 and a78 #252

bentheredonethat commented Sep 11, 2023

bentheredonethat commented Sep 12, 2023 •

edited

Loading

arnopo commented Sep 12, 2023

bentheredonethat commented Sep 12, 2023 •

edited

Loading

bentheredonethat commented Sep 12, 2023

edmooring commented Sep 26, 2023

arnopo commented Sep 26, 2023

bentheredonethat commented Oct 9, 2023

tnmysh commented Oct 11, 2023

tnmysh left a comment

bentheredonethat commented Oct 12, 2023 •

edited

Loading

arnopo left a comment

bentheredonethat commented Oct 13, 2023

tnmysh commented Oct 13, 2023

bentheredonethat commented Oct 13, 2023

tnmysh left a comment

bentheredonethat commented Oct 14, 2023

arnopo commented Oct 16, 2023

bentheredonethat commented Oct 16, 2023

tnmysh commented Oct 16, 2023

bentheredonethat commented Oct 17, 2023

Add xilinx a72 and a78 #252

Add xilinx a72 and a78 #252

Conversation

bentheredonethat commented Sep 11, 2023

bentheredonethat commented Sep 12, 2023 • edited Loading

arnopo commented Sep 12, 2023

bentheredonethat commented Sep 12, 2023 • edited Loading

bentheredonethat commented Sep 12, 2023

edmooring commented Sep 26, 2023

arnopo commented Sep 26, 2023

bentheredonethat commented Oct 9, 2023

tnmysh commented Oct 11, 2023

tnmysh left a comment

Choose a reason for hiding this comment

bentheredonethat commented Oct 12, 2023 • edited Loading

arnopo left a comment

Choose a reason for hiding this comment

bentheredonethat commented Oct 13, 2023

tnmysh commented Oct 13, 2023

bentheredonethat commented Oct 13, 2023

tnmysh left a comment

Choose a reason for hiding this comment

bentheredonethat commented Oct 14, 2023

arnopo commented Oct 16, 2023

bentheredonethat commented Oct 16, 2023

tnmysh commented Oct 16, 2023

bentheredonethat commented Oct 17, 2023

bentheredonethat commented Sep 12, 2023 •

edited

Loading

bentheredonethat commented Sep 12, 2023 •

edited

Loading

bentheredonethat commented Oct 12, 2023 •

edited

Loading