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

mm_drv: tlb: Fix compile time warning #62854

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

dbaluta
Copy link
Collaborator

@dbaluta dbaluta commented Sep 20, 2023

This fixes the following compile time warning;

zephyr/drivers/mm/mm_drv_intel_adsp_mtl_tlb.c: In function 'sys_mm_drv_mm_init': zephyr/include/zephyr/sys/__assert.h:44:52: error: format '%p' expects argument of type 'void *', but argument 2 has type 'long unsigned int' [-Werror=format=]
   44 | #define __ASSERT_MSG_INFO(fmt, ...) __ASSERT_PRINT("\t" fmt "\n", ##__VA_ARGS__)

by specifying the correct type in __ASSERT macro.

jxstelter
jxstelter previously approved these changes Sep 20, 2023
tmleman
tmleman previously approved these changes Sep 20, 2023
@dbaluta
Copy link
Collaborator Author

dbaluta commented Sep 20, 2023

Should I fix the commit message to fit into 75 chars? This would be very hard as I was trying to offer a snippet of the compile time warning.

I think we should go to 100 chars per line for commit message as Linux does.

@kv2019i
Copy link
Collaborator

kv2019i commented Sep 20, 2023

@dbaluta wrote:

Should I fix the commit message to fit into 75 chars? This would be very hard as I was trying to offer a snippet of the compile time warning.

I think we should go to 100 chars per line for commit message as Linux does.

Thanks for doing this, still not sure how this started failing builds now.

I think Zephyr folks prefer to line-wrap the commit messages with an indent on the wrapped line (like commit 5689916 ).

kv2019i
kv2019i previously approved these changes Sep 20, 2023
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

I'll approve, but I think this we'll need an small update to make the mandatory checks to pass.

dcpleung
dcpleung previously approved these changes Sep 20, 2023
@marc-hb
Copy link
Collaborator

marc-hb commented Sep 20, 2023

I think we should go to 100 chars per line for commit message as Linux does.

Please review #62876 first, it will it easier for you to submit that change later.

This would be very hard as I was trying to offer a snippet of the compile time warning.

For now just trim some verbosity and insert some newlines.

You can test locally with ./scripts/ci/check_compliance.py -m Gitlint or ./scripts/ci/check_compliance.py -e Kconfig -e Kconfigbasic when you use sof/west.yml

@@ -667,7 +667,7 @@ static int sys_mm_drv_mm_init(const struct device *dev)
L2_SRAM_BASE > UNUSED_L2_START_ALIGNED) {

__ASSERT(false,
"unused l2 pointer is outside of l2 sram range %p\n",
"unused l2 pointer is outside of l2 sram range %lu\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, is this not going to change the display from hex to decimal?

Why not just cast to (void *) instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@marc-hb I think you are right, we want to print the pointer in hex. Fixed this plus commit message.

Thanks!

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Wait, is this not going to change the display from hex to decimal?

Dismiss my request for changes if I'm wrong.

This fixes the following compile time warning;

drivers/mm/mm_drv_intel_adsp_mtl_tlb.c: In function 'sys_mm_drv_mm_init':
include/zephyr/sys/__assert.h:44:52: error: format '%p' expects argument
of type 'void *', but argument 2 has type
'long unsigned int' [-Werror=format=]
__ASSERT_PRINT("\t" fmt "\n", ##__VA_ARGS__)

Signed-off-by: Daniel Baluta <[email protected]>
@carlescufi carlescufi merged commit b4998c3 into zephyrproject-rtos:main Sep 22, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants