-
Notifications
You must be signed in to change notification settings - Fork 8.3k
soc: st: stm32: add '-align' flag for signing tool v2.21.0+ #99721
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
base: main
Are you sure you want to change the base?
soc: st: stm32: add '-align' flag for signing tool v2.21.0+ #99721
Conversation
|
Set this to draft because I realized this prob needs some updates to the docs. If this looks like the right direction, LMK and I'll update this PR with the doc changes as well. |
|
Thanks for pushing this. |
Agreed, hoping you can find a better solution! |
|
So, you can look at nrf-regtool which performs a version check once, then store it for later usage. |
etienne-lms
left a comment
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.
Worth to add a note in the migration guide to tell flash programming N6 boards require at least STM32_SigningTool_CLI v2.21.0 with a reference to #99456?
Preventing use of earlier versions would be another way to handle it. Radical but efficient. |
@erwango thanks for the pointer, I'll check it out a bit later today and try to rework this PR to do something similar. |
In case, you missed it: Complaining on versions earlier than 2.21 could be ok (at least in a first version). |
e484e68 to
92bee62
Compare
92bee62 to
3a9147c
Compare
soc/st/stm32/stm32n6x/CMakeLists.txt
Outdated
| else() | ||
| message(FATAL_ERROR | ||
| "Unable to determine ${SIGNING_TOOL} version (expected match to " | ||
| "${SIGNING_TOOL_VERSION_REGEX} regex)" | ||
| ) |
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.
Not sure if we want to bomb out here or just throw a warning. I figured it's better to have an explicit error than to arbitrarily choose either appending or omitting the -align flag in the case where we can't determine the version.
|
@erwango @etienne-lms I updated the PR to check the signing tool version as part of the build process. LMK if this looks OK and I'll remove the draft status. |
3a9147c to
1c0235b
Compare
soc/st/stm32/stm32n6x/CMakeLists.txt
Outdated
| RESULT_VARIABLE result | ||
| ) | ||
|
|
||
| set(SIGNING_TOOL_ARGS |
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.
Minor comment: CMake conventions recommend to use lower case names for varaibles local to a CMake file, and upper case names for variables exported to other CMake file.
That said, before your changes, this CMakeLists.txt file already defined SIGNING_TOOL and SIGNING_TOOL_ARGS that are local. Maybe worth to change them also, either as a follow-up P-R, or in your P-R if you will to.
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.
@etienne-lms thanks for your feedback! I updated the PR to use lower case names for all the local variables (LMK if there is any I missed).
soc/st/stm32/stm32n6x/CMakeLists.txt
Outdated
| "${SIGNING_TOOL} version is v${SIGNING_TOOL_VERSION}, " | ||
| "appending \"-align\" flag to post-build signing command " | ||
| "(applicable only with header v2.3 for MCUs, starting from " | ||
| "version v${SIGNING_TOOL_MIN_VERSION})" |
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.
Could you indent with 2 more space chars, here at below (in the fatal message).
|
I think you can remove the draft status. |
zacck
left a comment
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.
So I just tested this on both 2.20 and 2.21 and it works really smooth
Starting in v2.21.0, the STM32 signing tool ('STM32_SigningTool_CLI')
stopped automatically adding padding bytes at the beginning of the
payload to align it to the 0x400 offset. To restore this behavior, the
'-align' flag must be passed to the signing tool post-build command.
This commit checks for signing tool version v2.21.0 or higher and
appends the '-align' flag to the post-build signing command.
Fixes zephyrproject-rtos#99456
Signed-off-by: Chris Wilson <chris@binho.io>
1c0235b to
fb4c524
Compare
|



Starting in
v2.21.0, the STM32 signing tool (STM32_SigningTool_CLI) stopped automatically adding padding bytes at the beginning of the payload to align it to the0x400offset. To restore this behavior, the-alignflag must be passed to the signing tool post-build command.This commit checks for signing tool version
v2.21.0or higher and appends the-alignflag to the post-build signing command.Fixes #99456