Merge #1322

1322: Remove FirmwareWriter r=Dirbaio a=rmja

FirmwareWriter currently has a "max-write-size" parameter, but this is a limitation that should be handled by chunking inside the NorFlash driver, and not "up here" in user code. In case that the driver (e.g. qspi driver) is unaware of any max-write limitations, one could simply add an intermediate NorFlash adapter providing the chunk'ing capability.

Co-authored-by: Rasmus Melchior Jacobsen <[email protected]>

3 files changed, 12 insertions, 76 deletions

diff --git a/embassy-boot/boot/src/firmware_updater.rs b/embassy-boot/boot/src/firmware_updater.rs
index af1ba114a..dd22b2fa7 100644
--- a/embassy-boot/boot/src/firmware_updater.rs
+++ b/embassy-boot/boot/src/firmware_updater.rs
@@ -1,7 +1,7 @@
 use embedded_storage::nor_flash::{NorFlash, NorFlashError, NorFlashErrorKind};
 use embedded_storage_async::nor_flash::NorFlash as AsyncNorFlash;
 
-use crate::{FirmwareWriter, Partition, State, BOOT_MAGIC, SWAP_MAGIC};
+use crate::{Partition, State, BOOT_MAGIC, SWAP_MAGIC};
 
 /// Errors returned by FirmwareUpdater
 #[derive(Debug)]
@@ -243,7 +243,6 @@ impl FirmwareUpdater {
         offset: usize,
         data: &[u8],
         dfu_flash: &mut F,
-        block_size: usize,
     ) -> Result<(), FirmwareUpdaterError> {
         assert!(data.len() >= F::ERASE_SIZE);
 
@@ -251,25 +250,23 @@ impl FirmwareUpdater {
             .erase(dfu_flash, offset as u32, (offset + data.len()) as u32)
             .await?;
 
-        FirmwareWriter(self.dfu)
-            .write_block(offset, data, dfu_flash, block_size)
-            .await?;
+        self.dfu.write(dfu_flash, offset as u32, data).await?;
 
         Ok(())
     }
 
     /// Prepare for an incoming DFU update by erasing the entire DFU area and
-    /// returning a `FirmwareWriter`.
+    /// returning its `Partition`.
     ///
     /// Using this instead of `write_firmware` allows for an optimized API in
     /// exchange for added complexity.
     pub async fn prepare_update<F: AsyncNorFlash>(
         &mut self,
         dfu_flash: &mut F,
-    ) -> Result<FirmwareWriter, FirmwareUpdaterError> {
+    ) -> Result<Partition, FirmwareUpdaterError> {
         self.dfu.wipe(dfu_flash).await?;
 
-        Ok(FirmwareWriter(self.dfu))
+        Ok(self.dfu)
     }
 
     //
@@ -443,29 +440,25 @@ impl FirmwareUpdater {
         offset: usize,
         data: &[u8],
         dfu_flash: &mut F,
-        block_size: usize,
     ) -> Result<(), FirmwareUpdaterError> {
         assert!(data.len() >= F::ERASE_SIZE);
 
         self.dfu
             .erase_blocking(dfu_flash, offset as u32, (offset + data.len()) as u32)?;
 
-        FirmwareWriter(self.dfu).write_block_blocking(offset, data, dfu_flash, block_size)?;
+        self.dfu.write_blocking(dfu_flash, offset as u32, data)?;
 
         Ok(())
     }
 
     /// Prepare for an incoming DFU update by erasing the entire DFU area and
-    /// returning a `FirmwareWriter`.
+    /// returning its `Partition`.
     ///
     /// Using this instead of `write_firmware_blocking` allows for an optimized
     /// API in exchange for added complexity.
-    pub fn prepare_update_blocking<F: NorFlash>(
-        &mut self,
-        flash: &mut F,
-    ) -> Result<FirmwareWriter, FirmwareUpdaterError> {
+    pub fn prepare_update_blocking<F: NorFlash>(&mut self, flash: &mut F) -> Result<Partition, FirmwareUpdaterError> {
         self.dfu.wipe_blocking(flash)?;
 
-        Ok(FirmwareWriter(self.dfu))
+        Ok(self.dfu)
     }
 }
diff --git a/embassy-boot/boot/src/firmware_writer.rs b/embassy-boot/boot/src/firmware_writer.rs
deleted file mode 100644
index 46079e731..000000000
--- a/embassy-boot/boot/src/firmware_writer.rs
+++ /dev/null
@@ -1,55 +0,0 @@
-use embedded_storage::nor_flash::NorFlash;
-use embedded_storage_async::nor_flash::NorFlash as AsyncNorFlash;
-
-use crate::Partition;
-
-/// FirmwareWriter allows writing blocks to an already erased flash.
-pub struct FirmwareWriter(pub(crate) Partition);
-
-impl FirmwareWriter {
-    /// Write data to a flash page.
-    ///
-    /// The buffer must follow alignment requirements of the target flash and a multiple of page size big.
-    ///
-    /// # Safety
-    ///
-    /// Failing to meet alignment and size requirements may result in a panic.
-    pub async fn write_block<F: AsyncNorFlash>(
-        &mut self,
-        offset: usize,
-        data: &[u8],
-        flash: &mut F,
-        block_size: usize,
-    ) -> Result<(), F::Error> {
-        let mut offset = offset as u32;
-        for chunk in data.chunks(block_size) {
-            self.0.write(flash, offset, chunk).await?;
-            offset += chunk.len() as u32;
-        }
-
-        Ok(())
-    }
-
-    /// Write data to a flash page.
-    ///
-    /// The buffer must follow alignment requirements of the target flash and a multiple of page size big.
-    ///
-    /// # Safety
-    ///
-    /// Failing to meet alignment and size requirements may result in a panic.
-    pub fn write_block_blocking<F: NorFlash>(
-        &mut self,
-        offset: usize,
-        data: &[u8],
-        flash: &mut F,
-        block_size: usize,
-    ) -> Result<(), F::Error> {
-        let mut offset = offset as u32;
-        for chunk in data.chunks(block_size) {
-            self.0.write_blocking(flash, offset, chunk)?;
-            offset += chunk.len() as u32;
-        }
-
-        Ok(())
-    }
-}
diff --git a/embassy-boot/boot/src/lib.rs b/embassy-boot/boot/src/lib.rs
index 4c28d7aa4..428e7ca2b 100644
--- a/embassy-boot/boot/src/lib.rs
+++ b/embassy-boot/boot/src/lib.rs
@@ -7,12 +7,10 @@ mod fmt;
 
 mod boot_loader;
 mod firmware_updater;
-mod firmware_writer;
 mod partition;
 
 pub use boot_loader::{BootError, BootFlash, BootLoader, Flash, FlashConfig, MultiFlashConfig, SingleFlashConfig};
 pub use firmware_updater::{FirmwareUpdater, FirmwareUpdaterError};
-pub use firmware_writer::FirmwareWriter;
 pub use partition::Partition;
 
 pub(crate) const BOOT_MAGIC: u8 = 0xD0;
@@ -109,7 +107,7 @@ mod tests {
         let mut updater = FirmwareUpdater::new(DFU, STATE);
         let mut offset = 0;
         for chunk in update.chunks(4096) {
-            block_on(updater.write_firmware(offset, chunk, &mut flash, 4096)).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut flash)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut flash, &mut aligned)).unwrap();
@@ -182,7 +180,7 @@ mod tests {
 
         let mut offset = 0;
         for chunk in update.chunks(2048) {
-            block_on(updater.write_firmware(offset, chunk, &mut dfu, chunk.len())).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut dfu)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut state, &mut aligned)).unwrap();
@@ -235,7 +233,7 @@ mod tests {
 
         let mut offset = 0;
         for chunk in update.chunks(4096) {
-            block_on(updater.write_firmware(offset, chunk, &mut dfu, chunk.len())).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut dfu)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut state, &mut aligned)).unwrap();