From 7336603e952fcf048a25bd60b952bb95f41ba170 Mon Sep 17 00:00:00 2001 From: dcz Date: Fri, 13 Aug 2021 14:43:24 +0000 Subject: [PATCH] Enforce correct SD state at compilation using a new type The struct `SdMmcSpi` had two separate methods for initialization and deinitialization. It was up to the user not to mess them up at runtime. A new `BlockSpi` struct takes over `BlockDevice` interface duties, making it impossible to use block procedures while the SD interface is in the wrong state. --- src/lib.rs | 11 +- src/sdmmc.rs | 295 +++++++++++++++++++++++++++------------------------ 2 files changed, 161 insertions(+), 145 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 009b78a..56b36cf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,10 +35,11 @@ //! # let mut sdmmc_spi = DummySpi; //! # let mut sdmmc_cs = DummyCsPin; //! # let time_source = DummyTimeSource; -//! let mut cont = embedded_sdmmc::Controller::new(embedded_sdmmc::SdMmcSpi::new(sdmmc_spi, sdmmc_cs), time_source); +//! let mut spi_dev = embedded_sdmmc::SdMmcSpi::new(sdmmc_spi, sdmmc_cs); //! write!(uart, "Init SD card...").unwrap(); -//! match cont.device().init() { -//! Ok(_) => { +//! match spi_dev.acquire() { +//! Ok(block) => { +//! let mut cont = embedded_sdmmc::Controller::new(block, time_source); //! write!(uart, "OK!\nCard size...").unwrap(); //! match cont.device().card_size_bytes() { //! Ok(size) => writeln!(uart, "{}", size).unwrap(), @@ -51,7 +52,7 @@ //! } //! } //! Err(e) => writeln!(uart, "{:?}!", e).unwrap(), -//! } +//! }; //! ``` #![cfg_attr(not(test), no_std)] @@ -88,7 +89,7 @@ pub use crate::filesystem::{ Timestamp, MAX_FILE_SIZE, }; pub use crate::sdmmc::Error as SdMmcError; -pub use crate::sdmmc::SdMmcSpi; +pub use crate::sdmmc::{BlockSpi, SdMmcSpi}; // **************************************************************************** // diff --git a/src/sdmmc.rs b/src/sdmmc.rs index d1f8941..e9ebc21 100644 --- a/src/sdmmc.rs +++ b/src/sdmmc.rs @@ -11,7 +11,8 @@ use core::cell::RefCell; const DEFAULT_DELAY_COUNT: u32 = 32_000; -/// Represents an SD Card interface built from an SPI peripheral and a Chip +/// Represents an inactive SD Card interface. +/// Built from an SPI peripheral and a Chip /// Select pin. We need Chip Select to be separate so we can clock out some /// bytes without Chip Select asserted (which puts the card into SPI mode). pub struct SdMmcSpi @@ -26,6 +27,15 @@ where state: State, } +/// An initialized block device used to access the SD card. +/// **Caution**: any data must be flushed manually before dropping `BlockSpi`, see `deinit`. +/// Uses SPI mode. +pub struct BlockSpi<'a, SPI, CS>(&'a mut SdMmcSpi) +where + SPI: embedded_hal::blocking::spi::Transfer, + CS: embedded_hal::digital::v2::OutputPin, + >::Error: core::fmt::Debug; + /// The possible errors `SdMmcSpi` can generate. #[derive(Debug, Copy, Clone)] pub enum Error { @@ -132,12 +142,6 @@ where } } - /// Get a temporary borrow on the underlying SPI device. Useful if you - /// need to re-clock the SPI after performing `init()`. - pub fn spi(&mut self) -> core::cell::RefMut { - self.spi.borrow_mut() - } - fn cs_high(&self) -> Result<(), Error> { self.cs .borrow_mut() @@ -149,19 +153,13 @@ where self.cs.borrow_mut().set_low().map_err(|_| Error::GpioError) } - /// This routine must be performed with an SPI clock speed of around 100 - 400 kHz. - /// Afterwards you may increase the SPI clock speed. - pub fn init(&mut self) -> Result<(), Error> { - self.init_with_opts(Default::default()) - } - - /// De-init the card so it can't be used - pub fn deinit(&mut self) { - self.state = State::NoInit; + /// Initializes the card into a known state + pub fn acquire(&mut self) -> Result, Error> { + self.acquire_with_opts(Default::default()) } /// Initializes the card into a known state - pub fn init_with_opts(&mut self, options: AcquireOpts) -> Result<(), Error> { + pub fn acquire_with_opts(&mut self, options: AcquireOpts) -> Result, Error> { let f = |s: &mut Self| { // Assume it hasn't worked s.state = State::Error; @@ -244,47 +242,7 @@ where let result = f(self); self.cs_high()?; let _ = self.receive(); - result - } - - /// Return the usable size of this SD card in bytes. - pub fn card_size_bytes(&self) -> Result { - self.check_state()?; - self.with_chip_select(|s| { - let csd = s.read_csd()?; - match csd { - Csd::V1(ref contents) => Ok(contents.card_capacity_bytes()), - Csd::V2(ref contents) => Ok(contents.card_capacity_bytes()), - } - }) - } - - /// Erase some blocks on the card. - pub fn erase(&mut self, _first_block: BlockIdx, _last_block: BlockIdx) -> Result<(), Error> { - self.check_state()?; - unimplemented!(); - } - - /// Can this card erase single blocks? - pub fn erase_single_block_enabled(&self) -> Result { - self.check_state()?; - self.with_chip_select(|s| { - let csd = s.read_csd()?; - match csd { - Csd::V1(ref contents) => Ok(contents.erase_single_block_enabled()), - Csd::V2(ref contents) => Ok(contents.erase_single_block_enabled()), - } - }) - } - - /// Return an error if we're not in `State::Idle`. It probably means - /// they haven't called `begin()`. - fn check_state(&self) -> Result<(), Error> { - if self.state != State::Idle { - Err(Error::BadState) - } else { - Ok(()) - } + result.map(move |()| BlockSpi(self)) } /// Perform a function that might error with the chipselect low. @@ -311,77 +269,6 @@ where result } - /// Read the 'card specific data' block. - fn read_csd(&self) -> Result { - match self.card_type { - CardType::SD1 => { - let mut csd = CsdV1::new(); - if self.card_command(CMD9, 0)? != 0 { - return Err(Error::RegisterReadError); - } - self.read_data(&mut csd.data)?; - Ok(Csd::V1(csd)) - } - CardType::SD2 | CardType::SDHC => { - let mut csd = CsdV2::new(); - if self.card_command(CMD9, 0)? != 0 { - return Err(Error::RegisterReadError); - } - self.read_data(&mut csd.data)?; - Ok(Csd::V2(csd)) - } - } - } - - /// Read an arbitrary number of bytes from the card. Always fills the - /// given buffer, so make sure it's the right size. - fn read_data(&self, buffer: &mut [u8]) -> Result<(), Error> { - // Get first non-FF byte. - let mut delay = Delay::new(); - let status = loop { - let s = self.receive()?; - if s != 0xFF { - break s; - } - delay.delay(Error::TimeoutReadBuffer)?; - }; - if status != DATA_START_BLOCK { - return Err(Error::ReadError); - } - - for b in buffer.iter_mut() { - *b = self.receive()?; - } - - let mut crc = u16::from(self.receive()?); - crc <<= 8; - crc |= u16::from(self.receive()?); - - let calc_crc = crc16(buffer); - if crc != calc_crc { - return Err(Error::CrcError(crc, calc_crc)); - } - - Ok(()) - } - - /// Write an arbitrary number of bytes to the card. - fn write_data(&self, token: u8, buffer: &[u8]) -> Result<(), Error> { - let calc_crc = crc16(buffer); - self.send(token)?; - for &b in buffer.iter() { - self.send(b)?; - } - self.send((calc_crc >> 8) as u8)?; - self.send(calc_crc as u8)?; - let status = self.receive()?; - if (status & DATA_RES_MASK) != DATA_RES_ACCEPTED { - Err(Error::WriteError) - } else { - Ok(()) - } - } - /// Perform an application-specific command. fn card_acmd(&self, command: u8, arg: u32) -> Result { self.card_command(CMD55, 0)?; @@ -454,7 +341,126 @@ where } } -impl BlockDevice for SdMmcSpi +impl BlockSpi<'_, SPI, CS> +where + SPI: embedded_hal::blocking::spi::Transfer, + CS: embedded_hal::digital::v2::OutputPin, + >::Error: core::fmt::Debug, +{ + /// Get a temporary borrow on the underlying SPI device. Useful if you + /// need to re-clock the SPI. + pub fn spi(&mut self) -> core::cell::RefMut { + self.0.spi.borrow_mut() + } + + /// Mark the card as unused. + /// This should be kept infallible, because Drop is unable to fail. + /// See https://github.com/rust-lang/rfcs/issues/814 + // If there is any need to flush data, it should be implemented here. + fn deinit(&mut self) { + self.0.state = State::NoInit; + } + + /// Return the usable size of this SD card in bytes. + pub fn card_size_bytes(&self) -> Result { + self.0.with_chip_select(|_s| { + let csd = self.read_csd()?; + match csd { + Csd::V1(ref contents) => Ok(contents.card_capacity_bytes()), + Csd::V2(ref contents) => Ok(contents.card_capacity_bytes()), + } + }) + } + + /// Erase some blocks on the card. + pub fn erase(&mut self, _first_block: BlockIdx, _last_block: BlockIdx) -> Result<(), Error> { + unimplemented!(); + } + + /// Can this card erase single blocks? + pub fn erase_single_block_enabled(&self) -> Result { + self.0.with_chip_select(|_s| { + let csd = self.read_csd()?; + match csd { + Csd::V1(ref contents) => Ok(contents.erase_single_block_enabled()), + Csd::V2(ref contents) => Ok(contents.erase_single_block_enabled()), + } + }) + } + + /// Read the 'card specific data' block. + fn read_csd(&self) -> Result { + match self.0.card_type { + CardType::SD1 => { + let mut csd = CsdV1::new(); + if self.0.card_command(CMD9, 0)? != 0 { + return Err(Error::RegisterReadError); + } + self.read_data(&mut csd.data)?; + Ok(Csd::V1(csd)) + } + CardType::SD2 | CardType::SDHC => { + let mut csd = CsdV2::new(); + if self.0.card_command(CMD9, 0)? != 0 { + return Err(Error::RegisterReadError); + } + self.read_data(&mut csd.data)?; + Ok(Csd::V2(csd)) + } + } + } + + /// Read an arbitrary number of bytes from the card. Always fills the + /// given buffer, so make sure it's the right size. + fn read_data(&self, buffer: &mut [u8]) -> Result<(), Error> { + // Get first non-FF byte. + let mut delay = Delay::new(); + let status = loop { + let s = self.0.receive()?; + if s != 0xFF { + break s; + } + delay.delay(Error::TimeoutReadBuffer)?; + }; + if status != DATA_START_BLOCK { + return Err(Error::ReadError); + } + + for b in buffer.iter_mut() { + *b = self.0.receive()?; + } + + let mut crc = u16::from(self.0.receive()?); + crc <<= 8; + crc |= u16::from(self.0.receive()?); + + let calc_crc = crc16(buffer); + if crc != calc_crc { + return Err(Error::CrcError(crc, calc_crc)); + } + + Ok(()) + } + + /// Write an arbitrary number of bytes to the card. + fn write_data(&self, token: u8, buffer: &[u8]) -> Result<(), Error> { + let calc_crc = crc16(buffer); + self.0.send(token)?; + for &b in buffer.iter() { + self.0.send(b)?; + } + self.0.send((calc_crc >> 8) as u8)?; + self.0.send(calc_crc as u8)?; + let status = self.0.receive()?; + if (status & DATA_RES_MASK) != DATA_RES_ACCEPTED { + Err(Error::WriteError) + } else { + Ok(()) + } + } +} + +impl BlockDevice for BlockSpi<'_, SPI, CS> where SPI: embedded_hal::blocking::spi::Transfer, >::Error: core::fmt::Debug, @@ -469,21 +475,20 @@ where start_block_idx: BlockIdx, _reason: &str, ) -> Result<(), Self::Error> { - self.check_state()?; - let start_idx = match self.card_type { + let start_idx = match self.0.card_type { CardType::SD1 | CardType::SD2 => start_block_idx.0 * 512, CardType::SDHC => start_block_idx.0, }; - self.with_chip_select(|s| { + self.0.with_chip_select(|s| { if blocks.len() == 1 { // Start a single-block read s.card_command(CMD17, start_idx)?; - s.read_data(&mut blocks[0].contents)?; + self.read_data(&mut blocks[0].contents)?; } else { // Start a multi-block read s.card_command(CMD18, start_idx)?; for block in blocks.iter_mut() { - s.read_data(&mut block.contents)?; + self.read_data(&mut block.contents)?; } // Stop the read s.card_command(CMD12, 0)?; @@ -494,16 +499,15 @@ where /// Write one or more blocks, starting at the given block index. fn write(&self, blocks: &[Block], start_block_idx: BlockIdx) -> Result<(), Self::Error> { - self.check_state()?; - let start_idx = match self.card_type { + let start_idx = match self.0.card_type { CardType::SD1 | CardType::SD2 => start_block_idx.0 * 512, CardType::SDHC => start_block_idx.0, }; - self.with_chip_select_mut(|s| { + self.0.with_chip_select_mut(|s| { if blocks.len() == 1 { // Start a single-block write s.card_command(CMD24, start_idx)?; - s.write_data(DATA_START_BLOCK, &blocks[0].contents)?; + self.write_data(DATA_START_BLOCK, &blocks[0].contents)?; s.wait_not_busy()?; if s.card_command(CMD13, 0)? != 0x00 { return Err(Error::WriteError); @@ -516,7 +520,7 @@ where s.card_command(CMD25, start_idx)?; for block in blocks.iter() { s.wait_not_busy()?; - s.write_data(WRITE_MULTIPLE_TOKEN, &block.contents)?; + self.write_data(WRITE_MULTIPLE_TOKEN, &block.contents)?; } // Stop the write s.wait_not_busy()?; @@ -534,6 +538,17 @@ where } } +impl Drop for BlockSpi<'_, SPI, CS> +where + SPI: embedded_hal::blocking::spi::Transfer, + >::Error: core::fmt::Debug, + CS: embedded_hal::digital::v2::OutputPin, +{ + fn drop(&mut self) { + self.deinit() + } +} + // **************************************************************************** // // End Of File