Corrected debugWire and JTAG HIGH byte parameter values

Additional checks in TDF validation to ensure that the IO memory segment offset has been applied to the relevant registers

build/scripts/Targets/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php

@@ -132,12 +132,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
 
             if ($eepromAddressRegister instanceof TargetRegister) {
                 $output->eearAddressLow = $eepromAddressRegister->address;
-                $output->eearAddressHigh = $eepromAddressRegister->size > 1
+                $output->eearAddressHigh = $eepromAddressRegister->address + ($eepromAddressRegister->size - 1);
-                    ? $eepromAddressRegister->address !== null
-                        ? $eepromAddressRegister->address >> 8
-                        : null
-                    : $eepromAddressRegister->address
-                ;
 
             } else {
                 $eearAddressLow = $eepromPeripheral->getRegister('eeprom', 'eearl');
@@ -145,11 +140,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
 
                 if ($eearAddressLow instanceof TargetRegister) {
                     $output->eearAddressLow = $eearAddressLow->address;
-                    $output->eearAddressHigh = $eearAddressLow->size > 1
+                    $output->eearAddressHigh = $eearAddressLow->address + ($eearAddressLow->size - 1);
-                        ? $eearAddressLow->address !== null
-                            ? $eearAddressLow->address >> 8
-                            : null
-                        : $eearAddressLow->address;
                 }
 
                 if ($eearAddressHigh instanceof TargetRegister) {
@@ -283,12 +274,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
 
             if ($eepromAddressRegister instanceof TargetRegister) {
                 $output->eearAddressLow = $eepromAddressRegister->address;
-                $output->eearAddressHigh = $eepromAddressRegister->size > 1
+                $output->eearAddressHigh = $eepromAddressRegister->address + ($eepromAddressRegister->size - 1);
-                    ? $eepromAddressRegister->address !== null
-                        ? $eepromAddressRegister->address >> 2
-                        : null
-                    : $eepromAddressRegister->address
-                ;
 
             } else {
                 $eearAddressLow = $eepromPeripheral->getRegister('eeprom', 'eearl');
@@ -509,22 +495,46 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile
         return null;
     }
 
-    private function getProgramMemorySegment(): ?MemorySegment
+    public function getProgramMemorySegment(): ?MemorySegment
     {
         return $this->getMemorySegment('prog', 'internal_program_memory');
     }
 
-    private function getRamSegment(): ?MemorySegment
+    public function getGpRegistersMemorySegment(): ?MemorySegment
+    {
+        return $this->getMemorySegment('data', 'gp_registers')
+            ?? $this->getMemorySegment('register_file', 'gp_registers');
+    }
+
+    public function getRamSegment(): ?MemorySegment
     {
         return $this->getMemorySegment('data', 'internal_ram');
     }
 
-    private function getEepromSegment(): ?MemorySegment
+    public function getEepromSegment(): ?MemorySegment
     {
         return $this->getMemorySegment('eeprom', 'internal_eeprom')
             ?? $this->getMemorySegment('data', 'internal_eeprom');
     }
 
+    public function getIoMemorySegment(): ?MemorySegment
+    {
+        return $this->getMemorySegment('data', 'io')
+            ?? $this->getMemorySegment('data', 'mapped_io');
+    }
+
+    public function getSignaturesMemorySegment(): ?MemorySegment
+    {
+        return $this->getMemorySegment('data', 'signatures')
+            ?? $this->getMemorySegment('signatures', 'signatures');
+    }
+
+    public function getFusesMemorySegment(): ?MemorySegment
+    {
+        return $this->getMemorySegment('data', 'fuses')
+            ?? $this->getMemorySegment('fuses', 'fuses');
+    }
+
     private function getBootSectionOptions(): array
     {
         $output = [];

build/scripts/Targets/TargetDescriptionFiles/AVR8/Services/ValidationService.php

@@ -9,6 +9,7 @@ use Targets\TargetDescriptionFiles\Avr8\IspParameters;
 use Targets\TargetDescriptionFiles\Avr8\JtagParameters;
 use Targets\TargetDescriptionFiles\Avr8\PdiParameters;
 use Targets\TargetDescriptionFiles\Avr8\UpdiParameters;
+use Targets\TargetDescriptionFiles\MemorySegment;
 use Targets\TargetRegister;
 use Targets\TargetRegisterGroup;
 
@@ -137,7 +138,11 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
         }
 
         if (in_array(AvrPhysicalInterface::DEBUG_WIRE, $debugPhysicalInterfaces)) {
-            $failures = array_merge($failures, $this->validateDebugWireParameters($tdf->getDebugWireParameters()));
+            $failures = array_merge(
+                $failures,
+                $this->validateDebugWireParameters($tdf->getDebugWireParameters(), $tdf)
+            );
+
             $failures = array_merge($failures, $this->validateIspParameters($tdf->getIspParameters()));
 
             if (!in_array(AvrPhysicalInterface::ISP, $physicalInterfaces)) {
@@ -165,7 +170,7 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
             in_array(AvrPhysicalInterface::JTAG, $debugPhysicalInterfaces)
             && $family == AvrFamily::MEGA
         ) {
-            $failures = array_merge($failures, $this->validateJtagParameters($tdf->getJtagParameters()));
+            $failures = array_merge($failures, $this->validateJtagParameters($tdf->getJtagParameters(), $tdf));
 
             if (empty($tdf->getFuseBitsDescriptor('ocden'))) {
                 $failures[] = 'Could not find OCDEN fuse bit field for JTAG target';
@@ -233,8 +238,10 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
         return $failures;
     }
 
-    private function validateDebugWireParameters(DebugWireParameters $parameters): array
+    private function validateDebugWireParameters(
-    {
+        DebugWireParameters $parameters,
+        Avr8TargetDescriptionFile $tdf
+    ): array {
         $failures = [];
 
         if ($parameters->flashPageSize === null) {
@@ -336,6 +343,35 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
             $failures[] = 'OSCCALR address size exceeds 0xFF - corresponding EDBG device parameter size is 8 bits';
         }
 
+        /*
+         * Bloom removes the IO memory segment offset when sending some of these params to the debug tool. This means
+         * we assume that the offset has already been applied to the params. We enforce this here.
+         */
+        if (($ioMemorySegment = $tdf->getIoMemorySegment()) instanceof MemorySegment) {
+            if ($parameters->osccalAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'OSCCAL address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eearAddressLow < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEARL address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eearAddressHigh < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEARH address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eecrAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'EECR address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eedrAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEDR address does not have IO memory segment offset applied';
+            }
+
+        } else {
+            $failures[] = 'Could not verify address offset of debugWire parameters - IO memory segment missing';
+        }
+
         return $failures;
     }
 
@@ -394,7 +430,7 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
         return $failures;
     }
 
-    private function validateJtagParameters(JtagParameters $parameters): array
+    private function validateJtagParameters(JtagParameters $parameters, Avr8TargetDescriptionFile $tdf): array
     {
         $failures = [];
 
@@ -497,6 +533,35 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat
             $failures[] = 'OSCCALR address size exceeds 0xFF - corresponding EDBG device parameter size is 8 bits';
         }
 
+        /*
+         * Bloom removes the IO memory segment offset when sending some of these params to the debug tool. This means
+         * we assume that the offset has already been applied to the params. We enforce this here.
+         */
+        if (($ioMemorySegment = $tdf->getIoMemorySegment()) instanceof MemorySegment) {
+            if ($parameters->osccalAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'OSCCAL address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eearAddressLow < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEARL address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eearAddressHigh < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEARH address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eecrAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'EECR address does not have IO memory segment offset applied';
+            }
+
+            if ($parameters->eedrAddress < $ioMemorySegment->startAddress) {
+                $failures[] = 'EEDR address does not have IO memory segment offset applied';
+            }
+
+        } else {
+            $failures[] = 'Could not verify address offset of debugWire parameters - IO memory segment missing';
+        }
+
         return $failures;
     }
 

src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/Parameters/AVR8Generic/DebugWireJtagParameters.cpp

@@ -48,9 +48,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen
              * If the target doesn't have a high byte in the `EEAR` address, the `eearAddressHigh` parameter should be
              * equal to the `eearAddressLow` parameter, as stated in the "EDBG-based Tools Protocols" document.
              */
-            this->eearAddressHigh = static_cast<std::uint8_t>(
+            this->eearAddressHigh = static_cast<std::uint8_t>(startAddress + (eearDescriptor->get().size - 1));
-                eearDescriptor->get().size > 1 ? startAddress >> 8 : startAddress
-            );
 
         } else {
             const auto& eearlDescriptor = eepromRegisterGroupDescriptor.getRegisterDescriptor("eearl");
@@ -67,7 +65,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen
             this->eearAddressHigh = static_cast<std::uint8_t>(
                 eearhDescriptor.has_value()
                     ? eearhDescriptor->get().startAddress
-                    : eearlDescriptor.size > 1 ? eearlDescriptor.startAddress >> 8 : eearlDescriptor.startAddress
+                    : eearlDescriptor.startAddress + (eearlDescriptor.size - 1)
             );
         }
 
@@ -130,12 +128,21 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen
          *
          * It does *not* seem to apply to the SPMCR or OCDDR address.
          */
-        const auto& ioMemorySegment = targetDescriptionFile.getIoMemorySegment();
+        const auto& ioSegmentStartAddress = static_cast<std::uint8_t>(
+            targetDescriptionFile.getIoMemorySegment().startAddress
+        );
 
-        this->osccalAddress -= ioMemorySegment.startAddress;
+        // This is enforced in TDF validation
-        this->eearAddressLow -= ioMemorySegment.startAddress;
+        assert(this->osccalAddress >= ioSegmentStartAddress);
-        this->eearAddressHigh -= ioMemorySegment.startAddress;
+        assert(this->eearAddressLow >= ioSegmentStartAddress);
-        this->eecrAddress -= ioMemorySegment.startAddress;
+        assert(this->eearAddressHigh >= ioSegmentStartAddress);
-        this->eedrAddress -= ioMemorySegment.startAddress;
+        assert(this->eecrAddress >= ioSegmentStartAddress);
+        assert(this->eedrAddress >= ioSegmentStartAddress);
+
+        this->osccalAddress -= ioSegmentStartAddress;
+        this->eearAddressLow -= ioSegmentStartAddress;
+        this->eearAddressHigh -= ioSegmentStartAddress;
+        this->eecrAddress -= ioSegmentStartAddress;
+        this->eedrAddress -= ioSegmentStartAddress;
     }
 }