boards/stm32: use common place of openocd config files for all boards using stlink by aabadie · Pull Request #8475 · RIOT-OS/RIOT

aabadie · 2018-01-30T07:50:04Z

Contribution description

This PR moves the st-link related codes in boards/common/nucleo/Makefile.include in a common place, e.g boards/common/stm32-stlink.
This is because not only Nucleo boards use st-link, but also b-l072z-lrwan1 and b-l475e-iot01a boards. The support for these 2 boards is also updated by this PR.

Maybe the stlink related code could go in another place so suggestions are welcome.
Maybe this common code could be reused by stm32fxdiscovery boards since they also use stlink (but some has different openocd configurations).

Issues/PRs references

None

jnohlgard · 2018-01-30T08:13:32Z

I like the idea, better configuration reuse, but the interdependence between the board directories is a bit messy (stm32-stlink depends on files in the nucleo common directory)
Is it possible to place some of the configuration in makefiles/tools/ if it is reusable for all st-link users?

aabadie · 2018-01-30T08:26:05Z

Is it possible to place some of the configuration in makefiles/tools/ if it is reusable for all st-link users?

Not sure, st-link allows the use of openocd and this is directly related to boards. Maybe move the common openocd in another directory boards/common/stm32-openocd ?

aabadie · 2018-01-30T08:27:40Z

the interdependence between the board directories is a bit messy

Indeed, I missed that part...

haukepetersen · 2018-01-30T09:34:22Z

Good cleanup!

I am thinking though: doesn't it rather make sense to move the makefile out of the board tree and dump/merge all the lines from boards/common/stm32-stlink/Makefile.include into makefiles/tools/openocd-adapters/stlink.inc.mk? I think this would probably be the cleaner and more versatile approach.

aabadie · 2018-01-30T09:34:55Z

Updated this PR by moving the common openocd configuration from the common/nucleo to common/stm32-stlink. I think it's better like this.

aabadie · 2018-01-30T09:36:01Z

I think this would probably be the cleaner and more versatile approach.

Sorry, I missed that comment. I'll give this a try.

haukepetersen · 2018-01-30T09:43:32Z

cool. (actually, I also saw this file for the first time today :-) ).

Maybe we can add a cfg or config sub-dir to dist/tools/openocd/ where we can put all the openocd `.cfg files? This way we can maybe improve reusage of them also for many other platforms?

aabadie · 2018-01-30T09:52:54Z

I also saw this file for the first time today :-)

Same here !

This way we can maybe improve reusage of them also for many other platforms?

That's what I'm trying to do :)

aabadie · 2018-01-30T13:05:31Z

I reworked a bit this PR, including previous comments. Here the actual status:

move stm32 openocd config files to makefiles/tools/openocd-adapters/configs. It will certainly be possible to move other openocd configurations there (for other CPUs) and have a common place for all of them
kept boards/common/stm32-stlink to have a single place with common serial, stlink and includes

haukepetersen

I still like this PR and it is getting there :-)

But I am also still convinced, that we can get rid of boards/common/stm32-stlink/Makefile.include and move the contained information to more general makefiles.

haukepetersen · 2018-01-30T17:03:21Z

boards/common/stm32-stlink/Makefile.include

+export STLINK_VERSION ?= 2-1
+
+# Include openocd adapters for st-link
+include $(RIOTMAKE)/tools/openocd-adapters/stlink.inc.mk


there seems to be a include-loop: here we include stlink.inc.mk and below openocd.inc.mk. But openocd.inc.mk does also include stlink.inc.mk... So seems to me we can drop this line.

haukepetersen · 2018-01-30T17:04:47Z

boards/common/stm32-stlink/Makefile.include

+
+# Default is to use ST-link v2-1 adapter (used by all Nucleos)
+export DEBUG_ADAPTER ?= stlink
+export STLINK_VERSION ?= 2-1


How about to more this line to the top of makefiles/tools/openocd-adapters/stlink.inc.mk and add doc that version 2-1 is the default?

haukepetersen · 2018-01-30T17:16:07Z

boards/common/stm32-stlink/Makefile.include

+# configure the serial terminal
+PORT_LINUX ?= /dev/ttyACM0
+PORT_DARWIN ?= $(firstword $(sort $(wildcard /dev/tty.usbmodem*)))
+include $(RIOTMAKE)/tools/serial.inc.mk


I think it is not very clean to put the common serial configuration into a common board folder that is referring to a programmer...

I think I have an alternative idea, will PR something later tonight.

forgot to mention: examples where this might get confusion are for example the green discovery boards, which have an on-board stlink, but don't supply any serial adapter...

ok, I was not aware of that.

I think I have an alternative idea, will PR something later tonight.

Describe what you have in mind and I'll update this PR

Alright, so how about #8484 for this part.

kaspar030

Why move the openocd configs to dist/tools/openocd-adapters?
I'd prefer them in boards/common, or maybe if dist, why not just dist/tools/openocd/configs?

kaspar030 · 2018-01-30T20:26:02Z

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

+
+# select OpenOCD configuration depending on CPU type
+ifeq (,$(OPENOCD_CONFIG))
+  ifeq ($(CPU),stm32f0)


Why not OPENOCD_CONFIG := $(RIOTMAKE)/tools/openocd-adapters/configs/$(CPU).cfg?

aabadie · 2018-01-31T09:10:01Z

I am also still convinced, that we can get rid of boards/common/stm32-stlink/Makefile.include

Done, since #7695 might be a good solution for common serial port configuration

I'd prefer them in boards/common, or maybe in dist, why not just dist/tools/openocd/configs?

I can move them to boards/common, where I think it's simpler to find. In fact, I don't have a strong opinion on that.

aabadie · 2018-01-31T09:10:51Z

Why not OPENOCD_CONFIG := $(RIOTMAKE)/tools/openocd-adapters/configs/$(CPU).cfg?

Good point! Completely missed that factorisation. Done !

aabadie · 2018-02-05T14:22:22Z

I'd prefer them in boards/common, or maybe in dist, why not just dist/tools/openocd/configs?

Thinking of it now and it makes more sense to put the common openocd config files in boards/common. This is because those files contains openocd options that are specific to some boards.
Why not boards/common/stm32/openocd ?

kYc0o · 2018-03-19T14:37:37Z

Thinking of it now and it makes more sense to put the common openocd config files in boards/common. This is because those files contains openocd options that are specific to some boards.
Why not boards/common/stm32/openocd ?

I don't think it's directly bound to stm32, since that's a cpu and the config depends on the board. For this, I would say a bigger refactor on the boards folder is necessary, on which we can maybe separate the boards according to it's CPUs, or a similar approach.

However, it's also not bound to nucleo devices, even though they're very similar.

I'd vote for something like boards/common/st, which is bound only to the manufacturer.

cladmi

ACK please squash

aabadie · 2018-06-07T10:46:40Z

squashed. @haukepetersen @kaspar030 do you still have comments or can we dismiss your reviews since I think your previous comments were addressed ?

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

@cladmi @kaspar030 @haukepetersen, now that #8549 is merged, putting the openocd common configuration file in boards/stm32 makes more sense to me. What do you think ?

no time

kaspar030 · 2018-06-12T21:18:06Z

This looks OK to me.

aabadie · 2018-06-13T11:20:58Z

@cladmi, @haukepetersen do you have more comments here ? or can I squash ?

aabadie · 2018-06-14T09:40:46Z

@haukepetersen @cladmi, ok to squash here ?

cladmi · 2018-06-14T10:34:55Z

Ok for me

aabadie · 2018-06-14T10:36:03Z

It would be great if someone could ping @haukepetersen IRL or at least dismiss his review.

aabadie · 2018-06-14T16:06:29Z

Seems that @haukepetersen will never reply here. @cladmi @kaspar030, are you ok for dismissing his review ?

aabadie · 2018-06-19T06:45:48Z

@haukepetersen, if you don't reply or if you are not interested anymore by this PR, please let us know.

- introduce common place for boards using stlink: same serial, all use openocd - apply this to nucleos

aabadie · 2018-06-26T18:47:43Z

rebased and squashed. @cladmi, I think we left enough time to @haukepetersen to leave his opinion here and since it never came, I'll dismiss his review.

no reply

aabadie · 2018-06-28T13:13:43Z

Let's merge this one.

aabadie added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: tools Area: Supplementary tools Area: boards Area: Board ports labels Jan 30, 2018

aabadie requested a review from haukepetersen January 30, 2018 07:50

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

aabadie force-pushed the pr/common-stlink branch from 172a0f1 to 9840960 Compare January 30, 2018 09:26

aabadie force-pushed the pr/common-stlink branch 2 times, most recently from d15d0ce to 72b6156 Compare January 30, 2018 13:01

aabadie force-pushed the pr/common-stlink branch from 72b6156 to ac11203 Compare January 30, 2018 13:05

haukepetersen previously requested changes Jan 30, 2018

View reviewed changes

haukepetersen mentioned this pull request Jan 30, 2018

boards/make: use shared makefiles for serial port selection #8484

Closed

kaspar030 previously requested changes Jan 30, 2018

View reviewed changes

aabadie force-pushed the pr/common-stlink branch from ac11203 to a41f6bb Compare January 31, 2018 09:07

aabadie force-pushed the pr/common-stlink branch 2 times, most recently from 34e12ae to c6b895f Compare January 31, 2018 14:44

aabadie mentioned this pull request Feb 16, 2018

boards/nucleo: optimize openocd file selection #8391

Closed

aabadie mentioned this pull request Mar 19, 2018

boards/b-l072z-lrwan1: fix openocd configuration and 'make debug' #8790

Merged

aabadie force-pushed the pr/common-stlink branch from d83613d to 12f884c Compare June 6, 2018 19:50

cladmi approved these changes Jun 7, 2018

View reviewed changes

aabadie force-pushed the pr/common-stlink branch from 12f884c to 6a54b6a Compare June 7, 2018 10:36

aabadie changed the title ~~boards/stm32-stlink: introduce common place for boards using stlink (nucleos and others)~~ boards/stm32: use common place of openocd config files for all boards using stlink Jun 12, 2018

aabadie force-pushed the pr/common-stlink branch from 6a54b6a to 21e0af1 Compare June 12, 2018 21:28

aabadie added 6 commits June 26, 2018 20:45

boards/common/stm32-link: add common place for stlink

de15af4

- introduce common place for boards using stlink: same serial, all use openocd - apply this to nucleos

boards/b-l072z-lrwan1: use common stlink parts

b174cc2

boards/b-l475e-iot01a: use common stlink parts

68a5422

boards/stm32f3discovery: adapt to common stlink parts

3c4eb85

boards/stm32f769i-disco: adapt to common stlink parts

8bfa434

boards/maple-mini: use common stlink parts

265c152

aabadie force-pushed the pr/common-stlink branch from a83ee8d to 265c152 Compare June 26, 2018 18:45

cladmi mentioned this pull request Jun 28, 2018

newlib.mk: replace the error by a warning on nano header not found #9384

Closed

aabadie merged commit 4e1f07b into RIOT-OS:master Jun 28, 2018

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

aabadie mentioned this pull request Nov 30, 2018

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

Merged

aabadie deleted the pr/common-stlink branch December 28, 2018 16:31

Conversation

aabadie commented Jan 30, 2018

Contribution description

Issues/PRs references

Uh oh!

jnohlgard commented Jan 30, 2018

Uh oh!

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

Uh oh!

Uh oh!

aabadie commented Jan 30, 2018

Uh oh!

haukepetersen commented Jan 30, 2018

Uh oh!

aabadie commented Jan 30, 2018

Uh oh!

aabadie commented Jan 30, 2018

Uh oh!

haukepetersen commented Jan 30, 2018

Uh oh!

aabadie commented Jan 30, 2018

Uh oh!

aabadie commented Jan 30, 2018

Uh oh!

haukepetersen left a comment

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

aabadie Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

haukepetersen Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

kaspar030 left a comment

Choose a reason for hiding this comment

Uh oh!

kaspar030 Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jan 31, 2018

Uh oh!

aabadie commented Jan 31, 2018

Uh oh!

aabadie commented Feb 5, 2018

Uh oh!

kYc0o commented Mar 19, 2018

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

aabadie commented Jun 12, 2018

Uh oh!

kaspar030 commented Jun 12, 2018

Uh oh!

aabadie commented Jun 13, 2018

Uh oh!

aabadie commented Jun 14, 2018

Uh oh!

cladmi commented Jun 14, 2018

Uh oh!

aabadie commented Jun 14, 2018

Uh oh!

aabadie commented Jun 14, 2018

Uh oh!

aabadie commented Jun 19, 2018

aabadie commented Jan 30, 2018 •

edited

Loading

aabadie commented Jun 7, 2018 •

edited

Loading

aabadie commented Jun 26, 2018 •

edited

Loading