- Implemented program memory erasure routine in WchRiscV target driver

- Moved away from relying on WCH-Link debug tool command for erasing program memory, due to a bug that I couldn't fix
- Small tweaks to programming method selection in WCH-Link driver
- Refactored flash peripheral registers in WchRiscV target driver

src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp

@@ -241,6 +241,12 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink
         this->sendCommandAndWaitForResponse(Commands::EndProgrammingSession{});
     }
 
+    /*
+     * We don't actually use this anywhere, as the WCH-Link's erase function is buggy. For more, see the comment in
+     * the WchLinkDebugInterface::eraseMemory() member function.
+     *
+     * TODO: Consider removing this after v2.0.0
+     */
     void WchLinkInterface::eraseProgramMemory() {
         this->sendCommandAndWaitForResponse(Commands::EraseProgramMemory{});
     }

src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp

@@ -234,13 +234,12 @@ namespace DebugToolDrivers::Wch
              * the target. But the partial block write is faster and more suitable for writing buffers that are
              * smaller than 64 bytes, such as when we're inserting software breakpoints.
              */
-
             if (
-                buffer.size() <= WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE
+                buffer.size() > (WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE * 3)
-                || !this->fullBlockWriteCompatible(addressSpaceDescriptor, memorySegmentDescriptor, startAddress)
+                && this->fullBlockWriteCompatible(addressSpaceDescriptor, memorySegmentDescriptor, startAddress)
             ) {
-                Logger::debug("Using partial block write method");
+                Logger::debug("Using full block write method");
-                return this->writeProgramMemoryPartialBlock(
+                return this->writeProgramMemoryFullBlock(
                     addressSpaceDescriptor,
                     memorySegmentDescriptor,
                     startAddress,
@@ -248,8 +247,8 @@ namespace DebugToolDrivers::Wch
                 );
             }
 
-            Logger::debug("Using full block write method");
+            Logger::debug("Using partial block write method");
-            return this->writeProgramMemoryFullBlock(
+            return this->writeProgramMemoryPartialBlock(
                 addressSpaceDescriptor,
                 memorySegmentDescriptor,
                 startAddress,
@@ -269,21 +268,40 @@ namespace DebugToolDrivers::Wch
         const TargetAddressSpaceDescriptor& addressSpaceDescriptor,
         const TargetMemorySegmentDescriptor& memorySegmentDescriptor
     ) {
-        if (memorySegmentDescriptor == this->mainProgramSegmentDescriptor) {
+        /*
-            return this->wchLinkInterface.eraseProgramMemory();
+         * WCH-Link tools provide an erase function that erases the entire user section (main program segment).
-        }
+         * However, this function doesn't always work. Sometimes, it silently fails and leaves the target in a bad
-
+         * state, causing further issues (like program memory corruption).
-        Logger::debug("Ignoring erase operation on `" + memorySegmentDescriptor.key + "` segment - not supported");
+         *
+         * I spent a long time trying to fix this, but all attempts failed. I then decided to cease all use of this
+         * function and just implement the erase procedure myself, in the target driver.
+         *
+         * See the WchRiscV::eraseMainFlashSegment() member function for more.
+         *
+         * For future reference, my notes on this issue:
+         *
+         * - I discovered the issue with a WCH-LinkE, FW version 2.9, connected to a CH32V003. I'm not sure if other
+         *   targets are affected
+         *
+         * - The issue occurs rarely - a very specific sequence of events must take place for the issue to occur:
+         *     1. Perform exactly one partial block write, to the main program segment, at address 0x08000100, with
+         *        exactly 64 bytes of data
+         *     2. Reset the target
+         *     3. Erase the target via the erase function provided by the WCH-Link tool
+         *   The erase operation will silently fail, and the target will be left in a bad state, causing the subsequent
+         *   full block write to corrupt the target's program memory
+         *
+         * - The reset is what causes the erase operation to fail. But I have no idea why. I've inspected the relevant
+         *   registers, at the relevant times, but have found nothing significant that could explain this
+         *
+         * - Subsequent resets do not fix the issue, but another full block write does. My guess is that the first full
+         *   block write (which corrupted program memory) corrected the target state, cleaning the mess made by the
+         *   failed erase operation
+         */
+        throw TargetOperationFailure{"Not supported"};
     }
 
     void WchLinkDebugInterface::enableProgrammingMode() {
-        /*
-         * Nothing to do here
-         *
-         * We cannot prepare the WCH-Link tool for a programming session here, as the preparation process differs
-         * across the two types of flash write commands (full and partial block write). We don't know which command
-         * we'll be utilising at this point.
-         */
     }
 
     void WchLinkDebugInterface::disableProgrammingMode() {

src/Targets/DynamicRegisterValue.cpp

@@ -31,6 +31,13 @@ namespace Targets
         assert(registerDescriptor.size <= 8);
     }
 
+    DynamicRegisterValue& DynamicRegisterValue::operator = (const DynamicRegisterValue& other) {
+        assert(other.registerDescriptor == this->registerDescriptor);
+        this->value = other.value;
+
+        return *this;
+    }
+
     void DynamicRegisterValue::setBitField(
         const TargetBitFieldDescriptor& bitFieldDescriptor,
         std::uint64_t inputValue

src/Targets/DynamicRegisterValue.hpp

@@ -19,6 +19,8 @@ namespace Targets
         DynamicRegisterValue(const TargetRegisterDescriptor& registerDescriptor, TargetMemoryBufferSpan value);
         DynamicRegisterValue(const TargetRegisterDescriptor& registerDescriptor, std::uint64_t value);
 
+        DynamicRegisterValue& operator = (const DynamicRegisterValue& other);
+
         void setBitField(const TargetBitFieldDescriptor& bitFieldDescriptor, std::uint64_t inputValue);
         void setBitField(const std::string& bitFieldKey, std::uint64_t inputValue);
 

src/Targets/RiscV/Wch/Peripherals/Flash/FlashControlRegisterFields.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+#include "src/Targets/TargetBitFieldDescriptor.hpp"
+#include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp"
+
+namespace Targets::RiscV::Wch::Peripherals::Flash
+{
+    struct FlashControlRegisterFields
+    {
+        const TargetBitFieldDescriptor& locked;
+        const TargetBitFieldDescriptor& startErase;
+        const TargetBitFieldDescriptor& mainSegmentErase;
+    };
+}

src/Targets/RiscV/Wch/Peripherals/Flash/FlashStatusRegisterFields.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+#include "src/Targets/TargetBitFieldDescriptor.hpp"
+#include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp"
+
+namespace Targets::RiscV::Wch::Peripherals::Flash
+{
+    struct FlashStatusRegisterFields
+    {
+        const TargetBitFieldDescriptor& busy;
+        const TargetBitFieldDescriptor& bootLock;
+        const TargetBitFieldDescriptor& bootMode;
+    };
+}

src/Targets/RiscV/Wch/WchRiscV.cpp

@@ -2,6 +2,8 @@
 
 #include <utility>
 #include <cassert>
+#include <chrono>
+#include <thread>
 
 #include "src/Targets/DynamicRegisterValue.hpp"
 
@@ -12,6 +14,7 @@
 
 #include "src/Services/StringService.hpp"
 #include "src/Logger/Logger.hpp"
+#include "src/TargetController/Exceptions/TargetOperationFailure.hpp"
 
 namespace Targets::RiscV::Wch
 {
@@ -36,8 +39,21 @@ namespace Targets::RiscV::Wch
         , flashKeyRegisterDescriptor(this->flashPeripheralDescriptor.getRegisterDescriptor("flash", "keyr"))
         , flashBootKeyRegisterDescriptor(this->flashPeripheralDescriptor.getRegisterDescriptor("flash", "boot_modekeyr"))
         , flashStatusRegisterDescriptor(this->flashPeripheralDescriptor.getRegisterDescriptor("flash", "statr"))
-        , flashStatusBootLockFieldDescriptor(this->flashStatusRegisterDescriptor.getBitFieldDescriptor("boot_lock"))
+        , flashControlRegisterDescriptor(this->flashPeripheralDescriptor.getRegisterDescriptor("flash", "ctrl"))
-        , flashStatusBootModeFieldDescriptor(this->flashStatusRegisterDescriptor.getBitFieldDescriptor("boot_mode"))
+        , flashControlRegisterFields(
+            Peripherals::Flash::FlashControlRegisterFields{
+                .locked = this->flashControlRegisterDescriptor.getBitFieldDescriptor("lock"),
+                .startErase = this->flashControlRegisterDescriptor.getBitFieldDescriptor("strt"),
+                .mainSegmentErase = this->flashControlRegisterDescriptor.getBitFieldDescriptor("mer"),
+            }
+        )
+        , flashStatusRegisterFields(
+            Peripherals::Flash::FlashStatusRegisterFields{
+                .busy = this->flashStatusRegisterDescriptor.getBitFieldDescriptor("bsy"),
+                .bootLock = this->flashStatusRegisterDescriptor.getBitFieldDescriptor("boot_lock"),
+                .bootMode = this->flashStatusRegisterDescriptor.getBitFieldDescriptor("boot_mode"),
+            }
+        )
         , rccPeripheralDescriptor(this->targetDescriptionFile.getTargetPeripheralDescriptor("rcc"))
         , portPeripheralClockEnableRegisterDescriptor(
             this->rccPeripheralDescriptor.getRegisterDescriptor("rcc", "apb2pcenr")
@@ -321,11 +337,17 @@ namespace Targets::RiscV::Wch
         const TargetAddressSpaceDescriptor& addressSpaceDescriptor,
         const TargetMemorySegmentDescriptor& memorySegmentDescriptor
     ) {
-        if (memorySegmentDescriptor == this->mappedSegmentDescriptor) {
+        if (
-            return RiscV::eraseMemory(addressSpaceDescriptor, this->selectedProgramSegmentDescriptor);
+            memorySegmentDescriptor == this->mainProgramSegmentDescriptor
+            || (
+                memorySegmentDescriptor == this->mappedSegmentDescriptor
+                && this->selectedProgramSegmentDescriptor == this->mainProgramSegmentDescriptor
+            )
+        ) {
+            return this->eraseMainFlashSegment();
         }
 
-        RiscV::eraseMemory(addressSpaceDescriptor, memorySegmentDescriptor);
+        Logger::debug("Ignoring erase operation on `" + memorySegmentDescriptor.key + "` segment - not supported");
     }
 
     TargetMemoryAddress WchRiscV::getProgramCounter() {
@@ -659,11 +681,11 @@ namespace Targets::RiscV::Wch
 
         auto statusRegister = this->readRegisterDynamicValue(this->flashStatusRegisterDescriptor);
 
-        if (statusRegister.bitFieldAs<bool>(this->flashStatusBootLockFieldDescriptor)) {
+        if (statusRegister.bitFieldAs<bool>(this->flashStatusRegisterFields.bootLock)) {
             throw Exceptions::Exception{"Failed to unlock boot mode field"};
         }
 
-        statusRegister.setBitField(this->flashStatusBootModeFieldDescriptor, true);
+        statusRegister.setBitField(this->flashStatusRegisterFields.bootMode, true);
         this->writeRegister(statusRegister);
 
         this->reset();
@@ -675,16 +697,59 @@ namespace Targets::RiscV::Wch
 
         auto statusRegister = this->readRegisterDynamicValue(this->flashStatusRegisterDescriptor);
 
-        if (statusRegister.bitFieldAs<bool>(this->flashStatusBootLockFieldDescriptor)) {
+        if (statusRegister.bitFieldAs<bool>(this->flashStatusRegisterFields.bootLock)) {
             throw Exceptions::Exception{"Failed to unlock boot mode field"};
         }
 
-        statusRegister.setBitField(this->flashStatusBootModeFieldDescriptor, false);
+        statusRegister.setBitField(this->flashStatusRegisterFields.bootMode, false);
         this->writeRegister(statusRegister);
 
         this->reset();
     }
 
+    void WchRiscV::eraseMainFlashSegment() {
+        static constexpr auto ERASE_RESPONSE_DELAY = std::chrono::microseconds{10};
+        static constexpr auto ERASE_TIMEOUT = std::chrono::milliseconds{100};
+
+        this->unlockFlash();
+
+        auto statusRegister = this->readRegisterDynamicValue(this->flashStatusRegisterDescriptor);
+        if (statusRegister.bitFieldAs<bool>(this->flashStatusRegisterFields.busy)) {
+            throw Exceptions::TargetOperationFailure{"Flash peripheral is unavailable"};
+        }
+
+        auto controlRegister = this->readRegisterDynamicValue(this->flashControlRegisterDescriptor);
+        if (controlRegister.bitFieldAs<bool>(this->flashControlRegisterFields.locked)) {
+            throw Exceptions::TargetOperationFailure{"Failed to unlock flash"};
+        }
+
+        // It seems we have to write these bit fields individually. Otherwise, the target misbehaves
+        controlRegister.setBitField(this->flashControlRegisterFields.mainSegmentErase, true);
+        this->writeRegister(controlRegister);
+        controlRegister.setBitField(this->flashControlRegisterFields.startErase, true);
+        this->writeRegister(controlRegister);
+
+        statusRegister = this->readRegisterDynamicValue(this->flashStatusRegisterDescriptor);
+        for (
+            auto attempts = 0;
+            statusRegister.bitFieldAs<bool>(this->flashStatusRegisterFields.busy)
+                && (ERASE_RESPONSE_DELAY * attempts) <= ERASE_TIMEOUT;
+            ++attempts
+        ) {
+            std::this_thread::sleep_for(ERASE_RESPONSE_DELAY);
+            statusRegister = this->readRegisterDynamicValue(this->flashStatusRegisterDescriptor);
+        }
+
+        controlRegister = this->readRegisterDynamicValue(this->flashControlRegisterDescriptor);
+        controlRegister.setBitField(this->flashControlRegisterFields.mainSegmentErase, false);
+        controlRegister.setBitField(this->flashControlRegisterFields.startErase, false);
+        this->writeRegister(controlRegister);
+
+        if (statusRegister.bitFieldAs<bool>(this->flashStatusRegisterFields.busy)) {
+            throw Exceptions::TargetOperationFailure{"Erase operation timed out"};
+        }
+    }
+
     std::map<TargetPadId, GpioPadDescriptor> WchRiscV::generateGpioPadDescriptorMapping(
         const TargetRegisterDescriptor& portPeripheralClockEnableRegisterDescriptor,
         const std::vector<TargetPeripheralDescriptor>& portPeripheralDescriptors,

src/Targets/RiscV/Wch/WchRiscV.hpp

@@ -12,11 +12,16 @@
 
 #include "WchRiscVTargetConfig.hpp"
 #include "TargetDescriptionFile.hpp"
+
 #include "GpioPadDescriptor.hpp"
+#include "Peripherals/Flash/FlashControlRegisterFields.hpp"
+#include "Peripherals/Flash/FlashStatusRegisterFields.hpp"
 
 namespace Targets::RiscV::Wch
 {
-    class WchRiscV: public ::Targets::RiscV::RiscV
+    class WchRiscV
+        : public ::Targets::RiscV::RiscV
+        , public DeltaProgrammingInterface
     {
     public:
         WchRiscV(const TargetConfig& targetConfig, TargetDescriptionFile&& targetDescriptionFile);
@@ -88,9 +93,10 @@ namespace Targets::RiscV::Wch
         const TargetRegisterDescriptor& flashKeyRegisterDescriptor;
         const TargetRegisterDescriptor& flashBootKeyRegisterDescriptor;
         const TargetRegisterDescriptor& flashStatusRegisterDescriptor;
+        const TargetRegisterDescriptor& flashControlRegisterDescriptor;
 
-        const TargetBitFieldDescriptor& flashStatusBootLockFieldDescriptor;
+        const Peripherals::Flash::FlashControlRegisterFields flashControlRegisterFields;
-        const TargetBitFieldDescriptor& flashStatusBootModeFieldDescriptor;
+        const Peripherals::Flash::FlashStatusRegisterFields flashStatusRegisterFields;
 
         const TargetPeripheralDescriptor rccPeripheralDescriptor;
         const TargetRegisterDescriptor& portPeripheralClockEnableRegisterDescriptor;
@@ -109,6 +115,7 @@ namespace Targets::RiscV::Wch
         void unlockBootModeBitField();
         void enableBootMode();
         void enableUserMode();
+        void eraseMainFlashSegment();
 
         static std::map<TargetPadId, GpioPadDescriptor> generateGpioPadDescriptorMapping(
             const TargetRegisterDescriptor& portPeripheralClockEnableRegisterDescriptor,

src/Targets/TargetDescriptionFiles/RiscV/Wch/CH32V003.xml

@@ -1932,7 +1932,7 @@
                     <bit-field key="wrprterr" name="WRPRTERR" description="Write Protected Error Flag" mask="0x00000010" access="RW"/>
                     <bit-field key="bsy" name="BSY" description="Busy Status" mask="0x00000001" access="R"/>
                 </register>
-                <register key="ctlr" name="CTLR" description="Control" offset="0x10" size="4" initial-value="0x00008080" access="RW">
+                <register key="ctrl" name="CTRL" description="Control" offset="0x10" size="4" initial-value="0x00008080" access="RW">
                     <bit-field key="bufrst" name="BUFRST" description="Reset BUF Cache" mask="0x00080000" access="RW"/>
                     <bit-field key="bufload" name="BUFLOAD" description="BUF Cache Enable" mask="0x00040000" access="RW"/>
                     <bit-field key="fter" name="FTER" description="Select Fast Page Erase Operation" mask="0x00020000" access="RW"/>

src/Targets/TargetDescriptionFiles/RiscV/Wch/CH32X035.xml

@@ -3227,7 +3227,7 @@
                     <bit-field key="errie" name="ERRIE" description="Error Interrupt Enable" mask="0x00000400" access="RW"/>
                     <bit-field key="obwre" name="OBWRE" description="Flash Address Locked Flag" mask="0x00000200" access="RW"/>
                     <bit-field key="lock" name="LOCK" description="FPEC and CTRL Write-lock Enable" mask="0x00000080" access="RW"/>
-                    <bit-field key="strt" name="STRT" description="Start Operation" mask="0x00000040" access="RW"/>
+                    <bit-field key="strt" name="STRT" description="Start Erase Operation" mask="0x00000040" access="RW"/>
                     <bit-field key="ober" name="OBER" description="Select Word Erase Operation" mask="0x00000020" access="RW"/>
                     <bit-field key="obpg" name="OBPG" description="Select Word Programming Operation" mask="0x00000010" access="RW"/>
                     <bit-field key="mer" name="MER" description="Select User Area Erase Operation" mask="0x00000004" access="RW"/>