Skip to content
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

cmake: platforms: xlnx: Update machine from 'zynqmp_' to 'xlnx_' #305

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bentheredonethat
Copy link
Contributor

Match update so that the cmake platform files are coupled to vendor and not SOC which is previously 'zynqmp_'.

Match update so that the cmake platform files are coupled to vendor and not
SOC which is previously 'zynqmp_'.

Signed-off-by: Ben Levinsky <[email protected]>
@bentheredonethat
Copy link
Contributor Author

@arnopo @wmamills updated .cmake file names

@arnopo arnopo requested a review from wmamills September 9, 2024 07:05
Copy link
Contributor

@arnopo arnopo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to go.

@@ -1,5 +1,5 @@
set (CMAKE_SYSTEM_PROCESSOR "aarch64" CACHE STRING "")
set (MACHINE "zynqmp_a53" CACHE STRING "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the alignement

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After addressing this comment it's good to go.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but still not aligned

@@ -1,5 +1,5 @@
set (CMAKE_SYSTEM_PROCESSOR "arm" CACHE STRING "")
set (MACHINE "zynqmp_r5" CACHE STRING "")
set (MACHINE "xlnx_r5" CACHE STRING "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -1,5 +1,5 @@
set (CMAKE_SYSTEM_PROCESSOR "arm" CACHE STRING "")
set (MACHINE "zynqmp_r5" CACHE STRING "")
set (MACHINE "xlnx_r5" CACHE STRING "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -1,5 +1,5 @@
set (CMAKE_SYSTEM_PROCESSOR "aarch64" CACHE STRING "")
set (MACHINE "zynqmp_a53" CACHE STRING "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but still not aligned

@@ -1,5 +1,5 @@
set (CMAKE_SYSTEM_PROCESSOR "aarch64" CACHE STRING "")
set (MACHINE "zynqmp_a53" CACHE STRING "")
set (MACHINE "xlnx_a53" CACHE STRING "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@arnopo
Copy link
Contributor

arnopo commented Oct 17, 2024

@bentheredonethat could you address the comment that we merge it for this release ( code freeze tomorrow evening)?

@arnopo arnopo added this to the Release v2024.10 milestone Oct 17, 2024
@tnmysh
Copy link
Collaborator

tnmysh commented Oct 17, 2024

@bentheredonethat could you address the comment that we merge it for this release ( code freeze tomorrow evening)?

He is on vacation right now. I am not sure if I can push changes in this PR. I will find a way if it's possible, I will do it.

@tnmysh
Copy link
Collaborator

tnmysh commented Oct 17, 2024

As discussed in RP call, this will be merged after release.

CC: @arnopo, @bentheredonethat

@arnopo arnopo removed this from the Release v2024.10 milestone Oct 18, 2024
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.

5 participants