implementation suggestion for stlink clones

[KillerLink]

I'd like to propose a possible Implementation improvement for the uint32_t detect_rev(void) used for "stlink" hardware. I own one of these none genuin examples already discussed here, apparently known as "baite". There already exists some work by UweBonnes which i would like to improve upon and create a pull request.

The current issue is, that due to slight differences in hardware (no external pull-up on PC13, see github issue linked above) the board might be wrongly classified as "rev = 0", which leads to the LED beeing mapped to PA8 as opposed to PA9 in void platform_init(void).

I would like to detect this special case by first reading PC13 and PC14 once with internal pull up and once with internal pull down. The internal pull resistors values are expected to be inbetween 30kOhm to 50kOhm, which gives a weaker pull than an external 10kOhm would. As a result, if the readings from pull-up differs from pull-down, i think it can be concluded that the pins might be floating. This could already be sufficient as a detection mechanism, but i'd like to combine it with what UweBonnes did, read if RB11 is pulled high externally. If that is the case, classify as equivalent to "rev = 1".

My questions are now:

1. Does this make sense or do i have a flaw in my thinking?
2. Should i implement this and make a pull request?

[UweBonnes]

Make a PR. Remember we have a lot of legacy, e.g. F4-Disco boards with PC13/14 both floating. What hardware do you have to test?

Otherwise as platform, I now perfer pc-stlinkv2 as platform. I have reflash many boards with STlink firmware. Core development is much easier with a default debug output.

[KillerLink]

In regard to what test hardware I have available:
*) ST-Link V2 (see top half of image below)
*) ST-Link V2 "baite" (see bottom half of image below)
*) Some Discovery Boards, but unsure of exakt model right now, will report details if necessary.

I did some preliminary tests / implementation and attempted to create a table of all hardware that might exist. I measured the result of some pin states for PC13,PC14,PB11,PA15 resulted for the 2 mentioned devices i have available right now:
V2: PC13=low PC14=float PB11=high PA15=float
V2 "baite": PC13=float PC14=float PB11=high PA15=float
(also see image below)

Might the two devices i have be classified as already known Variants?
Does the table I created contain obvious Mistakes?
The "baite" might be similiar to the F4 Discovery with both pins floating you mentioned, do you know if that legacy F4 also has 2 contrairwise red/blue LEDs connected to PA9?

Also, while attempting to implement the classification that i noticed that it currently is interleaved with some hardware specific configuration, which i did not yet entirely understand where it applies. I tried to factor it out such that they can be distinguished easily when reading. Please find a preliminary implementation in a gist (still contains debug code, the upper 16 bits of rev are used to provide me with some addittional information right now, all pins are still read unconditionally for this reason as well) here.

[UweBonnes]

But you do not care for UART?
Antdthe baites you have can also work the theSTM stlink firmware?

Reflash the STLINK firmware and use PROBE_HOST=pc-stlinkv2 pc-hosted BMP software!

Have a look at https://github.com/UweBonnes/blackmagic/tree/master/src/platforms/stlink

[KillerLink]

Right now i do not care for the UART.
The "baite" originally had the STM stlink firmware on it, is used it with the linux st-flash utility. I also still have one of the "baite" variant that i did not overflash to BMP.
On a side note, BMP is working on both adapters, i only noticed the misclassification of the revision because the LED on the "baite" did not work.

If i understood correctly, pc-stlinkv2 opens up the gdb server, which acts inbetween the probe and gdb. The nice thing about BMP in my opinion is that i can use gdb directly targeting the BMP over /dev/ttyACM0, which is why i would like to stay on swlink.

Thanks for the link, i will read the information and incorporate the additional knowledge!

[UweBonnes]

With a recent STLINK firmware, you start blackmagic_stlinkv2 on the PC. Some infos about the adapter appear. Now you start arm...-gdb and in gdb you do "target extended :2000" and now you work in gdb as with BMP native. In the console starting blackmagic_stlinkv2 you will see the BMP debug messages. I will try to reflash a baite too.

[KillerLink]

So i did what you suggested, using blackmagic_stlinkv2 to obtain additional information. I used the another "baite" which still had it's original firmware (which required an upgrade to a current ST firmware though). It is classified as "STLINKV20". I also found a very helpful website that has a good overview of many existing variants here. One of my two variants can be found there, while the other (my "baite" with 2 leds but only 1 resistor) is very similar but not identical to any of the images.

On a side note, i discovered that i can also use "swlink" build instead of "stlink", with the benifit UART is avaialble on the Connector Pins "RST" and "SWIM" (and actually works). But i do realize that there are more differences here. But it might make sense to allow this remap for the stlink build aswell, a topic for another day though.

Now to that Legacy F4 Discovery Board you mentioned, what should it be classified as, i'd assume ST-Link V2? Do you happen to know what PB11 does on such a board (floating/high/low)?

EDIT: my current implementation, opinions?

[UweBonnes]

I still do not find your arguments about the advantages the stlink implementation has against the pc-hosted blackmagic_stlinkv2.

[KillerLink]

As already mentioned, i like the convenience that i can use BMP/GDB without any other software running.

Anyways, do you think my suggested implementation would be suitable for a pull request?

[UweBonnes]

There are such a lot of issues and pull-requests around STLink clones, baites and bluepill that my head is spinning. Probably changing one thing for the better for one user will make it worse for others, so any change must be well done. I also do not want to spend much time on these things, as in most situations I feel the pc-stlinkv2 branch is the better fit for anything that calls itself STLinkV2. Is calling "blackmagic_stlinkv2" really that big deal? No configuration files needed. And with the additional advantage that debug messages appear on the console. An it will work with pyOcd, pystink and other tools too.

B.t.w, what about adding the pullup to PC13 on those broken baites?

[KillerLink]

I saw the other pull requests/issues/your own branch. Thats why i attempted to identify the existing Variants, tabularize the conditions and write the Code in a way that is as understandable, extendable and modular as possible.

No, calling "blackmagic_stlinkv2" is not a big deal, but Stil more than Not calling it, IT occupies another terminal and ist a component more that i need to restart if i mess up somewhere. But this is not a question weather stlink or pc-stlinkv2 is better for me. I thought if i get it to work [better] on additional hardware it would make sense to bring that capabilty back to this repository, possibly also improving.

If this change is not desireable please just say so and i'll close the issue instead.

Ad adding pullup: might be possible, but since there is not even a Pad on PC13 i consider this very hard to do.

[UweBonnes]

Your table is wrong.

• VLdisco is float/float (UM0919)
• V2/1 is pulldown/float (UM1724)
• F4(xx) disco is pulldown/pulldown dispite the schematics, see Figure 6 in UM1472 (R5/6)

As one of your baits is float/float, PC13/14 is not the way to distinguish the type . But what about the SWIM connections that the baites provides. Test here
for (int i = 0; i < 100; i ++)
res = gpio_get(GPIOC, GPIO13);
if (res)
rev = 0;
for the PB9/10 connection as done in the swim platform.
/* Test connectivity between PB9 and PB10 needed for
* original ST software to implement SWIM.

[KillerLink]

You are right, my table is indeed wrong, confused some things. Which is why i asked for feedback in that post. I'll address all your points below and present a new table and implementation.

VLdiscovery with PC13 float/float has a ST-Link V1 aka rev=0 if i understood documentation correctly . I can distinguish with the already read PB11, which floats according to UM0919 for VLdisco and is pulled high on the baite (same method as you do in your stlink branch). I could also implement your suggestion to test connection between PB9 and PB10, but i think that would be less convenient due to it involving 2 pins, or would it provide an advantage i did not realize?

I assume with "V2/1" you mean "ST-Link V2-1" as it is referred to in UM1724, therefore rev=2? Corrected in updated table.

The F4(xx) is now considered as per your comment.

Some of the rows of the previous tables collapsed because of the new information i got from discussion with you and datasheets you referenced, which helped me to understand. With this table, the decisions would be very similar to the current implementation if you take out the "baite" variant, with difference that i'd actually check for floating instead of relying on "pullup + read 1 == floating" and could return an "unrecognized hardware" (or a dedicated default).

Updated Table:

Updated Implementation

[UweBonnes]

PB11 is a good indicator for a V2 Board with Swim capability. I only need to extend the code path when rev0 is detected.

[KillerLink]

Of course it would be possible to just update that specific Code path. I still think it would make sense to restructure the code (Split Up detection and actions, as i did), explicitly check for float (opposed to "is Not pulled down") and improve documentation.

Anyways, from what you said i now take away that these changes are not desired at the moment and will close this issue for now.

[UweBonnes]

A code restructure as first commit at a sensitive place is a large step. Offer the detection of the other baite as a KISS solution first, than let's talk about the possibility of more.

[KillerLink]

The suggested restructuring would have been limited to a single function, not change it's interface, and indicate its intents clearly with extensive comments. From my point of view it made sense to take the opportunity to split detection from action and add documentation. To get code quality right is why i opened this discussion (thanks alot about the hints to hardware variants and documentation!). Reducing it to a smaller change only to collect reputation feels unnecessary to me at this point. However, i do realize that this is a sensitive place, extensive testing is required and i do accept that you do not want to spend much time on this as you mentioned previously.

[grumat]

Just adding my 2 cents:

"Baite" stands for these clones:

The reason is simple. Just look the plastic case bottom:

Note that the case was originally used for USB-ASP for ATMega series of MCUs.