-
Couldn't load subscription status.
- Fork 8.1k
drivers: dp: fix build on M0 MCUs #89750
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
Conversation
drivers/dp/swdp_ll_pin.h
Outdated
| "subs r3, #1\n" | ||
| "sub r3, #1\n" | ||
| "cmp r3, #0\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUBS is valid on Cortex-M0+ as long as a low register (r0–r7) is used and the immediate is ≤255, which is true here. The issue is likely due to the assembler not using Unified Assembly Language (UAL). Adding -masm-syntax-unified in cmake/compiler/gcc/target_arm.cmake should fix this without needing to rewrite the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to build west build -p -b nucleo_c071rb samples/subsys/dap with below changes:
diff --git a/cmake/compiler/gcc/target_arm.cmake b/cmake/compiler/gcc/target_arm.cmake
index 7d46a4e9572..46ad4ec50a6 100644
--- a/cmake/compiler/gcc/target_arm.cmake
+++ b/cmake/compiler/gcc/target_arm.cmake
@@ -3,6 +3,7 @@
set(ARM_C_FLAGS)
list(APPEND ARM_C_FLAGS -mcpu=${GCC_M_CPU})
+list(APPEND ARM_C_FLAGS -masm-syntax-unified)
if(CONFIG_COMPILER_ISA_THUMB2)
list(APPEND ARM_C_FLAGS -mthumb)
Hope this helps!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @wearyzen, thanks a lot this is indeed very helpful, I'll look into it and open a followup PR!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #89772, bumped into few issues with existing snippets, may be worth fixing but I'm not too confident in changing it project wide myself to be honest, added a .syntax unified here instead as a stop gap, how does that sound?
Current code does not build on Cortex-M0, seems like it does not like subs: Error: instruction not supported in Thumb16 mode -- `subs r3,#1' Adding a unified assembler language declaration in the snippet seems to fix the problem, also add an M0+ board so this is tested in CI. Signed-off-by: Fabio Baltieri <[email protected]>
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Current code does not build on Cortex-M0, seems like it does not like subs:
Error: instruction not supported in Thumb16 mode -- `subs r3,#1'
Replace the instruction with a sub and cmp, add an M0+ board so this is tested in CI.