GDB load not working for Blue Pill target with CH32F103C8T6 MCU

[lasutek]

Gdb load to a Blue Pill board with a CH32F103C8T6 MCU will cause gdb to hang and use a lot of CPU.

Text on MCU:
CH32F103
C8T6
401525A20
WCH

ST-LINK V2 (cheap clone) with APM32F103CBT6 MCU

Black Magic git commit: cd1ef38

Gdb session:
arm-none-eabi-gdb -ex "target extended-remote /dev/ttyACM0"
monitor swdp_scan
attach 1
file main.elf
load

I have fix for this problem (use it locally as hobbyist), but have no idea if it have bad side effects or even is suitable for the Black Magic project. The “patch” affects the src/target/ch32f1.c file. Out of the four fixes only two, number three and four are necessary.

1. Typo/spelling
2. Remove “annoying” console messages
3. MCU does not have any info about flash size; set flash size to 64 kB
4. MCU need som uniterrupted time; adding platform_delay(1)

diff -ru a b

--- a/src/target/ch32f1.c       2023-02-25 22:18:40.000000000 +0100
+++ b/src/target/ch32f1.c       2023-02-25 22:00:42.066123026 +0100
@@ -125,7 +125,7 @@
                target_mem_write32(t, FLASH_MAGIC, magic);         \
        } while (0)
 
-/* Attempt ynlock ch32f103 in fast mode */
+/* Attempt unlock ch32f103 in fast mode */
 static bool ch32f1_flash_unlock(target_s *t)
 {
        DEBUG_INFO("CH32: flash unlock \n");
@@ -150,9 +150,15 @@
        SET_CR(FLASH_CR_LOCK);
        const uint32_t cr = target_mem_read32(t, FLASH_CR);
        // FLASH_CR_FLOCK_CH32 bit does not exists on *regular* clones and defaults to '0' (see PM0075 for STM32F1xx)
+       /* Don't know about this, possibly missing execution of macro SET_CR(FLASH_CR_FLOCK_CH32)...
+        * Everything works ok, so no change to the logic of this function.
+        * The if statement and DEBUG_WARN message commented out due to annoying console messages.
+        */
+       /*
        if (!(cr & FLASH_CR_FLOCK_CH32)) {
                DEBUG_WARN("Fast lock failed, cr: 0x%08" PRIx32 "\n", cr);
        }
+       */
        return cr & FLASH_CR_FLOCK_CH32;
 }
 
@@ -206,6 +212,11 @@
 
        t->part_id = device_id;
        uint32_t signature = target_mem_read32(t, FLASHSIZE);
+       // Some ch32f103c8t6 MCU's found on Blue Pill boards have a zero (0) in the flash memory capacity register
+       if (signature == 0) {
+               signature = 64;
+               DEBUG_WARN("CH32: FLASHSIZE = 0, assuming CH32F103C8T6 MCU, seting FLASHSIZE = 64\n");
+       }
        uint32_t flashSize = signature & 0xffffU;
 
        target_add_ram(t, 0x20000000, 0x5000);
@@ -260,8 +271,13 @@
 static bool ch32f1_wait_flash_ready(target_s *t, uint32_t addr)
 {
        uint32_t ff = 0;
-       for (size_t i = 0; i < 32U; i++)
+       uint32_t cnt = 0;
+       // Certain ch32f103c8t6 MCU's found on Blue Pill boards need some uninterrupted time (no SWD link activity)
+       platform_delay(1);
+       while ((ff != 0xffffffffU) && (cnt < 32U)) {
                ff = target_mem_read32(t, addr);
+               cnt++;
+       }
        if (ff != 0xffffffffU) {
                DEBUG_WARN("ch32f1 Not erased properly at %" PRIx32 " or flash access issue\n", addr);
                return false;

[dragonmux]

We would be happy to see you open a PR about this though with the following notes about the above patch addressed:

The code base comment style for multi-line comments is:

/*
 * First line
 * Second line
 */

(including the start-of-line *'s)

You might find DEBUG_WARN("Fast lock failed, cr: 0x%08" PRIx32 "\n", cr); to be an annoying message but it is useful to the project for debugging issues with these STM32 clones, so please leave that in.

Please use /* */ style single-line comments rather than C++ style // comments for consistency with the rest of the file

The ch32f1_wait_flash_ready() change is a good first try, but we would prefer to see it expressed as a for loop still as such:

/* Certain CH32F103C's found on Blue Pill boards need some uninterrupted time (no SWD link activity) */
platform_delay(1);
uint32_t ff = 0;
for (size_t cnt = 0; cnt < 32U && ff != 0xffffffffU; ++cnt)
	ff = target_mem_read32(t, addr);

Ideally we would find a better name for that variable named ff, as it's.. not a good name.. but those are our only review notes on the suitability of this. If you would, please open a PR and we will look to merge it into v1.10

[lasutek]

I will see what I can do -- almost never used git or Github, but know what a PR is. I will read up and hopefully come up with something useful in a couple of days.