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

makesfiles/jlink: fix exports for flashing #20779

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

crasbe
Copy link
Contributor

@crasbe crasbe commented Jul 6, 2024

Contribution description

Most details of this PR are detailed in #20775. tl;dr: In the makefiles/tools/jlink.inc.mk file, a % wildcard is used for the flash targets that includes all variants of flash... but not flash itself. Therefore important environment variables are not exported as they should be and others like JLINK_PRE_FLASH and JLINK_POST_FLASH do not work.

Additionally, the JLINK and JLINK_POST_FLASH variables were not exported at all. The former did not have an effect because the JLINK variable has a backup in dist/tools/jlink/jlink.sh. However it is defined in multiple places, for example in boards/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 add JLINK_DEBUG_TARGETS in a similar way to JLINK_FLASH_TARGETS and also add debug debug%.

Testing procedure

Of course, everything should still work as it should. Only very few boards use the JLINK_PRE_FLASH variable, but one that does is stk3200.
To test that this PR has the effect it should, you can compile for example examples/default with BOARD=stk3200 make flash (even if you don't have one, we don't need to actually flash it) and check the content of examples/default/bin/stk3200/burn.seg.

Without the PR, this is the content of burn.seg. The JLINK_PRE_FLASH = r from boards/stk3200/Makefile.include was ignored.

chris@ThinkPias:~/flashdev-riot/RIOT/examples/default$ cat bin/stk3200/burn.seg 
loadfile /home/chris/flashdev-riot/RIOT/examples/default/bin/stk3200/default.elf
r
g
exit

With the PR applied the output should look like this:

chris@ThinkPias:~/flashdev-riot/RIOT/examples/default$ cat bin/stk3200/burn.seg 
r
loadfile /home/chris/flashdev-riot/RIOT/examples/default/bin/stk3200/default.elf
r
g
exit

To test if the JLINK variable is properly exported, you can temporarily modify the boards/common/nrf52/Makefile.include and insert a random command for JLINK, in this case uname. It has to run successfully, otherwise the script will select openocd.

# set list of supported programmers
PROGRAMMERS_SUPPORTED += openocd bmp
# keep name of `JLINK` in sync with script jlink.sh in $(RIOTTOOLS)/jlink
- JLINK ?= JLinkExe
+ JLINK ?= uname
ifneq (,$(shell command -v $(JLINK)))
  PROGRAMMER ?= jlink
else
  PROGRAMMER ?= openocd
endif

Before applying the PR, the change of command does not have any effect and it should call JLinkExe anyway:

chris@ThinkPias:~/flashdev-riot/RIOT/examples/default$ BOARD=nrf52840dk make flash
Building application "default" for "nrf52840dk" with CPU "nrf52".

"make" -C /home/chris/flashdev-riot/RIOT/pkg/cmsis/ 
...
"make" -C /home/chris/flashdev-riot/RIOT/sys/net/gnrc/netif/pktq
   text	   data	    bss	    dec	    hex	filename
  45900	    172	   6452	  52524	   cd2c	/home/chris/flashdev-riot/RIOT/examples/default/bin/nrf52840dk/default.elf
/home/chris/flashdev-riot/RIOT/dist/tools/jlink/jlink.sh flash /home/chris/flashdev-riot/RIOT/examples/default/bin/nrf52840dk/default.elf
### Flashing Target ###
### Flashing elf file ###
SEGGER J-Link Commander V7.96e (Compiled Apr 17 2024 16:28:22)
DLL version V7.96e, compiled Apr 17 2024 16:27:56
...

After applying the PR, the error message Error: J-Link appears not to be installed on your PATH should be reported, because the dist/tools/jlink/jlink.sh script interprets the output of JLinkExe and obviously uname does not produce the correct output.

chris@ThinkPias:~/flashdev-riot/RIOT/examples/default$ BOARD=nrf52840dk make flash
Building application "default" for "nrf52840dk" with CPU "nrf52".

"make" -C /home/chris/flashdev-riot/RIOT/pkg/cmsis/ 
...
"make" -C /home/chris/flashdev-riot/RIOT/sys/ztimer
   text	   data	    bss	    dec	    hex	filename
  45932	    172	   6452	  52556	   cd4c	/home/chris/flashdev-riot/RIOT/examples/default/bin/nrf52840dk/default.elf
/home/chris/flashdev-riot/RIOT/dist/tools/jlink/jlink.sh flash /home/chris/flashdev-riot/RIOT/examples/default/bin/nrf52840dk/default.elf
### Flashing Target ###
### Flashing elf file ###
Error: J-Link appears not to be installed on your PATH
make: *** [/home/chris/flashdev-riot/RIOT/examples/default/../../Makefile.include:844: flash] Fehler 1

Issues/PRs references

See also #20775.

@github-actions github-actions bot added Area: build system Area: Build system Area: tools Area: Supplementary tools labels Jul 6, 2024
Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Thanks a lot for digging deep into this issue! The fix looks good to me.

@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 8, 2024
@riot-ci
Copy link

riot-ci commented Jul 8, 2024

Murdock results

✔️ PASSED

1261e03 makesfiles/jlink: fix exports for flashing

Success Failures Total Runtime
10178 0 10178 17m:27s

Artifacts

@benpicco benpicco added this pull request to the merge queue Jul 8, 2024
Merged via the queue into RIOT-OS:master with commit 4e4c4a1 Jul 8, 2024
27 checks passed
@crasbe
Copy link
Contributor Author

crasbe commented Jul 8, 2024

@cladmi @mguetschow Thank you for the support on this :)

@crasbe crasbe deleted the pr/jlink_flashing branch July 8, 2024 16:32
@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants