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.

Sign up for GitHub

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

Jump to bottom

lib: remove MB and GB definition in utilities.h #242

Closed
wants to merge 1 commit into from
Closed

lib: remove MB and GB definition in utilities.h #242

wants to merge 1 commit into from

Conversation

arnopo
Copy link
Contributor

@arnopo arnopo commented Jul 10, 2023

The introduction of MB and GB macros introduces warnings in Zephyr build.
Indeed they are already defined in zephyr/include/zephyr/sys/util.h. Remove them and put them back in
lib/system/generic/xlnx_common/zynqmp_aarch64/sys.c.

@arnopo arnopo requested review from edmooring and tnmysh July 10, 2023 09:12
@arnopo arnopo added this to the Release V2023.10 milestone Jul 10, 2023
@arnopo
Copy link
Contributor Author

arnopo commented Jul 13, 2023

@bentheredonethat:
This PR fix #237. Please, could you confirm that it works on your side

@bentheredonethat
Copy link
Contributor

@arnopo confirmed

tnmysh
tnmysh approved these changes Aug 2, 2023
View reviewed changes
Copy link
Collaborator

@tnmysh tnmysh 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 me.

tnmysh
tnmysh reviewed Aug 2, 2023
View reviewed changes
lib/system/generic/xlnx_common/zynqmp_aarch64/sys.c
@@ -27,6 +27,9 @@
#include "xreg_cortexa53.h"
#endif /* defined(versal) */

#define MB (1024 * 1024UL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @arnopo for this change.
Just to avoid any future warnings, we can do "#ifndef MB".
But, me or @bentheredonethat will do that change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

edmooring
edmooring approved these changes Aug 2, 2023
View reviewed changes
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.

Good to go.

@bentheredonethat
Copy link
Contributor

Hi @arnopo I would actually push back on this patch

for ports where we have support for multiple processors (which we plan to upstream) then we would have to revert this patch because multiple processors would use the MB/GB symbols.

CC @tnmysh

@arnopo
lib: remove MB and GB definition in utilities.h
3098875 
The introduction of MB and GB macros introduces warnings in Zephyr
build.
Indeed they are already defined in zephyr/include/zephyr/sys/util.h.
Remove them and put them back in
lib/system/generic/xlnx_common/zynqmp_aarch64/sys.c.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
@arnopo arnopo requested a review from tnmysh August 16, 2023 15:11
@arnopo
Copy link
Contributor Author

arnopo commented Aug 16, 2023

Hi @arnopo I would actually push back on this patch

for ports where we have support for multiple processors (which we plan to upstream) then we would have to revert this patch because multiple processors would use the MB/GB symbols.

CC @tnmysh

Hi @bentheredonethat
Could I merge this one or do you plan to provide a better fix in a the short term?

@tnmysh
Copy link
Collaborator

tnmysh commented Aug 16, 2023

Hi @arnopo I would actually push back on this patch
for ports where we have support for multiple processors (which we plan to upstream) then we would have to revert this patch because multiple processors would use the MB/GB symbols.
CC @tnmysh

Hi @bentheredonethat Could I merge this one or do you plan to provide a better fix in a the short term?

@arnopo I will provide fix.

@tnmysh tnmysh mentioned this pull request Aug 16, 2023
Merged
@tnmysh
Copy link
Collaborator

tnmysh commented Aug 16, 2023

@arnopo @bentheredonethat done: #249

@arnopo arnopo closed this Aug 21, 2023
@arnopo arnopo removed this from the Release V2023.10 milestone Aug 21, 2023
@arnopo arnopo deleted the fix_MB_GB branch October 16, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edmooring edmooring edmooring approved these changes

@tnmysh tnmysh Awaiting requested review from tnmysh

Assignees
No one assigned
Labels
duplicate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arnopo @bentheredonethat @tnmysh @edmooring