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

boards/rpi-pico: specify needed args when using jlink flasher #19537

Merged
merged 1 commit into from
May 3, 2023

Conversation

dylad
Copy link
Member

@dylad dylad commented May 2, 2023

Contribution description

This PR adds the jlink device variable and use hexfile to flash application with a jlink probe.

Testing procedure

Attach a jlink probe to SWD pins.
Try to flash any app with jlink
PROGRAMMER=jlink make BOARD=rpi-pico -C examples/blinky flash

Issues/PRs references

None.

@github-actions github-actions bot added the Area: boards Area: Board ports label May 2, 2023
@dylad dylad added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 2, 2023
@riot-ci
Copy link

riot-ci commented May 2, 2023

Murdock results

✔️ PASSED

bd5cff5 boards/rpi-pico: specify needed args when using jlink flasher

Success Failures Total Runtime
412 0 412 01m:44s

Artifacts

@dylad dylad requested a review from maribu May 3, 2023 07:08

ifeq ($(PROGRAMMER),jlink)
JLINK_DEVICE = RP2040_M0_0
FLASHFILE = $(HEXFILE)
Copy link
Member

Choose a reason for hiding this comment

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

Funny, yesterday there was some discussion in matrix and there seemed to be a consensus to prefer ELF files for flashing unless there is a reason against ELF files. Admittedly, only few actively participated in the discussion, though.

But if the ELF file works equally well, I'd personally prefer to stick with the ELF file for flashing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, especially because creating a .hex is an extra step (that takes time, ...)

Copy link
Member Author

@dylad dylad May 3, 2023

Choose a reason for hiding this comment

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

That's because of this discussion that I think about it. But I don't have strong opinion over .elf vs .hex. I wanted to use J-link probe for sometimes now with this board so it ease debugging when working on new peripheral drivers.
I can confirm that it works too with .elf file so I'll update this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

And squashed directly.

Signed-off-by: dylad <dylan.laduranty@mesotic.com>
@dylad dylad force-pushed the boards/rpi-pico/allow_jlink_flasher branch from a039dac to bd5cff5 Compare May 3, 2023 07:51

ifeq ($(PROGRAMMER),jlink)
JLINK_DEVICE = RP2040_M0_0
FLASHFILE = $(ELFFILE)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FLASHFILE = $(ELFFILE)

I think it should even work like this, as the ELF file is the default flash file anyway. Please squash / amend directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, I share some doubts here. When I played with J-link before this PR, it defaults to .bin file and flashing doesn't work. (Because I didn't provide the flash memory offset I guess) so I add to pass FLASHFILE manually during the make flash command.
But maybe I'm wrong here. I'll try again tonight. I left my rpi-pico board at home.

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be the culprit.

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's merge this as is. I can rebase #19541 on top of that and drop this line again there, as then the ELF file would indeed be the default.

Copy link
Member Author

Choose a reason for hiding this comment

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

As you wish. I've removed the waiting for PRs label.
I'll also try to test #19541 to see if there is any breakage.

@dylad
Copy link
Member Author

dylad commented May 3, 2023

Let's merge #19541 first then.

@dylad dylad added State: waiting for other PR State: The PR requires another PR to be merged first and removed State: waiting for other PR State: The PR requires another PR to be merged first labels May 3, 2023
@dylad
Copy link
Member Author

dylad commented May 3, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented May 3, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 2c8fc24 into RIOT-OS:master May 3, 2023
@dylad dylad deleted the boards/rpi-pico/allow_jlink_flasher branch May 3, 2023 10:33
@dylad
Copy link
Member Author

dylad commented May 3, 2023

Thanks for the review!

@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports 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.

5 participants