makesfiles/jlink: fix exports for flashing#20779
Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom Jul 8, 2024
Merged
Conversation
mguetschow
approved these changes
Jul 8, 2024
Contributor
mguetschow
left a comment
There was a problem hiding this comment.
Thanks a lot for digging deep into this issue! The fix looks good to me.
Contributor
Author
|
@cladmi @mguetschow Thank you for the support on this :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
Most details of this PR are detailed in #20775. tl;dr: In the
makefiles/tools/jlink.inc.mkfile, a%wildcard is used for the flash targets that includes all variants offlash...but notflashitself. Therefore important environment variables are not exported as they should be and others likeJLINK_PRE_FLASHandJLINK_POST_FLASHdo not work.Additionally, the
JLINKandJLINK_POST_FLASHvariables were not exported at all. The former did not have an effect because theJLINKvariable has a backup indist/tools/jlink/jlink.sh. However it is defined in multiple places, for example inboards/common/nrf52/Makefile.include, so it would be good to actually use the defined value and not the fallback value.The latter is not currently used by any boards, so it did not have an effect. However I want to use it, so this PR creates the basis for that.
The
debug%wildcard seems to call another target as well, so it is not affected? But I could addJLINK_DEBUG_TARGETSin a similar way toJLINK_FLASH_TARGETSand also adddebug debug%.Testing procedure
Of course, everything should still work as it should. Only very few boards use the
JLINK_PRE_FLASHvariable, but one that does isstk3200.To test that this PR has the effect it should, you can compile for example
examples/defaultwithBOARD=stk3200 make flash(even if you don't have one, we don't need to actually flash it) and check the content ofexamples/default/bin/stk3200/burn.seg.Without the PR, this is the content of
burn.seg. TheJLINK_PRE_FLASH = rfromboards/stk3200/Makefile.includewas ignored.With the PR applied the output should look like this:
To test if the
JLINKvariable is properly exported, you can temporarily modify theboards/common/nrf52/Makefile.includeand insert a random command forJLINK, in this caseuname. It has to run successfully, otherwise the script will selectopenocd.Before applying the PR, the change of command does not have any effect and it should call
JLinkExeanyway:After applying the PR, the error message
Error: J-Link appears not to be installed on your PATHshould be reported, because thedist/tools/jlink/jlink.shscript interprets the output ofJLinkExeand obviouslyunamedoes not produce the correct output.Issues/PRs references
See also #20775.