From fc7802492ab490ca32f8e50fc8c5a65ce3c92a40 Mon Sep 17 00:00:00 2001 From: timohueser Date: Mon, 14 Sep 2026 00:18:59 +0200 Subject: [PATCH] fix(storage): retry aborted SD read cleanup --- firmware/obc-fw-nrf54l/src/flat_store.rs | 6 +++- firmware/obc-fw-nrf54l/src/peak_view.rs | 18 ++++++++++-- firmware/obc-fw-nrf54l/src/semmc.rs | 35 ++++++++++++------------ 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/firmware/obc-fw-nrf54l/src/flat_store.rs b/firmware/obc-fw-nrf54l/src/flat_store.rs index 4c65d0212..829296bcb 100644 --- a/firmware/obc-fw-nrf54l/src/flat_store.rs +++ b/firmware/obc-fw-nrf54l/src/flat_store.rs @@ -152,9 +152,13 @@ impl BlockDevice for FlatCard { Ok(()) }) } - })?; + }) + .and_then(core::convert::identity); #[cfg(feature = "sd-bench")] crate::card_io::note_read_perf(bench_started, addr, blocks); + if let Err(error) = result { + defmt::warn!("SD: read at block {=u64}, {=usize} bytes failed: {}", lba, buf.len(), error); + } result } diff --git a/firmware/obc-fw-nrf54l/src/peak_view.rs b/firmware/obc-fw-nrf54l/src/peak_view.rs index a861846db..64c5ddc32 100644 --- a/firmware/obc-fw-nrf54l/src/peak_view.rs +++ b/firmware/obc-fw-nrf54l/src/peak_view.rs @@ -45,7 +45,10 @@ impl Platform for Hook<'_, '_> { let Some(source) = job.source else { return false }; let mut profile = PeakViewProfile::at(position.0, position.1, 0); profile.default_heading_q4 = app.peak_view_heading_q4(); - let Some(arm) = crate::arena::claim_peak(&mut profile, source, self.reader) else { return false }; + let Some(arm) = crate::arena::claim_peak(&mut profile, source, self.reader) else { + defmt::warn!("peak-view: could not start at {=i32},{=i32}", position.0, position.1); + return false; + }; app.state.peak_view_profile = Some(profile); job.arm = Some(arm); job.search = Default::default(); @@ -67,6 +70,11 @@ impl Platform for Hook<'_, '_> { arm.builder.step(&mut arm.terrain, 16); } if arm.terrain.failed() { + defmt::warn!( + "peak-view: terrain failed after {=u64} ms, progress {=u8}", + job.started.elapsed().as_millis(), + arm.builder.progress() + ); return Err(Failed); } if arm.builder.complete() { @@ -74,7 +82,13 @@ impl Platform for Hook<'_, '_> { defmt::info!("peak-view: generated in {=u64} ms", job.started.elapsed().as_millis()); } job.revision += 1; - job.search.refill(&mut arm.builder, self.reader).map_err(|_| Failed)?; + job.search.refill(&mut arm.builder, self.reader).map_err(|error| { + defmt::warn!("peak-view: summit refill failed: {}", defmt::Debug2Format(&error)); + Failed + })?; + if arm.builder.complete() { + defmt::info!("peak-view: ready in {=u64} ms", job.started.elapsed().as_millis()); + } } for (out, peak) in app.state.peak_view_peaks.iter_mut().zip(arm.builder.display_peaks()) { *out = peak; diff --git a/firmware/obc-fw-nrf54l/src/semmc.rs b/firmware/obc-fw-nrf54l/src/semmc.rs index b63c39c09..077020d42 100644 --- a/firmware/obc-fw-nrf54l/src/semmc.rs +++ b/firmware/obc-fw-nrf54l/src/semmc.rs @@ -1163,31 +1163,32 @@ impl Semmc { /// Blocking, and it holds the core while the transfer runs — the same profile the SPI transport /// had, and what `embedded_sdmmc`'s synchronous `BlockDevice` needs. See /// [`wait_completion`](Self::wait_completion) for why the wait is a bounded poll with an - /// interrupt fast path rather than a sleep. A controller abort gets two whole-command retries - /// after transfer recovery; all other errors return immediately. + /// interrupt fast path rather than a sleep. Read and cleanup commands each get two retries + /// after a controller abort. A new read starts only after cleanup succeeds. pub fn read_blocks(&mut self, lba: u32, buf: &mut [u8]) -> Result<(), SemmcError> { let n = self.check_request(buf.as_ptr() as usize, buf.len(), lba)?; self.clk_hz = self.read_clk_hz; let data = Some((buf.as_mut_ptr() as u32, BLOCK_BYTES as u32, n)); let mut retries_remaining = READ_ABORT_RETRIES; loop { - let transfer = if n == 1 { - match self.cmd(17, self.block_arg(lba), RESP_R1, PROC_IGNORE, data, READ_DEADLINE) { - Ok(_) => Ok(()), - Err(error) => match self.stop_transmission() { - Ok(()) => Err(error), - Err(cleanup_error) => return Err(cleanup_error), - }, + let command = if n == 1 { 17 } else { 18 }; + let transfer = self.cmd(command, self.block_arg(lba), RESP_R1, PROC_IGNORE, data, READ_DEADLINE); + if n != 1 || transfer.is_err() { + // The card can still be streaming after a read abort. An aborted stop must be + // retried before another read; resetting the host does not stop the card. + for attempt in 0..=READ_ABORT_RETRIES { + match self.stop_transmission() { + Err(SemmcError::Aborted(_)) if attempt < READ_ABORT_RETRIES => { + defmt::debug!("sEMMC: read cleanup retry {=usize}", attempt + 1); + } + result => { + result?; + break; + } + } } - } else { - // A failed CMD18 leaves the **card** streaming — the timeout path recovers the host - // (warm reboot), not the card — so STOP_TRANSMISSION goes out either way, or every - // later command talks to a card stuck in `data`. - let transfer = self.cmd(18, self.block_arg(lba), RESP_R1, PROC_IGNORE, data, READ_DEADLINE); - self.stop_transmission()?; - transfer.map(|_| ()) } - .and_then(|()| self.check_after_transfer()); + let transfer = transfer.map(|_| ()).and_then(|()| self.check_after_transfer()); match transfer { Err(SemmcError::Aborted(_)) if retries_remaining != 0 => {