From 52533e2878dbd96a192723685a1eb6fbceb2c571 Mon Sep 17 00:00:00 2001 From: Nav Date: Tue, 1 Mar 2022 22:40:00 +0000 Subject: [PATCH] Moved AVR8 physicalInterface config extraction out of EDBG driver --- .../EDBG/AVR/EdbgAvr8Interface.cpp | 33 ---------------- .../EDBG/AVR/EdbgAvr8Interface.hpp | 20 ++-------- .../Microchip/AVR/AVR8/Avr8DebugInterface.hpp | 11 +++++- src/Targets/Microchip/AVR/AVR8/Avr8.cpp | 38 ++++++++++++++++++- src/Targets/Microchip/AVR/AVR8/Avr8.hpp | 17 +++++++++ src/Targets/Microchip/AVR/Target.hpp | 3 +- 6 files changed, 68 insertions(+), 54 deletions(-) diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp index 6dc07a5d..4183f484 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp @@ -52,39 +52,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr using Bloom::Targets::TargetRegisters; void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) { - auto physicalInterface = targetConfig.jsonObject.find("physicalInterface")->toString().toLower().toStdString(); - - auto availablePhysicalInterfaces = this->getPhysicalInterfacesByName(); - - if (physicalInterface.empty() - || availablePhysicalInterfaces.find(physicalInterface) == availablePhysicalInterfaces.end() - ) { - throw InvalidConfig("Invalid or missing physical interface config parameter for AVR8 target."); - } - - auto selectedPhysicalInterface = availablePhysicalInterfaces.find(physicalInterface)->second; - - if (selectedPhysicalInterface == PhysicalInterface::DEBUG_WIRE) { - Logger::warning("AVR8 debugWire interface selected - the DWEN fuse will need to be enabled"); - } - - this->physicalInterface = selectedPhysicalInterface; - - if (!this->family.has_value()) { - if (this->physicalInterface == PhysicalInterface::JTAG) { - throw InvalidConfig("The JTAG physical interface cannot be used with an ambiguous target name" - " - please specify the exact name of the target in your configuration file. " - "See " + Paths::homeDomainName() + "/docs/supported-targets" - ); - - } else if (this->physicalInterface == PhysicalInterface::UPDI) { - throw InvalidConfig("The UPDI physical interface cannot be used with an ambiguous target name" - " - please specify the exact name of the target in your configuration file. " - "See " + Paths::homeDomainName() + "/docs/supported-targets" - ); - } - } - this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE); if (targetConfig.jsonObject.contains("disableDebugWirePreDisconnect")) { diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp index 9dfb7134..494ce437 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp @@ -83,6 +83,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr */ void configure(const TargetConfig& targetConfig) override; + void setPhysicalInterface(Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface) override { + this->physicalInterface = physicalInterface; + } + /** * Configures the target family. For some physical interfaces, the target family is required in order * properly configure the EDBG tool. See EdbgAvr8Interface::resolveConfigVariant() for more. @@ -346,22 +350,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr */ bool disableDebugWireOnDeactivate = false; - /** - * Users are required to set their desired physical interface in their Bloom configuration. This would take - * the form of a string, so we map the available options to the appropriate enums. - */ - static inline auto getPhysicalInterfacesByName() { - using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; - - return std::map({ - {"debugwire", PhysicalInterface::DEBUG_WIRE}, - {"debug-wire", PhysicalInterface::DEBUG_WIRE}, - {"pdi", PhysicalInterface::PDI}, - {"jtag", PhysicalInterface::JTAG}, - {"updi", PhysicalInterface::UPDI}, - }); - }; - /** * This mapping allows us to determine which config variant to select, based on the target family and the * selected physical interface. diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp index 9d227175..feec4e58 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp @@ -4,8 +4,10 @@ #include #include "src/Targets/Microchip/AVR/TargetSignature.hpp" -#include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp" #include "src/Targets/Microchip/AVR/AVR8/Family.hpp" +#include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp" +#include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp" + #include "src/Targets/TargetState.hpp" #include "src/Targets/TargetRegister.hpp" #include "src/Targets/TargetMemory.hpp" @@ -54,6 +56,13 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8 */ virtual void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) = 0; + /** + * Sets the selected physical interface. + * + * @param physicalInterface + */ + virtual void setPhysicalInterface(Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface) = 0; + /** * Should accept Avr8 target parameters for configuration of the interface. * diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index d6578b4d..42af9225 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -7,11 +7,11 @@ #include #include -#include "PadDescriptor.hpp" #include "src/Logger/Logger.hpp" +#include "src/Helpers/Paths.hpp" + #include "src/Exceptions/InvalidConfig.hpp" #include "src/Targets/TargetRegister.hpp" -#include "src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.hpp" // Derived AVR8 targets #include "XMega/XMega.hpp" @@ -25,8 +25,42 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit void Avr8::preActivationConfigure(const TargetConfig& targetConfig) { Target::preActivationConfigure(targetConfig); + auto physicalInterface = targetConfig.jsonObject.find("physicalInterface")->toString().toLower().toStdString(); + auto availablePhysicalInterfaces = Avr8::getPhysicalInterfacesByName(); + + if (physicalInterface.empty() || !availablePhysicalInterfaces.contains(physicalInterface)) { + throw InvalidConfig("Invalid or missing physical interface config parameter for AVR8 target."); + } + + const auto selectedPhysicalInterface = availablePhysicalInterfaces.at(physicalInterface); + + if (selectedPhysicalInterface == PhysicalInterface::DEBUG_WIRE) { + Logger::warning("AVR8 debugWire interface selected - the DWEN fuse will need to be enabled"); + } + + this->physicalInterface = selectedPhysicalInterface; + this->avr8DebugInterface->setPhysicalInterface(this->physicalInterface.value()); + if (this->family.has_value()) { this->avr8DebugInterface->setFamily(this->family.value()); + + } else { + if (this->physicalInterface == PhysicalInterface::JTAG) { + throw InvalidConfig( + "The JTAG physical interface cannot be used with an ambiguous target name" + " - please specify the exact name of the target in your configuration file. " + "See " + Paths::homeDomainName() + "/docs/supported-targets" + ); + + } + + if (this->physicalInterface == PhysicalInterface::UPDI) { + throw InvalidConfig( + "The UPDI physical interface cannot be used with an ambiguous target name" + " - please specify the exact name of the target in your configuration file. " + "See " + Paths::homeDomainName() + "/docs/supported-targets" + ); + } } this->avr8DebugInterface->configure(targetConfig); diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp index b589f3f0..49e35a61 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp @@ -122,13 +122,30 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface* avr8DebugInterface = nullptr; std::string name; std::optional family; + std::optional physicalInterface; std::optional targetDescriptionFile; + std::optional targetParameters; std::map padDescriptorsByName; std::map targetVariantsById; std::map targetRegisterDescriptorsByType; std::map targetMemoryDescriptorsByType; + /** + * Users are required to set their desired physical interface in their Bloom configuration. This would take + * the form of a string, so we map the available options to the appropriate enums. + */ + static inline auto getPhysicalInterfacesByName() { + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; + + return std::map({ + {"debugwire", PhysicalInterface::DEBUG_WIRE}, + {"debug-wire", PhysicalInterface::DEBUG_WIRE}, + {"pdi", PhysicalInterface::PDI}, + {"jtag", PhysicalInterface::JTAG}, + {"updi", PhysicalInterface::UPDI}, + }); + }; /** * Resolves the appropriate TDF for the AVR8 target and populates this->targetDescriptionFile. */ diff --git a/src/Targets/Microchip/AVR/Target.hpp b/src/Targets/Microchip/AVR/Target.hpp index 263c1192..08001371 100644 --- a/src/Targets/Microchip/AVR/Target.hpp +++ b/src/Targets/Microchip/AVR/Target.hpp @@ -1,9 +1,8 @@ #pragma once -#include #include -#include "../../Target.hpp" +#include "src/Targets/Target.hpp" #include "TargetSignature.hpp"