From 1f90f21870b19b00de62e84af98b712f42d8919d Mon Sep 17 00:00:00 2001 From: Nav Date: Fri, 26 May 2023 22:45:57 +0100 Subject: [PATCH] Refactored the preserveEeprom implementation to make use of the EESAVE fuse --- .../EDBG/AVR/EdbgAvr8Interface.cpp | 33 +---- .../EDBG/AVR/EdbgAvr8Interface.hpp | 7 +- .../Microchip/AVR/AVR8/Avr8DebugInterface.hpp | 5 + src/Targets/Microchip/AVR/AVR8/Avr8.cpp | 125 +++++++++++++++++- src/Targets/Microchip/AVR/AVR8/Avr8.hpp | 14 +- .../Microchip/AVR/AVR8/ProgrammingSession.hpp | 23 ++++ 6 files changed, 172 insertions(+), 35 deletions(-) create mode 100644 src/Targets/Microchip/AVR/AVR8/ProgrammingSession.hpp 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 848cbb5d..dca21adf 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp @@ -787,28 +787,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr return; } - /* - * For JTAG and UPDI targets, the erase command can only erase the entire chip (including EEPROM). This - * violates the Avr8DebugInterface contract - as this member function should only ever erase program memory. - * - * All we can do here is take a copy of EEPROM and restore it after the erase operation. - * - * TODO: Look into setting the EESAVE fuse bit as an alternative to the backup-then-restore approach. - */ - auto eepromSnapshot = std::optional(); - - if (this->targetConfig.preserveEeprom) { - Logger::debug("Capturing EEPROM data, in preparation for chip erase"); - eepromSnapshot = this->readMemory( - TargetMemoryType::EEPROM, - this->targetParameters.eepromStartAddress.value(), - this->targetParameters.eepromSize.value() - ); - - } else { - Logger::warning("EEPROM will be erased - use the 'preserveEeprom' parameter to preserve EEPROM"); - } + throw Exception("JTAG and UPDI targets do not support program memory erase."); + } + void EdbgAvr8Interface::eraseChip() { const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( EraseMemory(Avr8EraseMemoryMode::CHIP) ); @@ -816,15 +798,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr if (responseFrame.id == Avr8ResponseId::FAILED) { throw Avr8CommandFailure("AVR8 erase memory command failed", responseFrame); } - - if (eepromSnapshot.has_value()) { - Logger::debug("Restoring EEPROM data"); - this->writeMemory( - TargetMemoryType::EEPROM, - this->targetParameters.eepromStartAddress.value(), - std::move(*eepromSnapshot) - ); - } } TargetState EdbgAvr8Interface::getTargetState() { 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 ed3f7e87..f54726db 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp @@ -233,7 +233,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr ) override; /** - * Issues the "Erase" command to erase a particular section of program memory, or the entire chip. + * Issues the "Erase" command to erase a particular section of program memory. * * @param section */ @@ -241,6 +241,11 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr std::optional section = std::nullopt ) override; + /** + * Issues the "Erase" command to erase the entire chip. + */ + void eraseChip() override; + /** * Returns the current state of the target. * diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp index 1632dfcf..3f2359a1 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp @@ -184,6 +184,11 @@ namespace Bloom::DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8 std::optional section = std::nullopt ) = 0; + /** + * Should erase the chip. + */ + virtual void eraseChip() = 0; + /** * Should obtain the current target state. * diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index 320f1a13..c12fb8db 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -348,6 +348,20 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit return; } + /* + * For JTAG and UPDI targets, we must perform a chip erase. This means we could end up erasing EEPROM, + * unless the EESAVE fuse bit has been programmed. + * + * If configured to do so, we will ensure that the EESAVE fuse bit has been programmed when we start a + * programming session. See Avr8::enableProgrammingMode() for more. + */ + if ( + this->targetConfig.physicalInterface == PhysicalInterface::JTAG + || this->targetConfig.physicalInterface == PhysicalInterface::UPDI + ) { + return this->avr8DebugInterface->eraseChip(); + } + return this->avr8DebugInterface->eraseProgramMemory(); } @@ -552,17 +566,49 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } void Avr8::enableProgrammingMode() { + if (this->activeProgrammingSession.has_value()) { + return; + } + this->avr8DebugInterface->enableProgrammingMode(); - this->progModeEnabled = true; + this->activeProgrammingSession = ProgrammingSession(); + + if ( + this->targetConfig.preserveEeprom + && ( + this->targetConfig.physicalInterface == PhysicalInterface::JTAG + || this->targetConfig.physicalInterface == PhysicalInterface::UPDI + ) + ) { + Logger::debug("Inspecting EESAVE fuse bit"); + this->activeProgrammingSession->managingEesaveFuseBit = this->updateEesaveFuseBit(true); + + if (this->activeProgrammingSession->managingEesaveFuseBit) { + /* + * We must disable and re-enable programming mode, in order for the changes to the EESAVE fuse bit to + * take effect. + */ + this->avr8DebugInterface->disableProgrammingMode(); + this->avr8DebugInterface->enableProgrammingMode(); + } + } } void Avr8::disableProgrammingMode() { + if (!this->activeProgrammingSession.has_value()) { + return; + } + + if (this->activeProgrammingSession->managingEesaveFuseBit) { + this->updateEesaveFuseBit(false); + } + this->avr8DebugInterface->disableProgrammingMode(); - this->progModeEnabled = false; + this->activeProgrammingSession.reset(); } bool Avr8::programmingModeEnabled() { - return this->progModeEnabled; + return this->activeProgrammingSession.has_value(); } void Avr8::loadTargetRegisterDescriptors() { @@ -949,6 +995,79 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit } } + bool Avr8::updateEesaveFuseBit(bool enable) { + using Services::StringService; + + const auto eesaveFuseBitsDescriptor = this->targetDescriptionFile.getEesaveFuseBitsDescriptor(); + + if (!eesaveFuseBitsDescriptor.has_value()) { + throw Exception("Could not find EESAVE bit field in TDF."); + } + + const auto managingProgrammingMode = !this->programmingModeEnabled(); + + try { + if (managingProgrammingMode) { + this->enableProgrammingMode(); + } + + const auto eesaveFuseByteValue = this->avr8DebugInterface->readMemory( + TargetMemoryType::FUSES, + eesaveFuseBitsDescriptor->byteAddress, + 1 + ).at(0); + + Logger::debug("EESAVE fuse byte value (before update): 0x" + StringService::toHex(eesaveFuseByteValue)); + + if (this->isFuseEnabled(*eesaveFuseBitsDescriptor, eesaveFuseByteValue) == enable) { + Logger::debug("EESAVE fuse bit already set to desired value - aborting update operation"); + + if (managingProgrammingMode) { + this->disableProgrammingMode(); + } + + return false; + } + + const auto newValue = this->setFuseEnabled(*eesaveFuseBitsDescriptor, eesaveFuseByteValue, enable); + + Logger::debug("New EESAVE fuse byte value (to be written): 0x" + StringService::toHex(newValue)); + + Logger::warning("Updating EESAVE fuse bit"); + this->avr8DebugInterface->writeMemory( + TargetMemoryType::FUSES, + eesaveFuseBitsDescriptor->byteAddress, + {newValue} + ); + + Logger::debug("Verifying EESAVE fuse bit"); + const auto postUpdateEesaveByteValue = this->avr8DebugInterface->readMemory( + TargetMemoryType::FUSES, + eesaveFuseBitsDescriptor->byteAddress, + 1 + ).at(0); + + if (postUpdateEesaveByteValue != newValue) { + throw Exception("Failed to update EESAVE fuse bit - post-update verification failed"); + } + + Logger::info("EESAVE fuse bit updated"); + + if (managingProgrammingMode) { + this->disableProgrammingMode(); + } + + return true; + + } catch (const Exception& exception) { + if (managingProgrammingMode) { + this->disableProgrammingMode(); + } + + throw exception; + } + } + ProgramMemorySection Avr8::getProgramMemorySectionFromAddress(std::uint32_t address) { return this->targetParameters.bootSectionStartAddress.has_value() && address >= this->targetParameters.bootSectionStartAddress.value() diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp index 4059246d..87b50f6f 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp @@ -15,6 +15,7 @@ #include "TargetParameters.hpp" #include "PadDescriptor.hpp" #include "ProgramMemorySection.hpp" +#include "ProgrammingSession.hpp" #include "src/Targets/Microchip/AVR/Fuse.hpp" #include "src/Targets/TargetRegister.hpp" @@ -132,7 +133,7 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit std::map targetMemoryDescriptorsByType; - bool progModeEnabled = false; + std::optional activeProgrammingSession = std::nullopt; /** * Populates this->targetRegisterDescriptorsById with registers extracted from the TDF, as well as general @@ -185,6 +186,17 @@ namespace Bloom::Targets::Microchip::Avr::Avr8Bit */ void updateOcdenFuseBit(bool enable); + /** + * Updates the "Preserve EEPROM" (EESAVE) fuse bit on the AVR target. + * + * @param enable + * True to enable the fuse, false to disable it. + * + * @return + * True if the fuse bit was updated. False if the fuse bit was already set to the desired value. + */ + bool updateEesaveFuseBit(bool enable); + /** * Resolves the program memory section from a program memory address. * diff --git a/src/Targets/Microchip/AVR/AVR8/ProgrammingSession.hpp b/src/Targets/Microchip/AVR/AVR8/ProgrammingSession.hpp new file mode 100644 index 00000000..5c316a73 --- /dev/null +++ b/src/Targets/Microchip/AVR/AVR8/ProgrammingSession.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include + +namespace Bloom::Targets::Microchip::Avr::Avr8Bit +{ + /** + * Information relating to a specific AVR8 programming session. + * + * Whenever an AVR8 target is put into programming mode, a new programming session is started. + */ + struct ProgrammingSession + { + /** + * This flag indicates whether we need to manage the EESAVE fuse for this programming session. It will only be + * set to true if the user has chosen to preserve EEPROM and the EESAVE fuse wasn't already programmed when the + * programming session started. + */ + bool managingEesaveFuseBit = false; + + ProgrammingSession() = default; + }; +}