Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Preparations for future STLink-v3 support #863

Conversation

Nightwalker-87
Copy link
Member

No description provided.

Nightwalker-87 and others added 2 commits February 22, 2020 17:50
- Formatting fixes in CHANGELOG.md
- Added comment in .travis.sh
- Update for .travis.yml: gcc/g++ v5->v6
@Nightwalker-87 Nightwalker-87 self-assigned this Feb 24, 2020
@Nightwalker-87 Nightwalker-87 added code/feature-request programmer/STLINK/V3 V3SET / V3MODS / V3MINI / V3MINIE / V3E / V3EC labels Feb 24, 2020
@Nightwalker-87 Nightwalker-87 added this to the v1.6.1 milestone Feb 24, 2020
@Nightwalker-87 Nightwalker-87 removed the request for review from bmarvo February 24, 2020 22:17
@Nightwalker-87
Copy link
Member Author

Merged into develop branch with commit aad5cf1.
Related to issue #820.

@bmarvo
Copy link

bmarvo commented Feb 26, 2020

@Nightwalker-87 did you merge this into develop manually? What is the status of this PR? There are conflicts.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented Feb 26, 2020

@bmarvo: Yes, and I'm aware of that. I'll take a closer look at it after my next outstanding commit. It has mostly to do with patches in the changelog as a result of the ongoing ticket review. The rest is solely whitespace stuff.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented Feb 26, 2020

Conflicts resolved and content merged with PR #866.

@Nightwalker-87
Copy link
Member Author

@martonmiklos: Thx for your contribution! 👍

@Nightwalker-87 Nightwalker-87 changed the title Add stlink v3 support STLink-v3: Add stlink v3 support Mar 17, 2020
@Nightwalker-87 Nightwalker-87 changed the title STLink-v3: Add stlink v3 support [feature] STLink-v3: Add stlink v3 support Mar 27, 2020
@Nightwalker-87
Copy link
Member Author

@martonmiklos: It seems I have messed up proper merging of this last time.
Can you resolve the conflicts in your branch to allow for a clean merge?
You may also add further commits, should you have made any progress on this topic since then.

@martonmiklos
Copy link
Collaborator

Well for me it was quite a surprise that you merged the V3 support upstream, I just stated in the #820 that some testing needed not that it is finished. Anyway do not worry, I will figure this out.

@grevaillot
Copy link
Collaborator

i've been using a stlinkv3 recently, on a g474 board, and except a strange trace at start :

INFO common.c: *** stlink_force_debug_mode ***
INFO common.c: *** stlink_status ***
core status: unknown

that need to be taken care of, i've had no catastrophic issues.

src/usb.c Outdated Show resolved Hide resolved
@Nightwalker-87 Nightwalker-87 linked an issue Apr 18, 2020 that may be closed by this pull request
@Nightwalker-87
Copy link
Member Author

@martonmiklos: Could you add a missing comment related to #271 in /src/usb.c?
Any further news on this topic?

@Nightwalker-87 Nightwalker-87 linked an issue Apr 18, 2020 that may be closed by this pull request
6 tasks
@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented Apr 18, 2020

I've just found that merge-commit aad5cf1 accidentally introduced a bug on my STlink-v2 (clone) device. Entering st-info --probe gives the following output:

[!] send_recv read reply failed: LIBUSB_ERROR_PIPE
[!] send_recv STLINK_DEBUG_RESETSYS
[!] send_recv read reply failed: LIBUSB_ERROR_PIPE
[!] send_recv STLINK_DEBUG_READCOREID
[!] send_recv read reply failed: LIBUSB_ERROR_PIPE
[!] send_recv STLINK_JTAG_READDEBUG_32BIT
Found 1 stlink programmers

in contrast to the previous commit:

Found 1 stlink programmers

This error is reproducible as often as I retry, recompile or plug/unplug the device, so one can assume something got screwed up there. I haven't found the exact place in the code yet, but fortunately we already know exactly in which commit it appeared...

I am posting this here, because this commit derived from the same branch as this PR.

Flash programming is broken
@martonmiklos martonmiklos force-pushed the add_stlink_v3_support branch from f7344a8 to 09ea99a Compare April 21, 2020 21:11
@Nightwalker-87
Copy link
Member Author

@martonmiklos: Thank you. This is a lot better now. 👍
However there are still some compile errors with clang and on macOS that want to be addressed.

@chenguokai
Copy link
Collaborator

chenguokai commented Apr 22, 2020

The current compile errors are again associated with the 64 to 32 conversion. May need a unified way of dealing with size_t. Too many casts are really a burden.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 6, 2020

If no further interest appears (resulting in somehow active contribution) to fully implement this feature for the upcoming release, then we will leave it at that and push the related ticket to a later milestone. This feature will not block our next release, nor should this PR remain open for much longer. Also orphaned PRs for incomplete features do not comply with our contribution guidelines.

I've merged this into a separate branch to address remaining issues.

@Nightwalker-87 Nightwalker-87 requested a review from chenguokai May 6, 2020 23:40
@Nightwalker-87 Nightwalker-87 changed the base branch from develop to stlink-v3_pre May 7, 2020 00:02
@Nightwalker-87 Nightwalker-87 removed the request for review from chenguokai May 7, 2020 00:11
@Nightwalker-87 Nightwalker-87 self-assigned this May 7, 2020
@Nightwalker-87 Nightwalker-87 changed the title [feature] STLink-v3: Add stlink v3 support [feature] Preparations for future STLink-v3 support May 7, 2020
@Nightwalker-87
Copy link
Member Author

Merged into stlink-v3_pre branch with commit 442ade8.

@Nightwalker-87
Copy link
Member Author

The current compile errors are again associated with the 64 to 32 conversion. May need a unified way of dealing with size_t. Too many casts are really a burden.

@chenguokai: Indeed. I'm currently trying to deal with this. Maybe a change in stlink.h could be the key to success. Nevertheless suggestions are welcome.

@martonmiklos: We also need to know what what has been addressed here (short summary or listing) and what parts are still missing as of the current state of knowledge. You should know best.

@martonmiklos
Copy link
Collaborator

ST implemented two different JTAG command API version: V1 and V2. See here APIV1 suffixed commands in the OpenOCD code:
https://github.com/ntfreak/openocd/blob/master/src/jtag/drivers/stlink_usb.c#L227

The stlink uses only the V1 commands and it seems that ST dropped the support for them in the STLinkV3 firmware.

As it turns out from the openOCD code the V2 API was even supported by the STLinkV1 above a certain version:
https://github.com/ntfreak/openocd/blob/master/src/jtag/drivers/stlink_usb.c#L991

Apart from utilising the V2 command API there are some other minor differences in the V3 handling like extended version query, which generally seems to do it's job.

At the moment most of the commands are working, however the flash loaded never hits a breakpoint after loading as I mentioned somewhere else (sometimes for me it is hard to follow the multiple PRs/issues associated with the V3).

I have traced the SWD with a logic analyzer where I see a small amount of difference between the V2/V3 traces. However I would need to route the stlink cmdline output to a serial line which could be used to track down where exactly happens the difference.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 9, 2020

I have traced the SWD with a logic analyzer where I see a small amount of difference between the V2/V3 traces. However I would need to route the stlink cmdline output to a serial line which could be used to track down where exactly happens the difference.

We shall address this within #820 at a later point in time where the breakpoint issue is also mentioned. This issue shall remain the only one dealing with ST-Link-v3 from now on.

From your preceding explanations I read that basic support/compatibility seems to be present already (though not sufficiently tested yet). I have added the key points of your descriptions to a description for an upcoming PR which covers all changes that have been made here and will include a solution for the 64 to 32 conversion issue to ensure all build tests will pass.

@chenguokai
Copy link
Collaborator

Maybe a change in stlink.h could be the key to success. Nevertheless suggestions are welcome.

I would suggest unifying the return value types. For example, a ssize_t return value conflicts with an int variable. In most cases, an int works well enough, since ssize_t itself is variable in length and contains sign info, just like a simple int. When a fixed length variable is in need, uintX_t or intX_t will do.

@Nightwalker-87
Copy link
Member Author

I left a note on that in #954.
Thus we should close this long conversation here now, as all points have now been addressed.

Nightwalker-87 added a commit that referenced this pull request May 9, 2020
- Comment for selection of API-Version (#271)
- Whitespace cleanup
- Fixed casting (-Wshorten-64-to-32)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Protocol V1 and V2 difference
5 participants