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: WCH RISC-V support #1399

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

perigoso
Copy link
Contributor

@perigoso perigoso commented Feb 23, 2023

Detailed description

Support support for WCH RISC-V family of MCUs

WCH uses a custom DTM (Debug Transport Module) for their RISC-V MCUs, RVSWD a 2 wire SWD "like" protocol (not SWD), and a SDI 1 wire protocol

RVSWD is currently not publlicly documented, part of this implementation is the reverse engineering of the protocol, resulting documentation will be hosted here

Sigrok protocol decoder for RVSWD here

SDI documentation has been recently published, but implementation of will probably come as a separate PR
The only SDI MCU at the moment is the CH32V003, all other WCH RISC-V MCUs use RVSWD

As part of the process support for the WCH-Link debug probe by WCH is also implemented, it's a naive implementation based on a rough reverse engineer of the protocol, which too is not publicly documented

This PR might be broken down in the future

As this PR is built on top of feature/esp32-c3-support it might be hard to read, try this diff (feature/esp32-c3-support <- feature/wch-support)

Discord thread

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

This PR is targeted at v2.0 milestone

Depends on #292 and feature/esp32-c3-support

Closes #1576

@dragonmux
Copy link
Member

Please rebase this on feature/esp32-c3-support when you get a chance.

@koendv
Copy link
Contributor

koendv commented Dec 13, 2023

This is a PR I would not mind testing, But it needs rebasing.

@perigoso
Copy link
Contributor Author

perigoso commented Dec 13, 2023

This is a PR I would not mind testing, But it needs rebasing.

You can test as is, it should compile and work (and would be welcome!), that said it does need rebasing, but I'm tight on free time for a bit. Maybe at the begining of the new year.

@perigoso perigoso force-pushed the feature/wch-support branch 3 times, most recently from 828ef60 to 820b35c Compare December 15, 2023 18:13
@perigoso
Copy link
Contributor Author

@koendv Got some time and rebased this on main. Unfortunately the last commit is extremely experimental and not in a state for a clean rebase, so I just reverted the files to the state on the original PR, meaning it reverts some probably important changes on the current main, beware! It should work for scan of CH32 Risc-V chips with the WCH-LinkE, I just tested.

@koendv
Copy link
Contributor

koendv commented Dec 15, 2023 via email

Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently we started a review and then failed to ever actually post it, so here we are - it's incomplete, but a start.

@@ -51,6 +51,7 @@ static bool cmd_help(target_s *t, int argc, const char **argv);

static bool cmd_jtag_scan(target_s *target, int argc, const char **argv);
static bool cmd_swd_scan(target_s *target, int argc, const char **argv);
static bool cmd_rvswd_scan(target_s *target, int argc, const char **argv);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be defined contingent on ENABLE_RISCV and we'd suggest that, at least for now, this be limited to BMDA. Iff we can figure out how to implement the protocol in the firmware we can then de-restrict it by removing the suggested check for PC_HOSTED.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I will consider how to gate this when it's more complete, there will still be a native implementation

src/platforms/hosted/wchlink.c Outdated Show resolved Hide resolved
src/platforms/hosted/wchlink.c Outdated Show resolved Hide resolved
src/platforms/hosted/wchlink.c Outdated Show resolved Hide resolved
break;
}
bmda_probe_info.usb_link->interface = idx;
descriptor = interface_desc;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not missing a break; after this assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was copied from JLink, which also looks like it's missing a break, I refactored it here, but it's pending on testing still.

src/platforms/hosted/wchlink.c Outdated Show resolved Hide resolved
@perigoso perigoso force-pushed the feature/wch-support branch 4 times, most recently from c94ceed to bbdc854 Compare January 17, 2024 15:07
@mean00
Copy link

mean00 commented Aug 9, 2024

In case it is helpful to make things move forward , please find attached
a partially reverse engineered RVSWD/DM protocol document (jpeg image). it is incomplete but enough to make it work.

( as an example, there is a driver written in basic c++ here : https://github.com/mean00/swindle/blob/main/swindle/src/bmp_rvTap.cpp , it could be used as a starting point to write a native BMP driver. It is full of #define macros due to popular demand. It works with CH32V2 and CH32V3)

rvswd

Thank you.

@perigoso
Copy link
Contributor Author

perigoso commented Nov 7, 2024

Thank you very much @mean00, I'm planing on picking this up again soon, but it might take some time to get back up to speed

Used to mark codes that are not part of the JEP106 or any other standard
but are used internally by BMD to identify targets that don't provide a
usable identification codes
write_char writes a single char into a char buffer at the provided offset, returns the offset incremented by 1
Memory safety safeguards are in place, if no buffer is provided, a buffer size of 0 is given, or the offset is out of bounds of the buffer, the buffer is not accessed, but the offset is still incremented

Target use case is using in special print functions behaving similar to snprintf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Target New debug target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants