Feature/wch link support #2061

perigoso · 2025-01-23T00:12:09Z

Detailed description

This is the first part of #1399 which will be broken up into smaller PR's

This adds support for the WCH-Link debug probe by WCH, it's a naive implementation based on a rough reverse engineer of the protocol, which is not publicly documented

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 (see Building the firmware)
It builds as BMDA (see Building the BMDA)
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

Since spec v0.13 there is an explicit version value (15) to identify dm/dmi's that do not conform with any available specs

Identify but allow non-standard DMI versions but only process standard explicitly know DM versions

…tion

dragonmux

Do please update all the copyright strings across all the files to include the current year date. Eg 2023-2025 instead of just 2023.

This is only a preliminary review - we still have two files to go through, but this is looking great so far.

dragonmux · 2025-01-23T05:42:35Z

src/meson.build

+	libbmd_core_args += ['-DENABLE_RVSWD=1']
+	bmd_core_args += ['-DENABLE_RVSWD=1']


We suggest making this CONFIG_RVSWD to be consistent with the revamped macro naming style. The if just above needs checking as we think the condition is wrong - think it should be checking rvswd_support, not rtt_support.

You are correct, I noticed after pushing

dragonmux · 2025-01-23T15:42:19Z

src/target/riscv_debug.c

 	if (dmi->version == RISCV_DEBUG_UNKNOWN)
 		return;
 	if (dmi->version == RISCV_DEBUG_0_11) {
-		DEBUG_INFO("RISC-V debug v0.11 not presently supported\n");
+		DEBUG_INFO("RISC-V v0.11 DMI is not presently supported\n");


The inclusion of the term "debug" here is very intentional as the specification is the RISC-V Debug specification, not the ISA specification. For this piece of technical clarity in case we wind up needing to specify ISA specification conformance too, we would like to keep including this extra word in all messages involving the DTM, DMI, and DM versioning. Adding this is about the DMI version is good though 👍🏼

Hm, that does make sense, I'll reword it again

dragonmux · 2025-01-24T07:20:29Z

src/target/riscv_debug.c

@@ -586,18 +613,21 @@ static riscv_debug_version_e riscv_dm_version(const uint32_t status)
 	case 0:
 		return RISCV_DEBUG_UNIMPL;
 	case 1:
-		DEBUG_INFO("RISC-V debug v0.11 DM\n");
+		DEBUG_INFO("RISC-V v0.11 Debug Module\n");


Same for these strings as above.

dragonmux · 2025-01-24T07:25:10Z

src/target/ch32vx.c

+}
+
+/* Reads the 96 bit unique id */
+static bool ch32vx_uid_cmd(target_s *const target, const int argc, const char **const argv)


Is this able to use the stm32_common.c stm32_uid() function? Would be neat if it can.

Maybe, a lot of things can be shared with stm32, that's what #1642 was about, but that's a mess now, I'll look but I might push sharing stuff for later

dragonmux · 2025-01-24T07:31:36Z

src/command.c

+	}
+
+	if (!scan_result) {
+		platform_target_clk_output_enable(false);


This line and its identical counterpart below can be lifted out together just above the check for the scan result. That reduces repetition.

dragonmux · 2025-01-24T07:34:16Z

src/command.c

-			if (!scan_result)
+			if (!scan_result) {
 				gdb_out("SWD scan found no devices.\n");
+#if defined(ENABLE_RVSWD) && defined(PLATFORM_HAS_RVSWD)
+#if CONFIG_BMDA == 1
+				scan_result = bmda_rvswd_scan();
+#else
+				scan_result = false;
+#endif
+				if (!scan_result)
+					gdb_out("RVSWD scan found no devices.\n");
+#endif
+			}


Given how nested this is getting, perhaps lets invert this logic by extracting the post-scan actions into their own function (this allows all scan routines to stop duplicating those same actions) so we can check if (scan_result) and then return via that post-scan routine?

I have some ideas to help with that and make it more future proof too, but for now some minor refactor in that direction wouldn't hurt

hvegh · 2025-01-30T23:25:20Z

I have a CH32V103 and CH32V307 device to play around with.
Maybe I can help.

I have been able to flash the feature/wch-link-support on a blue-pill board.

DOCS:

For reference and a note to self, and maybe other players:

git clone https://github.com/perigoso/blackmagic.git --branch feature/wch-link-support
get toolchain arm-none-eabi in your path
Compile & flash:

meson setup build --cross-file cross-file/bluepill.ini  \
       -Dtargets=cortexm,riscv32,riscv64,gd32,ch32,ch32v,sam,stm,ti \
       -Drvswd_support=true
meson compile -C build
cd build
ninja boot-bin
st-flash --flash=0x20000 write blackmagic_bluepill_bootloader.bin 0x08000000
st-flash --flash=0x20000 write blackmagic_bluepill_firmware.bin  0x08002000

Add the project udev rules
Connect pins:

TARGET          blackmagic probe on bluepill board 
GND             GND
SWDIO           PB14
SWCLK           PA5
POWER           3.3V

RXD             PA3
TXD             PA2

hvegh · 2025-01-30T23:59:39Z

I found some documentation on the transport layer, maybe it is helpful.

RISC-V QingKeV2 Microprocessor Debug Manual.pdf

dragonmux · 2025-01-31T06:42:28Z

3. Patch cross-file/bluepill.ini:

You should never ever need to do this - these files provide defaults and you can override anything they specify on the meson setup command line - eg, in this case: meson setup --cross-file=cross-file/bluepill.ini -Dtargets=cortexm,riscv32,riscv64,gd32,ch32,ch32v,sam,stm,ti -Drvswd_support=true.

5. Add udev rules:

SUBSYSTEMS=="usb", ATTRS{idVendor}=="1d50", ATTRS{idProduct}=="6018", \
    MODE:="0660", GROUP:="plugdev" \
    SYMLINK+="blackmagic_%n"

Unsure what this is exactly trying to achieve, but we rather recommend instead using the project udev rules as there are two serial interfaces and addressing them by serial number when there is more than one probe attached to a host becomes important.

hvegh · 2025-01-31T09:15:45Z

Thank you @dragonmux, I have updated the comment.

perigoso · 2025-01-31T10:47:46Z

Thanks for the interest! a couple notes on your comment though.

I have a CH32V103 and CH32V307 device to play around with. Maybe I can help.

I have been able to flash the feature/wch-link-support on a blue-pill board.
...

This is a PR for WCH-Link support, their debugger probe, meaning it is only relevant for BMDA builds and flashing a probe will not be useful. Perhaps you have been looking at Feature: WCH RISC-V support #1399 ?

I found some documentation on the transport layer, maybe it is helpful.

RISC-V QingKeV2 Microprocessor Debug Manual.pdf

This documentation is already known, and it refers to the "SDI" WCH one line debug transport used in the CH32V003, the protocol we're missing documentation for is the "RVSWD" their two line debug transport used on most other RISC-V chips like the CH32V103 and CH32V307, but I've already reverse engineered it and have an initial implementation in Feature: WCH RISC-V support #1399

hvegh · 2025-01-31T11:02:59Z

This is a PR for WCH-Link support, their debugger probe, meaning it is only relevant for BMDA builds and flashing a probe will not be useful. Perhaps you have been looking at

The order for the wch-link is no its way from China takes some time.
I was hoping there was already some bitbang support for riscv swd..

Thanks for clarifying

perigoso · 2025-01-31T11:05:56Z

This is a PR for WCH-Link support, their debugger probe, meaning it is only relevant for BMDA builds and flashing a probe will not be useful. Perhaps you have been looking at

The order for the wch-link is no its way from China takes some time. I was hoping there was already some bitbang support for riscv swd..

Thanks for clarifying

There is a bit banging implementation, in #1399 .

hvegh · 2025-01-31T12:05:51Z

There is a bit banging implementation, in #1399 .

That PR is already 2 years pending, is there a way I can help in moving this forward?
And if so what would be the first step, that is acceptable to the maintainers.

I would love to help but if there is no one from maintainer side that is willing to help it will be to much of an uphill battle I think..

Discuss on Discord maybe?

dragonmux · 2025-01-31T12:21:13Z

The main issue that PR has seen has been Perigoso's ability to commit time to it (through no fault of their own) - it's a very interesting PR and we're very much on board with getting it merged in once it's actually ready. If some more people can test it and show it all working properly that will help too.

We have now properly marked it as deferred to v2.1 (we are in a feature freeze for v2.0 which is not long from being released in full), and hopefully between that and this PR we'll be able to get all the necessary support merged in for v2.1 in fairly short order, volunteer time permitting. We're happy to keep providing reviews and being a guiding hand for the process.

hvegh · 2025-01-31T13:56:14Z

I am grateful for any spare time anyone has put in.
So at the moment there is no help wanted, just wait for 2.1 release?

perigoso · 2025-01-31T15:13:18Z

I am grateful for any spare time anyone has put in. So at the moment there is no help wanted, just wait for 2.1 release?

Required, no, not really, but testing would be very welcome :) right now the only things missing before getting what is done already properly submitted is just spit and polish.

SDI implementation will be worked on after these get merged, right now just RVSWD is implemented.

Note that this is just the protocol implementation, target support will come after, you're welcome to work on it for targets you have on hand.

perigoso · 2025-01-31T15:15:24Z

Also, #1399 is indeed 2 years old, but not pending, more like work in progress, it's very much up to date with upstream :)

perigoso added 14 commits January 22, 2025 19:30

hosted: barebones wch-link implementation

7f2a122

hosted/wchlink: implement DMI transfer

4719dd2

riscv_debug: expose DMI operation and status defines

4330f7f

riscv_jtag_dtm: move RV_DMI_TOO_SOON handling one level up

f7bcbb0

riscv_debug: add non standard dm/dmi version enum

3ebb928

Since spec v0.13 there is an explicit version value (15) to identify dm/dmi's that do not conform with any available specs

riscv_debug: identify non-standard dm/dmi's

d355ef4

Identify but allow non-standard DMI versions but only process standard explicitly know DM versions

hosted/wchlink: implement RISC-V DTM handler

86ca7e3

riscv_debug: make dmi op constants more verbose

d7181f8

command: add rvswd scan routine command

49701ab

hosted/wchlink: implement RVSWD scan

15a3766

riscv32: add ch32v3 target probe

6a427b2

ch32vx: implement electronic signature (ESIG) register handling

e088f65

riscv32: add ch32v003 target probe

dbb40bb

rvswd: gate rvswd functionality behind platform define and project op…

c0e8934

…tion

dragonmux added this to the v2.1 release milestone Jan 23, 2025

dragonmux added the Enhancement General project improvement label Jan 23, 2025

dragonmux requested changes Jan 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/wch link support #2061

Feature/wch link support #2061

perigoso commented Jan 23, 2025

dragonmux left a comment

dragonmux Jan 23, 2025

perigoso Jan 27, 2025

dragonmux Jan 23, 2025

perigoso Jan 27, 2025

dragonmux Jan 24, 2025

dragonmux Jan 24, 2025

perigoso Jan 27, 2025

dragonmux Jan 24, 2025

dragonmux Jan 24, 2025

perigoso Jan 27, 2025

hvegh commented Jan 30, 2025 •

edited

Loading

hvegh commented Jan 30, 2025

dragonmux commented Jan 31, 2025

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

hvegh commented Jan 31, 2025 •

edited

Loading

dragonmux commented Jan 31, 2025 •

edited

Loading

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

perigoso commented Jan 31, 2025

		libbmd_core_args += ['-DENABLE_RVSWD=1']
		bmd_core_args += ['-DENABLE_RVSWD=1']

Feature/wch link support #2061

Are you sure you want to change the base?

Feature/wch link support #2061

Conversation

perigoso commented Jan 23, 2025

Detailed description

Your checklist for this pull request

Closing issues

dragonmux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hvegh commented Jan 30, 2025 • edited Loading

hvegh commented Jan 30, 2025

dragonmux commented Jan 31, 2025

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

hvegh commented Jan 31, 2025 • edited Loading

dragonmux commented Jan 31, 2025 • edited Loading

hvegh commented Jan 31, 2025

perigoso commented Jan 31, 2025

perigoso commented Jan 31, 2025

hvegh commented Jan 30, 2025 •

edited

Loading

hvegh commented Jan 31, 2025 •

edited

Loading

dragonmux commented Jan 31, 2025 •

edited

Loading