boards/b-l072z-lrwan1: fix openocd configuration and 'make debug' by aabadie · Pull Request #8790 · RIOT-OS/RIOT

aabadie · 2018-03-16T12:33:18Z

Contribution description

This PR is an attempt to fix the make flash and debug targets with the b-l072z-lrwan1 board. The current state in master prevents the debug from being used with it.

#8500 initially tried to provide a solution for flashing the board when the MCU is in low-power mode. Digging a little more in OpenOCD documentation, it appears that just the connect_assert_srst reset option was required: this option forces a wake up of the MCU before flashing (e.g ensure 'SRST is asserted'). So the first part of this PR is to restore the initial config (srst_only) and add connect_assert_srst because only this new option was required.
But the problem now is that when one adds this new option, it breaks the make debug command for this board (and maybe others).

So the second part of this PR tries to fix the make debug by forcing a reset in the openocd debug command.

It works for this board, now I can flash, even if the MCU is in low-power mode, and debug the board. But I'm not sure if the change in openocd.sh is valid.

Issues/PRs references

None

aabadie · 2018-03-19T08:41:17Z

This PR is quite important for a good user experience with the ST LRWAN1. I don't know who could be a good reviewer for this, maybe @gebart or @vincent-d ?

kYc0o · 2018-03-19T13:47:29Z

This was somehow already taken into account in boards/common/nucleo/dist/ where all the configs for stlink are stored. I suppose this needs a major refactor to avoid the declaration of similar configurations for this debugger.

Though this is not a nucleo board, it uses the same debugger.

Do you know if there are major changes in the physical connections compared to a nucleo board?

aabadie · 2018-03-19T13:55:29Z

Do you know if there are major changes in the physical connections compared to a nucleo board?

Yes and they are very similar, see #8475, where I tried to factorize all these boards.

But this PR adds an extra reset option (connect_assert_srst), which is required when the MCU is in sleep mode. Otherwise flashing failed.

The main thing I'm unsure is the change in openocd.sh.

kYc0o · 2018-03-19T14:15:04Z

Yes and they are very similar, see #8475, where I tried to factorize all these boards.

Ok, then I'd say first merge #8475, which is almost ready as far as I see the discussion.

But this PR adds an extra reset option, which is required when the MCU is in sleep mode. Otherwise flashing failed.

Which extra reset? On openocd.sh? Anyhow, currently the stm32l0 have the options you added, minus the connect_assert_srst which is not present. Is it really necessary? Is it currently working on stm32l0 nucleo boards?

The main thing I'm unsure is the change in openocd.sh.

This would need extensive testing to see if the boards using openocd are not affected. In my small experience with it, I'd say it doesn't hurt at all, it's just an extra reset, but I'd like to test on some boards to see if it really doesn't affect.

aabadie · 2018-03-19T14:15:15Z

The main thing I'm unsure is the change in openocd.sh.

But this change is required if I want use make debug on that board. The problem might be more general with openocd.

aabadie · 2018-03-19T14:28:11Z

Which extra reset? On openocd.sh? Anyhow, currently the stm32l0 have the options you added, minus the connect_assert_srst which is not present. Is it really necessary? Is it currently working on stm32l0 nucleo boards?

Sorry, maybe I was unclear, I updated my comment with the option name (already mentioned in the initial description of this PR though).

then I'd say first merge #8475, which is almost ready as far as I see the discussion

Not sure, I would say that the discussion there is staled. Maybe @haukepetersen or @gebart want to add something there.

I'd like to test on some boards to see if it really doesn't affect

That's exactly my point.

miri64 · 2018-03-27T17:16:41Z

dist/tools/openocd/openocd.sh

            -c 'init' \
            -c 'targets' \
-            -c 'halt' \
+            -c 'reset halt' \


I think this should be made configurable somehow so you can only set it for certain boards.

good idea !

@miri64 see #8856

aabadie · 2018-06-18T11:26:40Z

@cladmi, I think this one could be merged. @miri64 you change request was already addressed and merged in #8856.

cladmi · 2018-06-18T11:49:56Z

This PR is reverting your #8500 PRand updating the configuration.
I would really like to have the justification in the commit message(s).

Apart from that, if it works for you, I think we can go.

Maybe @jia200x you can also test if you have this board ?

aabadie · 2018-06-18T12:07:25Z

@cladmi, not sure it's better, but I added some information in the commit message.

cladmi · 2018-06-18T13:11:19Z

There's no need for trst signal, just srst is enough. Also not using srst_nogate
seems safer.

connect_assert_srst is used to enforce a CPU reset before connecting to
it with openocd. This is useful when debugging on-chip.

As described, there are changes for doing reset correctly == fix and changes for cleanup/safer things, + adding the 'rtos' thing. I always prefer them in separate commits in case one breaks someones board.
Also it forces to justify why a change is required.

There's no need for trst signal, just srst is enough. Also not using srst_nogate seems safer.

connect_assert_srst is used to enforce a CPU reset before connecting to it with openocd. This is useful when debugging on-chip. For details on openocd reset configuration, see

aabadie · 2018-06-19T11:26:17Z

I always prefer them in separate commits in case one breaks someones board.

@cladmi, I split the commit in 3, hope this is fine for you :)

aabadie · 2018-06-20T07:50:06Z

@miri64, are you ok with the changes here ?

@cladmi

I trust @cladmi's judgement more than my own, when it comes to openocd ;-)

aabadie force-pushed the pr/openocd_b-l072z-lrwan1 branch from 4f08b13 to b773e49 Compare March 19, 2018 13:57

miri64 previously requested changes Mar 27, 2018

View reviewed changes

aabadie mentioned this pull request Mar 30, 2018

dist/tools/openocd: add the possibility to do a reset before halt in openocd debug command #8856

Merged

aabadie mentioned this pull request Jun 4, 2018

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

Merged

aabadie force-pushed the pr/openocd_b-l072z-lrwan1 branch from b773e49 to f2cc3bc Compare June 4, 2018 15:24

aabadie removed the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Jun 4, 2018

cladmi requested a review from jia200x June 18, 2018 11:50

aabadie force-pushed the pr/openocd_b-l072z-lrwan1 branch from f2cc3bc to e28f6a0 Compare June 18, 2018 12:05

aabadie added 3 commits June 19, 2018 13:13

boards/b-l072z-lrwan1: update openocd config

618a65b

There's no need for trst signal, just srst is enough. Also not using srst_nogate seems safer.

boards/b-l072z-lrwan1: enable RTOS support in openocd config

58067c2

boards/b-l072z-lrwan1: enforce CPU reset when connecting with openocd

ba30315

connect_assert_srst is used to enforce a CPU reset before connecting to it with openocd. This is useful when debugging on-chip. For details on openocd reset configuration, see

aabadie force-pushed the pr/openocd_b-l072z-lrwan1 branch from e28f6a0 to ba30315 Compare June 19, 2018 11:17

cladmi approved these changes Jun 19, 2018

View reviewed changes

cladmi merged commit d055fde into RIOT-OS:master Jun 29, 2018

cladmi added this to the Release 2018.07 milestone Jul 10, 2018

aabadie deleted the pr/openocd_b-l072z-lrwan1 branch September 23, 2018 08:58

Conversation

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

Uh oh!

Contribution description

Issues/PRs references

Uh oh!

aabadie commented Mar 19, 2018

Uh oh!

kYc0o commented Mar 19, 2018

Uh oh!

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

Uh oh!

Uh oh!

kYc0o commented Mar 19, 2018

Uh oh!

aabadie commented Mar 19, 2018

Uh oh!

aabadie commented Mar 19, 2018

Uh oh!

miri64 Mar 27, 2018

Choose a reason for hiding this comment

Uh oh!

aabadie Mar 27, 2018

Choose a reason for hiding this comment

Uh oh!

aabadie Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 18, 2018

Uh oh!

cladmi commented Jun 18, 2018

Uh oh!

aabadie commented Jun 18, 2018

Uh oh!

cladmi commented Jun 18, 2018

Uh oh!

aabadie commented Jun 19, 2018

Uh oh!

aabadie commented Jun 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aabadie commented Mar 16, 2018 •

edited

Loading

aabadie commented Mar 19, 2018 •

edited

Loading