Skip to content

sam0: allow flashing with JLinkExe#11725

Merged
dylad merged 1 commit intoRIOT-OS:masterfrom
benpicco:sam0-jlink
Oct 8, 2019
Merged

sam0: allow flashing with JLinkExe#11725
dylad merged 1 commit intoRIOT-OS:masterfrom
benpicco:sam0-jlink

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Jun 19, 2019

Currently sam0 will always use OpenOCD, even when the JLink programmer is selected.
Instead, use JLinkExe when it's availiable and a J-Link adapter is used.

miri64
miri64 reviewed Jun 19, 2019
View reviewed changes
@benpicco
sam0: allow flashing with JLinkExe
b617e40 
Currently sam0 will always use OpenOCD, even when the JLink programmer is selected.
Instead, use JLinkExe when it's availiable and a J-Link programmer is used.
@miri64 miri64 added Area: tools Area: Supplementary tools Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jul 30, 2019
@miri64 miri64 requested a review from PeterKietzmann July 30, 2019 17:29
@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Sep 13, 2019

Maybe I should close this.
By now I have discovered that to select JlinkExe I have to replace

include $(RIOTMAKE)/boards/sam0.inc.mk

with

include $(RIOTMAKE)/tools/jlink.inc.mk

Not sure if this is the way it is intended.

@benpicco benpicco added the Type: question The issue poses a question regarding usage of RIOT label Sep 16, 2019
@dylad
Copy link
Copy Markdown
Member

dylad commented Sep 25, 2019

@benpicco
I must that I like this PR in the current shape. What's your issue with it ?

@miri64 miri64 added Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR and removed Type: question The issue poses a question regarding usage of RIOT labels Sep 25, 2019
@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Sep 25, 2019

@dylad Well I'm not sure how this is intended to be used.

I thought switching the programmer should be done by setting the PROGRAMMER variable. This is enabled by this PR.

But then I discovered that I can switch the programmer simply by replacing

include $(RIOTMAKE)/boards/sam0.inc.mk with include $(RIOTMAKE)/tools/jlink.inc.mk.

If this is the indented way to do it, then this PR would be obsolete.

@dylad
Copy link
Copy Markdown
Member

dylad commented Sep 25, 2019 •
edited
Loading

I thought switching the programmer should be done by setting the PROGRAMMER variable. This is enabled by this PR.

AFAIK, this is the way to go.

include $(RIOTMAKE)/boards/sam0.inc.mk with include $(RIOTMAKE)/tools/jlink.inc.mk.
If this is the indented way to do it, then this PR would be obsolete.

replace it like this will break EDBG isn't it ?

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Sep 25, 2019

Well then this PR should still be valid.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Sep 25, 2019
@dylad
Copy link
Copy Markdown
Member

dylad commented Sep 25, 2019

I'll test it ASAP.

dylad
dylad reviewed Sep 25, 2019
View reviewed changes
makefiles/boards/sam0.inc.mk
PROGRAMMER ?= edbg
else ifeq ($(DEBUG_ADAPTER),jlink)
# only use JLinkExe if it's installed
ifneq (,$(shell command -v JLinkExe)))
Copy link
Copy Markdown
Member

@dylad dylad Sep 25, 2019

Choose a reason for hiding this comment

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

Does it work on macOS (I'm a total newbie with macOS)

Copy link
Copy Markdown
Contributor Author

@benpicco benpicco Sep 29, 2019

Choose a reason for hiding this comment

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

Given that I added it out of considerations for portability, I would think so.

dylad
dylad approved these changes Oct 8, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Tested ACK.
Try to flash tests/leds with jlink and edbg with several sam0 boards.

@dylad dylad added this to the Release 2019.10 milestone Oct 8, 2019
@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Oct 8, 2019
@dylad dylad merged commit cdcdaa6 into RIOT-OS:master Oct 8, 2019
@benpicco benpicco deleted the sam0-jlink branch October 8, 2019 19:05
@benpicco benpicco mentioned this pull request Oct 21, 2019
Merged
@miri64 miri64 mentioned this pull request Nov 6, 2019
Closed
@benpicco benpicco mentioned this pull request Feb 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@dylad dylad dylad approved these changes

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

Labels

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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.10

Development

Successfully merging this pull request may close these issues.

3 participants

@benpicco @dylad @miri64