From 0931bc649f1070a17f6e78e63d60484036a343e3 Mon Sep 17 00:00:00 2001 From: Nav Date: Sat, 26 Jun 2021 04:30:01 +0100 Subject: [PATCH] Decoupled AVR8 physical interface enum from EDBG protocol code --- .../VendorSpecific/EDBG/AVR/Avr8Generic.hpp | 20 ++++---- .../EDBG/AVR/EdbgAvr8Interface.cpp | 10 ++-- .../EDBG/AVR/EdbgAvr8Interface.hpp | 48 +++++++++++-------- .../AVR8Generic/GetDeviceId.hpp | 15 +++--- src/Targets/Microchip/AVR/AVR8/Avr8.cpp | 1 + .../Microchip/AVR/AVR8/PhysicalInterface.hpp | 12 +++++ 6 files changed, 67 insertions(+), 39 deletions(-) create mode 100644 src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp index 88a77a9d..906a467c 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp @@ -1,5 +1,7 @@ #pragma once +#include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp" + namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr { struct Avr8EdbgParameter @@ -76,14 +78,16 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr DEBUGGING = 0x02, }; - enum class Avr8PhysicalInterface: unsigned char - { - NONE = 0x00, - JTAG = 0x04, - DEBUG_WIRE = 0x05, - PDI = 0x06, - PDI_1W = 0x08, - }; + static inline auto getAvr8PhysicalInterfaceToIdMapping() { + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; + + return std::map({ + {PhysicalInterface::DEBUG_WIRE, 0x05}, + {PhysicalInterface::PDI, 0x06}, + {PhysicalInterface::JTAG, 0x04}, + {PhysicalInterface::UPDI, 0x08}, + }); + } enum class Avr8MemoryType: unsigned char { 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 e812c11a..28065dab 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp @@ -69,7 +69,7 @@ std::vector EdbgAvr8Interface::getParameter(const Avr8EdbgParamet void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) { auto physicalInterface = targetConfig.jsonObject.find("physicalInterface")->toString().toLower().toStdString(); - auto availablePhysicalInterfaces = EdbgAvr8Interface::physicalInterfacesByName; + auto availablePhysicalInterfaces = this->getPhysicalInterfacesByName(); if (physicalInterface.empty() || availablePhysicalInterfaces.find(physicalInterface) == availablePhysicalInterfaces.end() @@ -79,13 +79,13 @@ void EdbgAvr8Interface::configure(const TargetConfig& targetConfig) { auto selectedPhysicalInterface = availablePhysicalInterfaces.find(physicalInterface)->second; - if (selectedPhysicalInterface == Avr8PhysicalInterface::DEBUG_WIRE) { + if (selectedPhysicalInterface == PhysicalInterface::DEBUG_WIRE) { Logger::warning("AVR8 debugWire interface selected - the DWEN fuse will need to be enabled"); } this->physicalInterface = selectedPhysicalInterface; - if (this->physicalInterface == Avr8PhysicalInterface::JTAG && !this->family.has_value()) { + if (this->physicalInterface == PhysicalInterface::JTAG && !this->family.has_value()) { throw InvalidConfig("Cannot use JTAG physical interface with ambiguous target name - please specify the" " exact name of the target in your configuration file. See https://bloom.oscillate.io/docs/supported-targets"); } @@ -327,7 +327,7 @@ void EdbgAvr8Interface::init() { this->setParameter( Avr8EdbgParameters::PHYSICAL_INTERFACE, - static_cast(this->physicalInterface) + getAvr8PhysicalInterfaceToIdMapping().at(this->physicalInterface) ); } @@ -438,7 +438,7 @@ void EdbgAvr8Interface::activate() { void EdbgAvr8Interface::deactivate() { if (this->targetAttached) { - if (this->physicalInterface == Avr8PhysicalInterface::DEBUG_WIRE && this->disableDebugWireOnDeactivate) { + if (this->physicalInterface == PhysicalInterface::DEBUG_WIRE && this->disableDebugWireOnDeactivate) { try { this->disableDebugWire(); Logger::warning("Successfully disabled debugWire on the AVR8 target - this is only temporary - " 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 9867e1eb..c60080ab 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp @@ -10,6 +10,7 @@ #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/EdbgInterface.hpp" #include "src/Targets/Microchip/AVR/Target.hpp" #include "src/Targets/Microchip/AVR/AVR8/Family.hpp" +#include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp" namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr { @@ -66,7 +67,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * Currently, the AVR8 Generic protocol supports 4 physical interfaces: debugWire, JTAG, PDI and UPDI. * The desired physical interface must be selected by setting the "AVR8_PHY_PHYSICAL" parameter. */ - Avr8PhysicalInterface physicalInterface = Avr8PhysicalInterface::NONE; + Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface = + Targets::Microchip::Avr::Avr8Bit::PhysicalInterface::DEBUG_WIRE; /** * EDBG-based debug tools require target specific parameters such as memory locations, page sizes and @@ -123,11 +125,15 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * 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 std::map physicalInterfacesByName = { - {"debugwire", Avr8PhysicalInterface::DEBUG_WIRE}, - {"pdi", Avr8PhysicalInterface::PDI}, - {"jtag", Avr8PhysicalInterface::JTAG}, -// {"updi", Avr8PhysicalInterface::PDI_1W}, // Disabled for now - will add support later + static inline auto getPhysicalInterfacesByName() { + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; + + return std::map({ + {"debugwire", PhysicalInterface::DEBUG_WIRE}, + {"pdi", PhysicalInterface::PDI}, + {"jtag", PhysicalInterface::JTAG}, + {"updi", PhysicalInterface::UPDI}, + }); }; /** @@ -136,44 +142,45 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr */ static inline auto getConfigVariantsByFamilyAndPhysicalInterface() { using Targets::Microchip::Avr::Avr8Bit::Family; - return std::map>({ + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; + return std::map>({ { Family::MEGA, { - {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG}, - {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, + {PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG}, + {PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, } }, { Family::TINY, { - {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG}, - {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, + {PhysicalInterface::JTAG, Avr8ConfigVariant::MEGAJTAG}, + {PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, } }, { Family::XMEGA, { - {Avr8PhysicalInterface::JTAG, Avr8ConfigVariant::XMEGA}, - {Avr8PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA}, + {PhysicalInterface::JTAG, Avr8ConfigVariant::XMEGA}, + {PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA}, } }, { Family::DA, { - {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI}, + {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI}, } }, { Family::DB, { - {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI}, + {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI}, } }, { Family::DD, { - {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI}, + {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI}, } }, }); @@ -211,10 +218,11 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * variant. Users are required to specify the exact target name in their config, when using the JTAG * physical interface. That way, this->family will be set by the time resolveConfigVariant() is called. */ - static std::map physicalInterfacesToConfigVariants = { - {Avr8PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, - {Avr8PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA}, - {Avr8PhysicalInterface::PDI_1W, Avr8ConfigVariant::UPDI}, + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; + static std::map physicalInterfacesToConfigVariants = { + {PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE}, + {PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA}, + {PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI}, }; if (physicalInterfacesToConfigVariants.contains(this->physicalInterface)) { diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/ResponseFrames/AVR8Generic/GetDeviceId.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/ResponseFrames/AVR8Generic/GetDeviceId.hpp index cea2eb98..97ffb738 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/ResponseFrames/AVR8Generic/GetDeviceId.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/ResponseFrames/AVR8Generic/GetDeviceId.hpp @@ -1,7 +1,7 @@ #pragma once #include "Avr8GenericResponseFrame.hpp" -#include "src/Targets/Microchip/AVR/Target.hpp" +#include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp" namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr::ResponseFrames::Avr8Generic { @@ -11,25 +11,28 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr::ResponseFrame GetDeviceId() = default; explicit GetDeviceId(const std::vector& AvrResponses): Avr8GenericResponseFrame(AvrResponses) {} - Targets::Microchip::Avr::TargetSignature extractSignature(Avr8PhysicalInterface physicalInterface) { + Targets::Microchip::Avr::TargetSignature extractSignature( + Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface + ) { + using Targets::Microchip::Avr::Avr8Bit::PhysicalInterface; auto payloadData = this->getPayloadData(); switch (physicalInterface) { - case Avr8PhysicalInterface::DEBUG_WIRE: { + case PhysicalInterface::DEBUG_WIRE: { /* * When using the DebugWire physical interface, the get device ID command will return * four bytes, where the first can be ignored. */ return Targets::Microchip::Avr::TargetSignature(payloadData[1], payloadData[2], payloadData[3]); } - case Avr8PhysicalInterface::PDI: - case Avr8PhysicalInterface::PDI_1W: { + case PhysicalInterface::PDI: + case PhysicalInterface::UPDI: { /* * When using the PDI physical interface, the signature is returned in LSB format. */ return Targets::Microchip::Avr::TargetSignature(payloadData[3], payloadData[2], payloadData[1]); } - case Avr8PhysicalInterface::JTAG: { + case PhysicalInterface::JTAG: { /* * When using the JTAG interface, the get device ID command returns a 32 bit JTAG ID. This takes * the following form: diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index 7c37f9ca..a1a94109 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -7,6 +7,7 @@ #include "Avr8.hpp" #include "PadDescriptor.hpp" +#include "PhysicalInterface.hpp" #include "src/Logger/Logger.hpp" #include "src/Exceptions/InvalidConfig.hpp" #include "src/Targets/TargetRegister.hpp" diff --git a/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp new file mode 100644 index 00000000..3b769991 --- /dev/null +++ b/src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp @@ -0,0 +1,12 @@ +#pragma once + +namespace Bloom::Targets::Microchip::Avr::Avr8Bit +{ + enum class PhysicalInterface: int + { + JTAG, + DEBUG_WIRE, + PDI, + UPDI, + }; +}