sdmmc: refactor Error to struct with *kind* and *message*

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<sdmmc::Response, sdmmc::Error> {
         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(),

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<FixedVec<u8>>,
     },
-    /// 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<ErrorKind> 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<FixedVec<u8>, 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)),
         }
     }
 }