platformio / platform-ststm32

ST STM32: development platform for PlatformIO
https://registry.platformio.org/platforms/platformio/ststm32
Apache License 2.0
385 stars 305 forks source link

New BMP pre-attach option needed for STM32L0 and STM32F7 #144

Open tve opened 5 years ago

tve commented 5 years ago

A new command/option has recently been added to the black magic probe in order to assert reset before probing/scanning: https://github.com/blacksphere/blackmagic/commit/44fc24e0e747293fa05b62ed7439501553829b0b The brief reason is that this is necessary to wake-up an MCU in stop/standby mode.

The tl;dr; for pio is that in https://github.com/platformio/platform-ststm32/blob/develop/builder/main.py#L121-L130 an option "-ex", "monitor assert_srst scan" needs to be added before the monitor swdp_scan (I assume also for the jtag case, but I cannot verify that). I do not know whether it can always be added or whether it must only be done for the L0 and F7 family...

To make it work I just hacked the source. I know I can also duplicate the whole upload command using $UPLOADCMD but then I loose the port autodetection. Any thoughts on how to fix this? Add an UPLOAD_SRST env set in the board that can be set to never|scan|attach?

tve commented 5 years ago

Any comments? I'm happy to provide a pull request but I don't know what you'd like to see...

ivankravets commented 5 years ago

I don't see any changes here https://github.com/blacksphere/blackmagic/wiki/Useful-GDB-commands

@esden could you comment here?

tve commented 5 years ago

Looks like the wiki hasn't been updated. I linked to the commit that introduced the new monitor command option in my first comment. The change I made in my ststm32 is:

diff --git a/builder/main.py b/builder/main.py
index a4e852e..96b5029 100644
--- a/builder/main.py
+++ b/builder/main.py
@@ -116,18 +116,23 @@ if upload_protocol == "mbed":
     ]

 elif upload_protocol.startswith("blackmagic"):
+    upload_reset = env.BoardConfig().get("upload.reset", "never")
+    assert_srst = "monitor assert_srst " + upload_reset
+    if upload_reset == "never":
+        assert_srst = "#" + assert_srst # make it compat with old BMP versions
     env.Replace(
         UPLOADER="$GDB",
         UPLOADERFLAGS=[
             "-nx",
             "--batch",
             "-ex", "target extended-remote $UPLOAD_PORT",
+            "-ex", assert_srst,
             "-ex", "monitor %s_scan" %
             ("jtag" if upload_protocol == "blackmagic-jtag" else "swdp"),
             "-ex", "attach 1",
             "-ex", "load",
             "-ex", "compare-sections",
             "-ex", "kill"
         ],
         UPLOADCMD="$UPLOADER $UPLOADERFLAGS $SOURCE"
     )

This allows me to define upload.reset in my board file. For example:

"upload": {
  "maximum_ram_size": 20480,
  "maximum_size": 196608,
  "protocol": "blackmagic",
  "protocols": [ "jlink", "stlink", "blackmagic", "mbed" ],
  "reset": "scan"
}
esden commented 5 years ago

This is a change in the new firmware that is not officially released. I actually want to have a backwards compatible parameter available. Also the srst assert is only necessary in certain situations and not all the time. So this should be a parameter the user can choose to switch on or off and not something that should be on all the time. This will break things and reset some chips that have broken reset chains. (which is unfortunately common :( yes I am looking at you SAM >_< )

Edit: Sorry for the slightly incorrect reply here. I did not spend the correct amount of time parsing the existing comment here. I see that @tve is actually adding this as an option. That is correct way of doing it. The fact that I want there to be a backwards compatible option "enable" available again too is still true though.

tve commented 5 years ago

I'm not sure what you mean by "backwards compatible option enable". The way I coded things is that if there is no upload.reset option in the board file then no monitor assert_srst command is issued (it issues a commented command). So right off the bat nothing changes from the status quo until someone edits a board file to add the upload.reset option. I assume this is not the backwards-compatibility you're looking for? Could you be more specific? What I didn't do is provide a way in the platform.ini file to override/change this. If you want that, I believe a new UPLOAD_XXX variable needs to be defined in a number of places so it makes its way through to the builder.

esden commented 5 years ago

I am talking about the Black Magic Probe firmware. The new option is only available in the still unreleased version of the firmware that you flashed yourself. No official units ship with the firmware that contains this new options.

What I want is that the official release BMP firmware will have the 'monitor assert_srst enable' and 'disable' option that is backwards compatible. So your change will not be necessary.

Still I think your patch is useful as it adds the ability to use the new assertion types that are not available in the stable release of the Black Magic Probe firmware.

Also you should be careful that your change still works with the older firmware releases that use 'enable' and 'disable' keywords.

tve commented 5 years ago

OK. It might be useful to add a comment to the commit explaining what changes you'd like to see... Maybe Uwe could then modify his commit...

As far as I can tell, the current released command is monitor connect_srst enable|disable, not monitor assert_srst .... Also, connect_srst enable appears to be the same as assert_srst attach and assert_srst scan is new functionality and is necessary for stm32l0. So in the end some patch in pio to select assert_srst scan (whatever it's called) is still necessary somehow. Or am I missing something?

esden commented 5 years ago

You are correct. And yes the backwards compatibility will just require an alias from enable to attach. Fixing this in BMP was on my TODO list for quite a while now. I will try to prioritize it.

And yes pio still needs a patch to be able to work right with stm32l0 targets.

mtbspace commented 3 years ago

Option is also needed for my STM32F0 target board