Factorize MemoryInterface sligth variations

This also "backports" `fn write()` from ArmMemoryInterface (formerly ArmProbe) to MemoryInterface as relying on the default implementation showed some test failures. Co-Authored-By: Dániel Buga <bugadani@gmail.com>
probe-rs · Jul 25, 2024 · f1f7b98 · f1f7b98
1 parent 893fc55
commit f1f7b98
Show file tree

Hide file tree

Showing 13 changed files with 374 additions and 703 deletions.
diff --git a/changelog/changed-factorize-MemoryInterface-slight-variations.md b/changelog/changed-factorize-MemoryInterface-slight-variations.md
@@ -0,0 +1 @@
+Factorize the variants of `MemoryInterface` where only the return type changes into `MemoryInterface` with a generic type defaulting to `probe_rs::Error`.
diff --git a/probe-rs/src/architecture/arm/core/armv6m.rs b/probe-rs/src/architecture/arm/core/armv6m.rs
@@ -9,7 +9,7 @@ use crate::{
     error::Error,
     memory::valid_32bit_address,
     Architecture, BreakpointCause, CoreInformation, CoreInterface, CoreRegister, CoreStatus,
-    CoreType, HaltReason, InstructionSet, MemoryInterface, MemoryMappedRegister,
+    CoreType, HaltReason, InstructionSet, MemoryMappedRegister,
 };
 use bitfield::bitfield;
 use std::{
@@ -908,100 +908,11 @@ impl<'probe> CoreInterface for Armv6m<'probe> {
     }
 }
 
-impl<'probe> MemoryInterface for Armv6m<'probe> {
-    fn supports_native_64bit_access(&mut self) -> bool {
-        self.memory.supports_native_64bit_access()
+impl super::CoreMemoryInterface for Armv6m<'_> {
+    fn memory(&self) -> &dyn ArmMemoryInterface {
+        &*self.memory
     }
-    fn read_word_64(&mut self, address: u64) -> Result<u64, Error> {
-        let value = self.memory.read_word_64(address)?;
-        Ok(value)
-    }
-    fn read_word_32(&mut self, address: u64) -> Result<u32, Error> {
-        let value = self.memory.read_word_32(address)?;
-        Ok(value)
-    }
-    fn read_word_16(&mut self, address: u64) -> Result<u16, Error> {
-        let value = self.memory.read_word_16(address)?;
-        Ok(value)
-    }
-    fn read_word_8(&mut self, address: u64) -> Result<u8, Error> {
-        let value = self.memory.read_word_8(address)?;
-        Ok(value)
-    }
-
-    fn read_64(&mut self, address: u64, data: &mut [u64]) -> Result<(), Error> {
-        self.memory.read_64(address, data)?;
-        Ok(())
-    }
-
-    fn read_32(&mut self, address: u64, data: &mut [u32]) -> Result<(), Error> {
-        self.memory.read_32(address, data)?;
-        Ok(())
-    }
-
-    fn read_16(&mut self, address: u64, data: &mut [u16]) -> Result<(), Error> {
-        self.memory.read_16(address, data)?;
-        Ok(())
-    }
-
-    fn read_8(&mut self, address: u64, data: &mut [u8]) -> Result<(), Error> {
-        self.memory.read_8(address, data)?;
-        Ok(())
-    }
-
-    fn write_word_64(&mut self, address: u64, data: u64) -> Result<(), Error> {
-        self.memory.write_word_64(address, data)?;
-        Ok(())
-    }
-
-    fn write_word_32(&mut self, address: u64, data: u32) -> Result<(), Error> {
-        self.memory.write_word_32(address, data)?;
-        Ok(())
-    }
-
-    fn write_word_16(&mut self, address: u64, data: u16) -> Result<(), Error> {
-        self.memory.write_word_16(address, data)?;
-        Ok(())
-    }
-
-    fn write_word_8(&mut self, address: u64, data: u8) -> Result<(), Error> {
-        self.memory.write_word_8(address, data)?;
-        Ok(())
-    }
-
-    fn write_64(&mut self, address: u64, data: &[u64]) -> Result<(), Error> {
-        self.memory.write_64(address, data)?;
-        Ok(())
-    }
-
-    fn write_32(&mut self, address: u64, data: &[u32]) -> Result<(), Error> {
-        self.memory.write_32(address, data)?;
-        Ok(())
-    }
-
-    fn write_16(&mut self, address: u64, data: &[u16]) -> Result<(), Error> {
-        self.memory.write_16(address, data)?;
-        Ok(())
-    }
-
-    fn write_8(&mut self, address: u64, data: &[u8]) -> Result<(), Error> {
-        self.memory.write_8(address, data)?;
-        Ok(())
-    }
-
-    fn write(&mut self, address: u64, data: &[u8]) -> Result<(), Error> {
-        self.memory.write(address, data)?;
-        Ok(())
-    }
-
-    fn supports_8bit_transfers(&self) -> Result<bool, Error> {
-        let value = self.memory.supports_8bit_transfers()?;
-        Ok(value)
-    }
-
-    fn flush(&mut self) -> Result<(), Error> {
-        self.memory.flush()?;
-
-        Ok(())
+    fn memory_mut(&mut self) -> &mut dyn ArmMemoryInterface {
+        &mut *self.memory
     }
 }
diff --git a/probe-rs/src/architecture/arm/core/armv7a.rs b/probe-rs/src/architecture/arm/core/armv7a.rs
@@ -767,11 +767,11 @@ impl<'probe> CoreInterface for Armv7a<'probe> {
         }
     }
 
-    fn fpu_support(&mut self) -> Result<bool, crate::error::Error> {
+    fn fpu_support(&mut self) -> Result<bool, Error> {
         Ok(self.state.fp_reg_count != 0)
     }
 
-    fn floating_point_register_count(&mut self) -> Result<usize, crate::error::Error> {
+    fn floating_point_register_count(&mut self) -> Result<usize, Error> {
         Ok(self.state.fp_reg_count)
     }
 
@@ -818,7 +818,7 @@ impl<'probe> MemoryInterface for Armv7a<'probe> {
         false
     }
 
-    fn read_word_64(&mut self, address: u64) -> Result<u64, crate::error::Error> {
+    fn read_word_64(&mut self, address: u64) -> Result<u64, Error> {
         let mut ret: u64 = self.read_word_32(address)? as u64;
         ret |= (self.read_word_32(address + 4)? as u64) << 32;
 
@@ -865,7 +865,7 @@ impl<'probe> MemoryInterface for Armv7a<'probe> {
         Ok(data.to_le_bytes()[byte_offset as usize])
     }
 
-    fn read_64(&mut self, address: u64, data: &mut [u64]) -> Result<(), crate::error::Error> {
+    fn read_64(&mut self, address: u64, data: &mut [u64]) -> Result<(), Error> {
         for (i, word) in data.iter_mut().enumerate() {
             *word = self.read_word_64(address + ((i as u64) * 8))?;
         }
@@ -897,7 +897,7 @@ impl<'probe> MemoryInterface for Armv7a<'probe> {
         Ok(())
     }
 
-    fn write_word_64(&mut self, address: u64, data: u64) -> Result<(), crate::error::Error> {
+    fn write_word_64(&mut self, address: u64, data: u64) -> Result<(), Error> {
         let data_low = data as u32;
         let data_high = (data >> 32) as u32;
 
@@ -949,7 +949,7 @@ impl<'probe> MemoryInterface for Armv7a<'probe> {
         self.write_word_32(word_start, word)
     }
 
-    fn write_64(&mut self, address: u64, data: &[u64]) -> Result<(), crate::error::Error> {
+    fn write_64(&mut self, address: u64, data: &[u64]) -> Result<(), Error> {
         for (i, word) in data.iter().enumerate() {
             self.write_word_64(address + ((i as u64) * 8), *word)?;
         }
@@ -1042,9 +1042,7 @@ mod test {
         }
     }
 
-    impl ArmMemoryInterface for MockProbe {
-        fn update_core_status(&mut self, _: CoreStatus) {}
-
+    impl MemoryInterface<ArmError> for MockProbe {
         fn read_8(&mut self, _address: u64, _data: &mut [u8]) -> Result<(), ArmError> {
             todo!()
         }
@@ -1138,10 +1136,26 @@ mod test {
             todo!()
         }
 
+        fn read_64(&mut self, _address: u64, _data: &mut [u64]) -> Result<(), ArmError> {
+            todo!()
+        }
+
+        fn write_64(&mut self, _address: u64, _data: &[u64]) -> Result<(), ArmError> {
+            todo!()
+        }
+
         fn supports_8bit_transfers(&self) -> Result<bool, ArmError> {
             Ok(false)
         }
 
+        fn supports_native_64bit_access(&mut self) -> bool {
+            false
+        }
+    }
+
+    impl ArmMemoryInterface for MockProbe {
+        fn update_core_status(&mut self, _: CoreStatus) {}
+
         fn get_arm_communication_interface(
             &mut self,
         ) -> Result<
@@ -1155,21 +1169,9 @@ mod test {
             })
         }
 
-        fn read_64(&mut self, _address: u64, _data: &mut [u64]) -> Result<(), ArmError> {
-            todo!()
-        }
-
-        fn write_64(&mut self, _address: u64, _data: &[u64]) -> Result<(), ArmError> {
-            todo!()
-        }
-
         fn ap(&mut self) -> MemoryAp {
             todo!()
         }
-
-        fn supports_native_64bit_access(&mut self) -> bool {
-            false
-        }
     }
 
     impl SwdSequence for MockProbe {

diff --git a/probe-rs/src/architecture/arm/core/armv7m.rs b/probe-rs/src/architecture/arm/core/armv7m.rs
@@ -18,7 +18,7 @@ use crate::{
     },
     error::Error,
     memory::valid_32bit_address,
-    BreakpointCause, CoreRegister, CoreType, InstructionSet, MemoryInterface,
+    BreakpointCause, CoreRegister, CoreType, InstructionSet,
 };
 use bitfield::bitfield;
 use std::{
@@ -1134,91 +1134,12 @@ impl<'probe> CoreInterface for Armv7m<'probe> {
     }
 }
 
-impl<'probe> MemoryInterface for Armv7m<'probe> {
-    fn supports_native_64bit_access(&mut self) -> bool {
-        self.memory.supports_native_64bit_access()
+impl super::CoreMemoryInterface for Armv7m<'_> {
+    fn memory(&self) -> &dyn ArmMemoryInterface {
+        &*self.memory
     }
-
-    fn read_word_64(&mut self, address: u64) -> Result<u64, Error> {
-        self.memory.read_word_64(address).map_err(Error::from)
-    }
-
-    fn read_word_32(&mut self, address: u64) -> Result<u32, Error> {
-        self.memory.read_word_32(address).map_err(Error::from)
-    }
-
-    fn read_word_16(&mut self, address: u64) -> Result<u16, Error> {
-        self.memory.read_word_16(address).map_err(Error::from)
-    }
-
-    fn read_word_8(&mut self, address: u64) -> Result<u8, Error> {
-        self.memory.read_word_8(address).map_err(Error::from)
-    }
-
-    fn read_64(&mut self, address: u64, data: &mut [u64]) -> Result<(), Error> {
-        self.memory.read_64(address, data).map_err(Error::from)
-    }
-
-    fn read_32(&mut self, address: u64, data: &mut [u32]) -> Result<(), Error> {
-        self.memory.read_32(address, data).map_err(Error::from)
-    }
-
-    fn read_16(&mut self, address: u64, data: &mut [u16]) -> Result<(), Error> {
-        self.memory.read_16(address, data).map_err(Error::from)
-    }
-
-    fn read_8(&mut self, address: u64, data: &mut [u8]) -> Result<(), Error> {
-        self.memory.read_8(address, data).map_err(Error::from)
-    }
-
-    fn write_word_64(&mut self, address: u64, data: u64) -> Result<(), Error> {
-        self.memory
-            .write_word_64(address, data)
-            .map_err(Error::from)
-    }
-
-    fn write_word_32(&mut self, address: u64, data: u32) -> Result<(), Error> {
-        self.memory
-            .write_word_32(address, data)
-            .map_err(Error::from)
-    }
-
-    fn write_word_16(&mut self, address: u64, data: u16) -> Result<(), Error> {
-        self.memory
-            .write_word_16(address, data)
-            .map_err(Error::from)
-    }
-
-    fn write_word_8(&mut self, address: u64, data: u8) -> Result<(), Error> {
-        self.memory.write_word_8(address, data).map_err(Error::from)
-    }
-
-    fn write_64(&mut self, address: u64, data: &[u64]) -> Result<(), Error> {
-        self.memory.write_64(address, data).map_err(Error::from)
-    }
-
-    fn write_32(&mut self, address: u64, data: &[u32]) -> Result<(), Error> {
-        self.memory.write_32(address, data).map_err(Error::from)
-    }
-
-    fn write_16(&mut self, address: u64, data: &[u16]) -> Result<(), Error> {
-        self.memory.write_16(address, data).map_err(Error::from)
-    }
-
-    fn write_8(&mut self, address: u64, data: &[u8]) -> Result<(), Error> {
-        self.memory.write_8(address, data).map_err(Error::from)
-    }
-
-    fn write(&mut self, address: u64, data: &[u8]) -> Result<(), Error> {
-        self.memory.write(address, data).map_err(Error::from)
-    }
-
-    fn supports_8bit_transfers(&self) -> Result<bool, Error> {
-        self.memory.supports_8bit_transfers().map_err(Error::from)
-    }
-
-    fn flush(&mut self) -> Result<(), Error> {
-        self.memory.flush().map_err(Error::from)
+    fn memory_mut(&mut self) -> &mut dyn ArmMemoryInterface {
+        &mut *self.memory
     }
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Factorize the variants of `MemoryInterface` where only the return type changes into `MemoryInterface` with a generic type defaulting to `probe_rs::Error`.