From 7eb2b267d0a1d910228e8678a6ae0ed213c6f73b Mon Sep 17 00:00:00 2001 From: Jonas Spanoghe Date: Fri, 8 Dec 2023 20:43:06 +0100 Subject: [PATCH] sdmmc: refactor Error to struct with *kind* and *message* --- .../allwinner-d1/d1-core/src/drivers/smhc.rs | 37 ++++----- source/kernel/src/services/sdmmc.rs | 75 +++++++++++++++---- 2 files changed, 78 insertions(+), 34 deletions(-) diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs index 0a356164..51619e51 100644 --- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs +++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs @@ -376,7 +376,7 @@ impl Smhc { #[tracing::instrument(level = tracing::Level::DEBUG, skip(self, params))] async fn command(&self, params: sdmmc::Command) -> Result { if self.smhc.smhc_cmd.read().cmd_load().bit_is_set() { - return Err(sdmmc::Error::Busy); + return Err(sdmmc::Error::from(sdmmc::ErrorKind::Busy)); } let cmd_idx = params.index; @@ -418,7 +418,7 @@ impl Smhc { }, _ => { tracing::error!("did not provide a buffer for read/write"); - return Err(sdmmc::Error::Other); + return Err(sdmmc::Error::from(sdmmc::ErrorKind::Buffer)); } }; @@ -436,7 +436,7 @@ impl Smhc { // Currently we limit the number of data that can be read at once if *cnt > DESCR_BUFF_SIZE * descriptors.len() || buf.capacity() < *cnt { - return Err(sdmmc::Error::Buffer); + return Err(sdmmc::Error::from(sdmmc::ErrorKind::Buffer)); } tracing::debug!(cnt, "Creating descriptor chain from buffer"); @@ -459,7 +459,7 @@ impl Smhc { .link((!last).then(|| (&descriptors[index + 1]).into())) .expect("Should be able to link to next descriptor") .buff_slice(slice) - .map_err(|_| sdmmc::Error::Buffer)? + .map_err(|_| sdmmc::Error::from(sdmmc::ErrorKind::Buffer))? .build(); index += 1; } @@ -496,20 +496,21 @@ impl Smhc { tracing::trace!("SMHC operation completed"); if let Some(error) = guard.data.err.take() { tracing::warn!(?error, "SMHC error"); - Err(match error { - ErrorKind::Response => sdmmc::Error::Response, - ErrorKind::ResponseCrc => sdmmc::Error::Response, - ErrorKind::DataCrc => sdmmc::Error::Data, - ErrorKind::ResponseTimeout => sdmmc::Error::Timeout, - ErrorKind::DataTimeout => sdmmc::Error::Timeout, - ErrorKind::DataStarvationTimeout => sdmmc::Error::Timeout, - ErrorKind::FifoUnderrunOverflow => sdmmc::Error::Data, - ErrorKind::CommandBusyIllegalWrite => sdmmc::Error::Busy, - ErrorKind::DataStart => sdmmc::Error::Data, - ErrorKind::DataEnd => sdmmc::Error::Data, - ErrorKind::Dma => sdmmc::Error::Other, - ErrorKind::Other => sdmmc::Error::Other, - }) + let (kind, msg) = match error { + ErrorKind::Response => (sdmmc::ErrorKind::Response, "Response"), + ErrorKind::ResponseCrc => (sdmmc::ErrorKind::Response, "CRC"), + ErrorKind::DataCrc => (sdmmc::ErrorKind::Data, "CRC"), + ErrorKind::ResponseTimeout => (sdmmc::ErrorKind::Timeout, "Response"), + ErrorKind::DataTimeout => (sdmmc::ErrorKind::Timeout, "Data"), + ErrorKind::DataStarvationTimeout => (sdmmc::ErrorKind::Timeout, "DataStarvation"), + ErrorKind::FifoUnderrunOverflow => (sdmmc::ErrorKind::Data, "FIFO"), + ErrorKind::CommandBusyIllegalWrite => (sdmmc::ErrorKind::Busy, "Command"), + ErrorKind::DataStart => (sdmmc::ErrorKind::Data, "DataStart"), + ErrorKind::DataEnd => (sdmmc::ErrorKind::Data, "DataEnd"), + ErrorKind::Dma => (sdmmc::ErrorKind::Other, "DMA"), + ErrorKind::Other => (sdmmc::ErrorKind::Other, "Unknown"), + }; + Err(sdmmc::Error::new(kind, msg)) } else if long_resp { let rsp: [u32; 4] = [ self.smhc.smhc_resp0.read().bits(), diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs index 10a03ab4..c1a4e5ca 100644 --- a/source/kernel/src/services/sdmmc.rs +++ b/source/kernel/src/services/sdmmc.rs @@ -108,24 +108,32 @@ pub enum ResponseType { Long, } -/// Response returned by the card, can be short or long, depending on command. +/// Response returned by the card, can be short or long, depending on command #[must_use] pub enum Response { /// The 32-bit value from the 48-bit response. - /// Potentially also includes the data buffer if this was a read command. + /// Potentially also includes the data buffer if this was a read command Short { - /// The response on the command line. + /// The response on the command line value: u32, - /// The received data, in case of a read command. + /// The received data, in case of a read command data: Option>, }, - /// The 128-bit value from the 136-bit response. + /// The 128-bit value from the 136-bit response Long(u128), } /// Errors returned by the [`SdmmcService`] #[derive(Debug, Eq, PartialEq)] -pub enum Error { +pub struct Error { + kind: ErrorKind, + message: Option<&'static str>, +} + +/// The different types of errors that can occur +#[derive(Debug, Eq, PartialEq)] +#[non_exhaustive] +pub enum ErrorKind { /// The service is currently busy and cannot handle the request Busy, /// Invalid or unexpected response was received @@ -154,6 +162,41 @@ impl Default for Command { } } +impl Error { + /// Create an error from a type and message + pub fn new(kind: ErrorKind, message: &'static str) -> Self { + Self { + kind, + message: Some(message), + } + } +} + +impl From for Error { + fn from(value: ErrorKind) -> Self { + Self { + kind: value, + message: None, + } + } +} + +impl core::fmt::Display for Error { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "SD/MMC error: ")?; + match self.kind { + ErrorKind::Busy => write!(f, "busy "), + ErrorKind::Response => write!(f, "response "), + ErrorKind::Data => write!(f, "data "), + ErrorKind::Buffer => write!(f, "buffer "), + ErrorKind::Timeout => write!(f, "timeout "), + ErrorKind::Other => write!(f, "other "), + }?; + + self.message.map_or_else(|msg| msg.fmt(f), Ok(())) + } +} + impl core::fmt::Debug for Response { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { @@ -248,7 +291,7 @@ impl SdCardClient { .await .map_err(|error| { tracing::warn!(?error, "failed to send request to SD/MMC service"); - Error::Other // TODO + Error::from(ErrorKind::Other) // TODO }) .and_then(|resp| resp.body); tracing::trace!("CMD{} response: {:?}", index, result); @@ -291,7 +334,7 @@ impl SdCardClient { card_type = CardType::SD2; } } - Response::Long(_) => return Err(Error::Response), + Response::Long(_) => return Err(Error::from(ErrorKind::Response)), } // TODO: limit the number of attempts @@ -331,7 +374,7 @@ impl SdCardClient { break value; } } - Response::Long(_) => return Err(Error::Response), + Response::Long(_) => return Err(Error::from(ErrorKind::Response)), } time::sleep(Duration::from_millis(1)).await; @@ -358,7 +401,7 @@ impl SdCardClient { }) .await? { - Response::Short { .. } => Err(Error::Response), + Response::Short { .. } => Err(Error::from(ErrorKind::Response)), Response::Long(value) => Ok(CardIdentification(value)), } } @@ -378,7 +421,7 @@ impl SdCardClient { .await? { Response::Short { value, .. } => Ok(RelativeCardAddress(value)), - Response::Long(_) => Err(Error::Response), + Response::Long(_) => Err(Error::from(ErrorKind::Response)), } } @@ -397,7 +440,7 @@ impl SdCardClient { .await? { Response::Short { value, .. } => Ok(CardStatus(value)), - Response::Long(_) => Err(Error::Response), + Response::Long(_) => Err(Error::from(ErrorKind::Response)), } } @@ -419,7 +462,7 @@ impl SdCardClient { match self .cmd(Command { index: 6, - argument: 0b10, + argument: 0b10, // instruct card to use 4-bits bus options: HardwareOptions::SetBusWidth(BusWidth::Quad), kind: CommandKind::Control, rsp_type: ResponseType::Short, @@ -429,7 +472,7 @@ impl SdCardClient { .await? { Response::Short { value, .. } => Ok(CardStatus(value)), - Response::Long(_) => Err(Error::Response), + Response::Long(_) => Err(Error::from(ErrorKind::Response)), } } @@ -442,7 +485,7 @@ impl SdCardClient { ) -> Result, Error> { // The provider buffer should have space for the requested amount of data if buf.capacity() < 512 * blocks { - return Err(Error::Buffer); + return Err(Error::from(ErrorKind::Buffer)); } match self @@ -460,7 +503,7 @@ impl SdCardClient { Response::Short { data: Some(res), .. } => Ok(res), - _ => Err(Error::Response), + _ => Err(Error::from(ErrorKind::Response)), } } }