Connect under reset fails for STLINKV3 and SAMD51

[UtkarshVerma]

I discussed this shortly on Discord but I think it makes for a valid issue.

As the title says, the following results in a failed scan for me:

mon connect_rst enable
mon swd_scan

The hypothesis so far is that the F7 on the STLINKV3 is too fast and hence the scan command is being issued earlier than it should be.

The following lines support this claim.

https://github.com/blackmagic-debug/blackmagic/blob/main/src%2Fplatforms%2Fstlinkv3%2Fplatform.c#L116-L128

A possible way forward is to rely on timing or hardware feedback so that the reset sequence becomes platform agnostic. Alternatively, we could also have target-specific reset sequences but that would likely become a maintenance burden with time.

I would be interested in working up a PR for this so let's have a fruitful discussion on this.

[nickd4]

I also noticed this a while ago, and I believe the culprit is the DSU. For context:

12.1 Overview

The Device Service Unit (DSU) provides a means of detecting debugger probes. It enables the ARM Debug Access Port (DAP) to have control over multiplexed debug pads and CPU Reset. ...

The DSU is provided by the vendor (Microchip) and it seems they designed it with their own conventions e.g. it has a CTRL register with a SWRST bit, similar to many of the other peripherals. My guess is that this peripheral doesn't operate whilst the chip is in reset, and this would explain why we cannot connect under reset.

Of course, the reason we want to connect under reset is to make sure that the CPU is halted before it can execute any code (particularly if that code is putting the chip in weird states and thus bricking it). Luckily, Microchip provides a way to do this:

12.6.2 CPU Reset Extension

“CPU Reset extension” refers to the extension of the Reset phase of the CPU core after the external Reset is released. This ensures that the CPU is not executing code at start-up while a debugger is connects to the system. The debugger is detected on a RESET release event when SWCLK is low. ... To release the CPU, write a '1' to STATUSA.CRSTEXT. ...

So I think what we have to do is provide a third option for connect_rst to support this mode. Perhaps connect_rst could be enable, disable or samd51.

The normal procedure for enable is something like:

1. Set NRST low, delay as appropriate
2. Query the ADIv5
3. Halt the CPU with DHCSR
4. Set NRST hi-Z, delay as appropriate (long delay in case pull-up is high value)

My proposed procedure for samd51 would be something like:

1. Set NRST low, delay as appropriate
2. Set SWCLK low, delay as appropriate
3. Set NRST hi-Z, delay as appropriate (long delay in case pull-up is high value)
4. Query the ADIv5
5. Halt the CPU with DHCSR
6. Write a 1 to STATUSA.CRSTEXT, delay or poll for synchronization

I have been having a look through the code to see what would be involved in making this change (because I have a bricked board ☹️) and I think with a little more study I can probably find a way, even if the first try might be a bit hacky.

I am looking at src/target/swdptap_generic.c, src/platforms/common/swdptap.c, src/platforms/hosted/ftdi_swd.c and so far it's not entirely clear to me what the interface / abstraction boundary is, there seem to be virtual functions for sending and receiving bits with or without parity but a non-virtual function for doing a turnaround. Hmm.

One useful feature however is that all the SWD functions seem to leave SWCLK low, so perhaps if we just initialize the swdptap interface we will then be able to release NRST before issuing further commands. But perhaps this wouldn't apply to the FTDI versions, which look rather complicated since I assume they transfer multiple bits per command. So I'd appreciate any tips from someone who knows.

By the way, I remember when I first encountered the issue, I experimented with various delays and that did not help for me. So I prefer the theory outlined here.