dist/tools/openocd: use reset halt command for board using stlink by aabadie · Pull Request #8976 · RIOT-OS/RIOT

aabadie · 2018-04-18T16:05:00Z

Contribution description

Based on ~~#8856~~ and ~~#8475~~, this PR uses the reset halt command of openocd for all board using an stlink adapter.

It might not be the best solution and has side effects: after starting the debug session the program is always halted in the entry point (for ARM) otherwise it can be at any position which might be something expected (but doesn't work after).

Issues/PRs references

Fixes #8975, based on ~~#8856~~ and ~~#8475~~

cladmi

A good idea to refactor this, but still a bit messy and not sure all changes are valid. See details inline.

cladmi · 2018-04-20T14:17:27Z

boards/stm32f4discovery/Makefile.include


 export DEBUG_ADAPTER ?= stlink
 export STLINK_VERSION ?= 2
+export OPENOCD_CONFIG := $(RIOTBOARD)/$(BOARD)/dist/openocd.cfg


I do not like that you need here to set it to the default openocd value.
I would prefer it handled in the stlink-adapter, see below.

cladmi · 2018-04-20T14:18:30Z

makefiles/tools/openocd-adapters/stlink.inc.mk

+export OPENOCD_DBG_START_CMD = -c 'reset halt'
+
+# select OpenOCD configuration depending on CPU type
+ifeq (,$(OPENOCD_CONFIG))


Follow up to setting to the default value in boards:

You could add another check if $(RIOTBOARD)/$(BOARD)/dist/openocd.cfg does not exist before using the st configuration.

cladmi · 2018-04-20T14:23:56Z

boards/common/st/configs/stm32l0.cfg

@@ -1,3 +1,3 @@
 source [find target/stm32l0.cfg]
-reset_config srst_only
+reset_config srst_only connect_assert_srst


Why do you change the reset_config configurations in a "boards/common/stm32-link: add common place for stlink" commit ?
If it it necessary, it should be another properly justified commit.

cladmi · 2018-04-20T14:31:01Z

boards/b-l072z-lrwan1/Makefile.include

 # this board has an on-board ST-link adapter
 export DEBUG_ADAPTER ?= stlink

-# call a 'reset halt' command before starting the debugger


Please do not call this commit "revert" as it is not reverting the change, only using the now common definition.

cladmi · 2018-04-20T14:34:38Z

boards/b-l072z-lrwan1/dist/openocd.cfg

@@ -1,3 +0,0 @@
-source [find target/stm32l0.cfg]
-
-reset_config trst_and_srst srst_nogate connect_assert_srst


This reset configuration is different from the common one in st/configs/stm32l0.cfg

I think this file should be kept.

I think this file should be kept

It could but it makes no difference if using the common one (with the connect_assert_srst). I checked on other L0 based boards (nucleo-l053/nucleo-l073) and the openocd configuration file provided by this PR works for all of them.

cladmi · 2018-04-20T14:35:28Z

boards/stm32f7discovery/dist/openocd.cfg

@@ -1 +0,0 @@
-source [find target/stm32f7x.cfg]


It does not match the st/configs/stm32f7.cfg configuration.

cladmi · 2018-04-20T14:39:35Z

boards/common/nucleo/Makefile.include

-endif

-# this board uses openocd
+# include openocd


Previous comment before was better than repeating what the next line does.

aabadie · 2018-04-24T08:50:29Z

@cladmi, maybe you missed the fact that this PR is based on #8475 and most of your comments are related to this one.

cladmi · 2018-04-24T08:54:25Z

Oh absolutely right. Should I re-do them there then ?

aabadie · 2018-04-24T11:20:52Z

Should I re-do them there then ?

Not needed, both PRs are linked. But I'll apply the requested changes in #8475

aabadie · 2018-04-24T14:42:34Z

Rebased on top of latest #8475, not dependent of #8856 anymore.

cladmi · 2018-06-29T16:08:50Z

I think you can now rebase again.

aabadie · 2018-07-02T13:41:22Z

@cladmi, rebased and comments addressed.

cladmi

ACK changes looks good for me.

I trust you that it is necessary for all stlink boards.

cladmi · 2018-07-06T10:07:26Z

Merge now if you want, or ask if you want someone else testing.

kaspar030 · 2018-07-06T10:14:46Z

This implies that it is now not possible anymore to "attach" to a running node, right?

Please wait a bit before merging, I'd like to get some opinions over lunch.

kYc0o · 2018-07-18T15:14:23Z

Please wait a bit before merging, I'd like to get some opinions over lunch.

/edit, it was for @kaspar030 not for @smlng
@kaspar030 did you? I'm asking because this is marked as a bugfix, which would be good to have for the release.

miri64 · 2018-08-24T13:33:58Z

Ping?

'reset halt' is now set globally for all boards using st-link when debugging

aabadie · 2018-09-18T15:09:57Z

After discussing briefly about this PR with @cladmi at the summit, it seems that the best solution is:

to keep a configuration that works most of the time: when one wants to start a gdb debug session on the board, gdb should always be able to attach the gdb server, even if the CPU is in sleep mode. This is what this PR does, by forcing a reset
for other not so common cases, e.g. attach to a running application or a crashed application to see it's current state, that were raised by @kaspar030, it's highly possible that the debug target fails with stlink. But we should still provide this possibility: I pushed a fixup commit to address this point. Using

OPENOCD_DBG_START_CMD= make BOARD=<any stlink board> debug

will put -c halt (default value in openocd) in OPENOCD_DBG_START_CMD.

aabadie · 2018-10-12T11:20:52Z

@cladmi, what's your opinion about my latest comment ?

aabadie · 2018-10-23T11:07:51Z

@cladmi ?

jnohlgard · 2018-11-28T09:21:17Z

After discussing briefly about this PR with @cladmi at the summit, it seems that the best solution is:

to keep a configuration that works most of the time: when one wants to start a gdb debug session on the board, gdb should always be able to attach the gdb server, even if the CPU is in sleep mode. This is what this PR does, by forcing a reset

I agree completely with this. It is incredibly annoying to have the debug or flash commands fail because the CPU happened to be in a low power mode at the time that the tool tried to connect. This is going to become more common now that the boards are getting more low power support fixes and applications will begin using low power modes to a greater extent.

for other not so common cases, e.g. attach to a running application or a crashed application to see it's current state, that were raised by @kaspar030, it's highly possible that the debug target fails with stlink. But we should still provide this possibility: I pushed a fixup commit to address this point.

It would be useful to have a command which tries to attach to the running system, and if that fails fall back to a connect under reset configuration, but someone would need to implement this as a shell script since OpenOCD needs a restart after a failed init AFAIK. I have experimented with various combinations of catch statements in the OpenOCD TCL scripts, but so far have not found a configuration which can recover from a failed connection attempt.

aabadie · 2018-11-29T10:22:42Z

It is incredibly annoying to have the debug or flash commands fail because the CPU happened to be in a low power mode at the time that the tool tried to connect. This is going to become more common now that the boards are getting more low power support fixes and applications will begin using low power modes to a greater extent.

@gebart, I'm happy we are on the same line about this. I'm +1 for merging #10479 since it does the same thing but globally. What would be great is to add some documentation somewhere about how to attach a crashed/running application without resetting it all. I'm thinking of this kind of thing:

OPENOCD_DBG_START_CMD="-c halt" make BOARD=<any openocd programmed board> debug

cladmi · 2018-12-06T13:19:44Z

Sorry I missed the ping, I agree that having reliable flash is better.

I think #10479 only takes care on the debug-server though, am I right ?

aabadie · 2018-12-06T18:24:50Z

I think #10479 only takes care on the debug-server though, am I right ?

Not really. In the current state, #10479 applies '-c reset halt' by default to all debug sessions using OpenOCD, see here.
But it will still be possible to override OPENOCD_DBG_START_CMD as described in #8976 (comment).

stale · 2019-08-10T03:08:19Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

aabadie added Area: tools Area: Supplementary tools Area: boards Area: Board ports labels Apr 18, 2018

aabadie requested a review from cladmi April 18, 2018 16:05

aabadie added State: waiting for other PR State: The PR requires another PR to be merged first Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Apr 18, 2018

cladmi reviewed Apr 20, 2018

View reviewed changes

aabadie mentioned this pull request Apr 24, 2018

boards/stm32: use common place of openocd config files for all boards using stlink #8475

Merged

aabadie force-pushed the pr/stlink_debug_fix branch from a1f0103 to 5d85034 Compare April 24, 2018 14:41

aabadie mentioned this pull request Jun 18, 2018

boards/iotlab-m3|a8: fix openocd configuration #8977

Merged

aabadie removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 1, 2018

aabadie force-pushed the pr/stlink_debug_fix branch from 5d85034 to b9a33ff Compare July 1, 2018 12:35

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 1, 2018

cladmi approved these changes Jul 6, 2018

View reviewed changes

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 24, 2018

aabadie added 3 commits September 18, 2018 17:07

makefiles/tools: use reset halt for all stlink based boards

44f297a

boards/b-l072Z-lrwan1: remove useless openocd debug option

ada3543

'reset halt' is now set globally for all boards using st-link when debugging

fixup! makefiles/tools: use reset halt for all stlink based boards

84b6ca6

aabadie force-pushed the pr/stlink_debug_fix branch from b1c2d2a to 84b6ca6 Compare September 18, 2018 15:08

aabadie mentioned this pull request Nov 28, 2018

openocd: Use reset halt when starting debug-server #10479

Closed

aabadie mentioned this pull request Nov 30, 2018

boards/stm32l0: Fix openocd to prevent flash locking #10343

Merged

fjmolinas mentioned this pull request Aug 7, 2019

dist/tools/openocd: add OPENOCD_EXTRA_INIT_RESET use for nucleo-f091rc #11976

Merged

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

stale bot closed this Sep 10, 2019

aabadie deleted the pr/stlink_debug_fix branch June 24, 2020 06:09

		@@ -1,3 +0,0 @@
		source [find target/stm32l0.cfg]

		reset_config trst_and_srst srst_nogate connect_assert_srst

Conversation

aabadie commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Issues/PRs references

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Apr 24, 2018

Uh oh!

cladmi commented Apr 24, 2018

Uh oh!

aabadie commented Apr 24, 2018

Uh oh!

aabadie commented Apr 24, 2018

Uh oh!

cladmi commented Jun 29, 2018

Uh oh!

aabadie commented Jul 2, 2018

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

cladmi commented Jul 6, 2018

Uh oh!

kaspar030 commented Jul 6, 2018

Uh oh!

kYc0o commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Aug 24, 2018

Uh oh!

aabadie commented Sep 18, 2018

Uh oh!

aabadie commented Oct 12, 2018

Uh oh!

aabadie commented Oct 23, 2018

Uh oh!

jnohlgard commented Nov 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie commented Nov 29, 2018

Uh oh!

cladmi commented Dec 6, 2018

Uh oh!

aabadie commented Dec 6, 2018

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aabadie commented Apr 18, 2018 •

edited

Loading

kYc0o commented Jul 18, 2018 •

edited

Loading

jnohlgard commented Nov 28, 2018 •

edited

Loading