diff --git a/src/DebugToolDrivers/WCH/Protocols/WchLink/Commands/Control/PostAttach.hpp b/src/DebugToolDrivers/WCH/Protocols/WchLink/Commands/Control/PostAttach.hpp new file mode 100644 index 00000000..24482974 --- /dev/null +++ b/src/DebugToolDrivers/WCH/Protocols/WchLink/Commands/Control/PostAttach.hpp @@ -0,0 +1,20 @@ +#pragma once + +#include + +#include "src/DebugToolDrivers/WCH/Protocols/WchLink/Commands/Command.hpp" + +namespace DebugToolDrivers::Wch::Protocols::WchLink::Commands::Control +{ + class PostAttach: public Command> + { + public: + PostAttach() + : Command(0x0d) + { + this->payload = { + 0x03 + }; + } + }; +} diff --git a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.cpp b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.cpp index c8145493..38d4c798 100644 --- a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.cpp +++ b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.cpp @@ -6,6 +6,7 @@ #include "Commands/Control/GetDeviceInfo.hpp" #include "Commands/Control/AttachTarget.hpp" #include "Commands/Control/DetachTarget.hpp" +#include "Commands/Control/PostAttach.hpp" #include "Commands/SetClockSpeed.hpp" #include "Commands/DebugModuleInterfaceOperation.hpp" #include "Commands/PreparePartialFlashPageWrite.hpp" @@ -61,16 +62,47 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink void WchLinkInterface::activate() { this->setClockSpeed(WchLinkTargetClockSpeed::CLK_6000_KHZ); - const auto response = this->sendCommandAndWaitForResponse(Commands::Control::AttachTarget{}); + auto response = this->sendCommandAndWaitForResponse(Commands::Control::AttachTarget{}); if (response.payload.size() != 5) { throw Exceptions::DeviceCommunicationFailure{"Unexpected response payload size for AttachTarget command"}; } - this->cachedTargetId = static_cast( + this->cachedTargetId = response.payload[0]; + + /* + * For some WCH targets, we must send another command to the debug tool, immediately after attaching. + * + * I don't know what this post-attach command does. But what I *do* know is that the target and/or the debug + * tool will misbehave if we don't send it immediately after the attach. + * + * More specifically, the debug tool will read an invalid target variant ID upon the mutation of the target's + * program buffer. So when we write to progbuf2, progbuf3, progbuf4 or progbuf5, all subsequent reads of the + * target variant ID will yield invalid values, until the target and debug tool have been power cycled. + * Interestingly, when we restore those progbuf registers to their original values, the reading of the target + * variant ID works again. So I suspect the debug tool is using the target's program buffer to read the + * variant ID, but it's assuming the program buffer hasn't changed. Maybe. + * + * So how does this post-attach command fix this issue? I don't know. I just know that it does. + * + * In addition to sending the post-attach command, we have to send another attach command, because the target + * variant ID returned in the response of the first attach command may be invalid. Sending another attach + * command will ensure that we have a valid target variant ID. + */ + if (this->cachedTargetId == 0x09) { + this->sendCommandAndWaitForResponse(Commands::Control::PostAttach{}); + response = this->sendCommandAndWaitForResponse(Commands::Control::AttachTarget{}); + + if (response.payload.size() != 5) { + throw Exceptions::DeviceCommunicationFailure{ + "Unexpected response payload size for subsequent AttachTarget command" + }; + } + } + + this->cachedVariantId = static_cast( (response.payload[1] << 24) | (response.payload[2] << 16) | (response.payload[3] << 8) | (response.payload[4]) ); - this->cachedTargetGroupId = response.payload[0]; } void WchLinkInterface::deactivate() { @@ -81,7 +113,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink } std::string WchLinkInterface::getDeviceId() { - return "0x" + Services::StringService::toHex(this->cachedTargetId.value()); + return "0x" + Services::StringService::toHex(this->cachedVariantId.value()); } DebugModule::RegisterValue WchLinkInterface::readDebugModuleRegister(DebugModule::RegisterAddress address) { @@ -261,7 +293,7 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink }; const auto response = this->sendCommandAndWaitForResponse( - Commands::SetClockSpeed{this->cachedTargetGroupId.value_or(0x01), speedIdsBySpeed.at(speed)} + Commands::SetClockSpeed{this->cachedTargetId.value_or(0x01), speedIdsBySpeed.at(speed)} ); if (response.payload.size() != 1) { diff --git a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp index 3aeaeaa9..e69c9116 100644 --- a/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp +++ b/src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp @@ -75,17 +75,12 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink /** * The 'target activation' command returns a payload of 5 bytes. * - * The last 4 bytes hold the WCH RISC-V target ID. Given that the 'target activation' command appears to be - * the only way to obtain the target ID, we cache it via WchLinkInterface::cachedTargetId and return the - * cached value in WchLinkInterface::getTargetId(). - * - * As for the first byte in the payload, I'm not really sure what it is. It appears to be some kind of - * identifier for groups of WCH RISC-V targets. It's unclear. All I know is that it has some significance, as - * it's expected in the payload of some other commands, such as the command to set clock speed. For this - * reason, we have to keep hold of it via WchLinkInterface::cachedTargetGroupId. + * The last 4 bytes hold the WCH target variant ID. Given that the 'target activation' command appears to be + * the only way to obtain this ID, we cache it via WchLinkInterface::cachedVariantId and return the cached + * value in WchLinkInterface::getTargetId(). */ + std::optional cachedVariantId; std::optional cachedTargetId; - std::optional cachedTargetGroupId; void setClockSpeed(WchLinkTargetClockSpeed speed); diff --git a/src/DebugToolDrivers/WCH/WchGeneric.hpp b/src/DebugToolDrivers/WCH/WchGeneric.hpp index b5ee1bbc..3eaf5c31 100644 --- a/src/DebugToolDrivers/WCH/WchGeneric.hpp +++ b/src/DebugToolDrivers/WCH/WchGeneric.hpp @@ -4,7 +4,8 @@ namespace DebugToolDrivers::Wch { - using WchTargetId = std::uint32_t; + using WchTargetId = std::uint8_t; + using WchTargetVariantId = std::uint32_t; enum class WchLinkVariant: std::uint8_t {