From ece1b7ac44f18cb1b6cd81900b1507f737f9ca40 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 30 Jul 2023 17:51:16 +0200
Subject: [PATCH 01/38] Create SMHC driver skeleton

---
 .../allwinner-d1/d1-core/src/drivers/mod.rs   |   1 +
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 208 ++++++++++++++++++
 2 files changed, 209 insertions(+)
 create mode 100644 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/mod.rs b/platforms/allwinner-d1/d1-core/src/drivers/mod.rs
index 2c8259ff..d8f16282 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/mod.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/mod.rs
@@ -1,5 +1,6 @@
 #[cfg(feature = "sharp-display")]
 pub mod sharp_display;
+pub mod smhc;
 pub mod spim;
 pub mod twi;
 pub mod uart;
diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
new file mode 100644
index 00000000..ed695016
--- /dev/null
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -0,0 +1,208 @@
+//! Driver for the Allwinner D1's SMHC peripheral.
+//!
+//! The D1 contains three separate SD/MMC Host Controllers: [`SMHC0`], [`SMHC1`] and [`SMHC2`].
+//! - [`SMHC0`] controls *Secure Digital* memory devices (SD cards)
+//! - [`SMHC1`] controls *Secure Digital I/O* devices (SDIO)
+//! - [`SMHC2`] controls *MultiMedia Card* devices (MMC)
+//!
+//! Each SMHC also has an internal DMA controller that can be used for offloading
+//! the transfer and reception of large amounts of data to/from the device.
+use core::{
+    cell::UnsafeCell,
+    ops::{Deref, DerefMut},
+    task::{Poll, Waker},
+};
+
+use crate::ccu::{BusGatingResetRegister, Ccu};
+use d1_pac::{smhc, Interrupt, GPIO, SMHC0, SMHC1, SMHC2};
+use kernel::{
+    comms::kchannel::{KChannel, KConsumer},
+    mnemos_alloc::containers::FixedVec,
+    registry, Kernel,
+};
+
+/// TODO
+pub struct Smhc {
+    isr: &'static IsrData,
+    smhc: &'static smhc::RegisterBlock,
+    int: (Interrupt, fn()),
+}
+
+/// TODO
+struct IsrData {
+    data: UnsafeCell<SmhcData>,
+}
+unsafe impl Sync for IsrData {}
+
+struct SmhcDataGuard<'a> {
+    smhc: &'a smhc::RegisterBlock,
+    data: &'a mut SmhcData,
+}
+
+struct SmhcData {
+    state: State,
+    op: SmhcOp,
+    err: Option<ErrorKind>,
+    waker: Option<Waker>,
+}
+
+static SMHC0_ISR: IsrData = IsrData {
+    data: UnsafeCell::new(SmhcData {
+        state: State::Idle,
+        op: SmhcOp::None,
+        err: None,
+        waker: None,
+    }),
+};
+
+enum SmhcOp {
+    // TODO
+    None,
+}
+
+/// TODO
+#[derive(Debug, Copy, Clone)]
+#[non_exhaustive]
+pub enum ErrorKind {
+    /// A transmit bit error, end bit error, or CMD index error has occurred.
+    Response,
+    /// Invalid CRC in response.
+    ResponseCrc,
+    /// When receiving data, this means that the received data has data CRC error.
+    /// When transmitting data, this means that the received CRC status taken is negative.
+    DataCrc,
+    /// Did not receive a response in time.
+    ResponseTimeout,
+    /// Did not receive data in time.
+    DataTimeout,
+    /// Data starvation detected.
+    DataStarvationTimeout,
+    /// FIFO underrun or overflow.
+    FifoUnderrunOverflow,
+    /// Command busy and illegal write. TODO: understand this + add better explanation
+    CommandBusyIllegalWrite,
+    /// When receiving data, this means that the host controller found an error start bit.
+    /// When transmitting data, this means that the busy signal is cleared after the last block.
+    DataStart,
+    /// When receiving data, this means that we did not receive a valid data end bit.
+    /// When transmitting data, this means that we did not receive the CRC status token.
+    DataEnd,
+    /// An error occurred in the internal DMA controller.
+    Dma,
+    /// A different error occurred. The original error may contain more information.
+    Other,
+}
+
+/// TODO
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[allow(dead_code)]
+enum State {
+    Idle,
+    // TODO
+}
+
+impl Smhc {
+    /// Initialize SMHC0 for SD cards.
+    ///
+    /// # Safety
+    /// TODO
+    pub unsafe fn new(mut smhc: SMHC0, ccu: &mut Ccu, gpio: &mut GPIO) -> Self {
+        // Configure default pin mapping for TF (micro SD) card socket.
+        // This is valid for the Lichee RV SOM and Mango Pi MQ Pro.
+        gpio.pf_cfg0.modify(|_, w| {
+            w.pf0_select().sdc0_d1();
+            w.pf1_select().sdc0_d0();
+            w.pf2_select().sdc0_clk();
+            w.pf3_select().sdc0_cmd();
+            w.pf4_select().sdc0_d3();
+            w.pf5_select().sdc0_d2();
+            w
+        });
+
+        // Make sure the card clock is turned off before changing the module clock
+        smhc.smhc_clkdiv.write(|w| w.cclk_enb().off());
+
+        ccu.disable_module(&mut smhc);
+        // Set module clock rate to 200 MHz
+        // TODO: ccu should provide a higher-level abstraction for this
+        ccu.borrow_raw().smhc0_clk.write(|w| {
+            w.clk_src_sel().pll_peri_1x();
+            w.factor_n().variant(d1_pac::ccu::smhc0_clk::FACTOR_N_A::N1);
+            w.factor_m().variant(2);
+            w.clk_gating().set_bit();
+            w
+        });
+        ccu.enable_module(&mut smhc);
+
+        Self::init(
+            unsafe { &*SMHC0::ptr() },
+            Interrupt::SMHC0,
+            Self::handle_smhc0_interrupt,
+        )
+    }
+
+    /// This assumes the GPIO pin mappings are already configured.
+    unsafe fn init(smhc: &'static smhc::RegisterBlock, int: Interrupt, isr: fn()) -> Self {
+        // Closure to change the card clock
+        let prg_clk = || {
+            smhc.smhc_cmd.write(|w| {
+                w.wait_pre_over().wait();
+                w.prg_clk().change();
+                w.cmd_load().set_bit();
+                w
+            });
+
+            while smhc.smhc_cmd.read().cmd_load().bit_is_set() {
+                core::hint::spin_loop()
+            }
+
+            smhc.smhc_rintsts.write(|w| unsafe { w.bits(0xFFFFFFFF) });
+        };
+
+        // Reset the SD/MMC controller
+        smhc.smhc_ctrl.modify(|_, w| w.soft_rst().reset());
+        while smhc.smhc_ctrl.read().soft_rst().is_reset() {
+            core::hint::spin_loop();
+        }
+
+        // Reset the FIFO
+        smhc.smhc_ctrl.modify(|_, w| w.fifo_rst().reset());
+        while smhc.smhc_ctrl.read().fifo_rst().is_reset() {
+            core::hint::spin_loop();
+        }
+
+        // Global interrupt disable
+        smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
+
+        prg_clk();
+
+        // Set card clock = module clock / 2
+        smhc.smhc_clkdiv.modify(|_, w| w.cclk_div().variant(1));
+
+        // Set the sample delay to 0 (also done in Linux and Allwinner BSP)
+        smhc.smhc_smap_dl.write(|w| {
+            w.samp_dl_sw().variant(0);
+            w.samp_dl_sw_en().set_bit()
+        });
+
+        // Enable the card clock
+        smhc.smhc_clkdiv.modify(|_, w| w.cclk_enb().on());
+
+        prg_clk();
+
+        // Default bus width after power up or idle is 1-bit
+        smhc.smhc_ctype.write(|w| w.card_wid().b1());
+        // Blocksize is fixed to 512 bytes
+        smhc.smhc_blksiz.write(|w| unsafe { w.bits(0x200) });
+
+        Self {
+            smhc,
+            isr: &SMHC0_ISR,
+            int: (int, isr),
+        }
+    }
+
+    fn handle_smhc0_interrupt() {
+        todo!()
+    }
+}

From d7f9249baeb21dc2a115b28cea6ae9983f6c5c13 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 30 Jul 2023 18:29:26 +0200
Subject: [PATCH 02/38] Create sdmmc kernel service, copied from i2c service

---
 source/kernel/src/services/mod.rs   |  1 +
 source/kernel/src/services/sdmmc.rs | 59 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 source/kernel/src/services/sdmmc.rs

diff --git a/source/kernel/src/services/mod.rs b/source/kernel/src/services/mod.rs
index 792d26c3..9f97a4a0 100644
--- a/source/kernel/src/services/mod.rs
+++ b/source/kernel/src/services/mod.rs
@@ -21,6 +21,7 @@
 pub mod emb_display;
 pub mod forth_spawnulator;
 pub mod i2c;
+pub mod sdmmc;
 pub mod keyboard;
 pub mod serial_mux;
 pub mod simple_serial;
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
new file mode 100644
index 00000000..4b8d5d17
--- /dev/null
+++ b/source/kernel/src/services/sdmmc.rs
@@ -0,0 +1,59 @@
+//! SD/MMC Driver Service
+//!
+//! TODO
+// TODO: #![warn(missing_docs)]
+use self::messages::*;
+use crate::{
+    comms::{
+        kchannel::{KChannel, KConsumer, KProducer},
+        oneshot::{self, Reusable},
+    },
+    mnemos_alloc::containers::FixedVec,
+    registry::{known_uuids, Envelope, KernelHandle, RegisteredDriver},
+    Kernel,
+};
+use core::{convert::Infallible, fmt, time::Duration};
+use uuid::Uuid;
+
+////////////////////////////////////////////////////////////////////////////////
+// Service Definition
+////////////////////////////////////////////////////////////////////////////////
+
+/// [Service](crate::services) definition for SD/MMC protocol drivers.
+pub struct SdmmcService;
+
+impl RegisteredDriver for SdmmcService {
+    type Request = StartTransaction;
+    type Response = Transaction;
+    type Error = core::convert::Infallible;
+
+    const UUID: Uuid = known_uuids::kernel::I2C;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Message and Error Types
+////////////////////////////////////////////////////////////////////////////////
+
+/// TODO
+#[must_use]
+pub struct Transaction {
+    tx: KProducer<Transfer>,
+    rsp_rx: Reusable<Result<FixedVec<u8>, () /* TODO */>>,
+    ended: bool,
+}
+
+pub mod messages {
+    use super::*;
+
+    pub struct StartTransaction {}
+    pub struct Transfer {}
+}
+////////////////////////////////////////////////////////////////////////////////
+// Client Definition
+////////////////////////////////////////////////////////////////////////////////
+
+/// A client for the [`SdmmcService`].
+pub struct SdmmcClient {
+    handle: KernelHandle<SdmmcService>,
+    reply: Reusable<Envelope<Result<Transaction, Infallible>>>,
+}

From f2488b9f7e9caf5e53792daf81ce3a9ca5f30c68 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 30 Jul 2023 18:30:27 +0200
Subject: [PATCH 03/38] Copy some kernel communication stuff from TWI to SMHC
 driver

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 81 ++++++++++++++++++-
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index ed695016..351b4891 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -18,7 +18,9 @@ use d1_pac::{smhc, Interrupt, GPIO, SMHC0, SMHC1, SMHC2};
 use kernel::{
     comms::kchannel::{KChannel, KConsumer},
     mnemos_alloc::containers::FixedVec,
-    registry, Kernel,
+    registry,
+    services::sdmmc::{messages::Transfer, SdmmcService, Transaction},
+    tracing, Kernel,
 };
 
 /// TODO
@@ -141,7 +143,7 @@ impl Smhc {
         )
     }
 
-    /// This assumes the GPIO pin mappings are already configured.
+    /// This assumes the GPIO pin mappings and module clock are already configured.
     unsafe fn init(smhc: &'static smhc::RegisterBlock, int: Interrupt, isr: fn()) -> Self {
         // Closure to change the card clock
         let prg_clk = || {
@@ -203,6 +205,81 @@ impl Smhc {
     }
 
     fn handle_smhc0_interrupt() {
+        let _isr = kernel::isr::Isr::enter();
+        let smhc = unsafe { &*SMHC0::ptr() };
+        let data = unsafe { &mut (*SMHC0_ISR.data.get()) };
+
+        data.advance_isr(smhc, 0);
+    }
+
+    pub async fn register(self, kernel: &'static Kernel, queued: usize) -> Result<(), ()> {
+        let (tx, rx) = KChannel::new_async(queued).await.split();
+
+        kernel.spawn(self.run(rx)).await;
+        tracing::info!("SMHC driver task spawned");
+        kernel
+            .with_registry(move |reg| reg.register_konly::<SdmmcService>(&tx).map_err(drop))
+            .await?;
+
+        Ok(())
+    }
+
+    #[tracing::instrument(name = "SMHC", level = tracing::Level::INFO, skip(self, rx))]
+    async fn run(self, rx: KConsumer<registry::Message<SdmmcService>>) {
+        tracing::info!("starting SMHC driver task");
+        while let Ok(registry::Message { msg, reply }) = rx.dequeue_async().await {
+            todo!()
+        }
+    }
+
+    #[tracing::instrument(level = tracing::Level::DEBUG, skip(self, txn))]
+    async fn transaction(&self, txn: KConsumer<Transfer>) {
+        todo!()
+    }
+}
+
+impl IsrData {
+    #[must_use]
+    fn lock<'a>(&'a self, smhc: &'a smhc::RegisterBlock) -> SmhcDataGuard<'a> {
+        // disable interrupts while holding the guard.
+        smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
+        let data = unsafe { &mut *(self.data.get()) };
+        SmhcDataGuard { data, smhc }
+    }
+}
+
+impl Drop for SmhcDataGuard<'_> {
+    fn drop(&mut self) {
+        self.smhc.smhc_ctrl.modify(|_, w| w.ine_enb().enable());
+    }
+}
+
+impl SmhcDataGuard<'_> {
+    async fn wait_for_irq(&mut self) {
+        futures::future::poll_fn(|cx| {
+            // TODO
+            Poll::Ready(())
+        })
+        .await;
+    }
+}
+
+impl Deref for SmhcDataGuard<'_> {
+    type Target = SmhcData;
+
+    fn deref(&self) -> &Self::Target {
+        &*self.data
+    }
+}
+
+impl DerefMut for SmhcDataGuard<'_> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.data
+    }
+}
+
+impl SmhcData {
+    fn advance_isr(&mut self, smhc: &smhc::RegisterBlock, num: u8) {
         todo!()
     }
 }

From ef24fa1b333f0cd6f91e34523cd21bd9f1963b73 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 30 Jul 2023 23:04:04 +0200
Subject: [PATCH 04/38] Add SDMMC uuid

---
 source/kernel/src/registry/mod.rs   | 1 +
 source/kernel/src/services/sdmmc.rs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/source/kernel/src/registry/mod.rs b/source/kernel/src/registry/mod.rs
index baa21405..f5699d85 100644
--- a/source/kernel/src/registry/mod.rs
+++ b/source/kernel/src/registry/mod.rs
@@ -44,6 +44,7 @@ pub mod known_uuids {
         pub const KEYBOARD: Uuid = uuid!("524d77b1-499c-440b-bd62-e63c0918efb5");
         pub const KEYBOARD_MUX: Uuid = uuid!("70861d1c-9f01-4e9b-89e6-ede77d8f26d8");
         pub const EMB_DISPLAY_V2: Uuid = uuid!("aa6a2af8-afd8-40e3-83c2-2c501c698aa8");
+        pub const SDMMC: Uuid = uuid!("9f4f8244-c986-4212-982e-d35890260de4");
     }
 
     // In case you need to iterate over every UUID
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 4b8d5d17..2b1200cc 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -27,7 +27,7 @@ impl RegisteredDriver for SdmmcService {
     type Response = Transaction;
     type Error = core::convert::Infallible;
 
-    const UUID: Uuid = known_uuids::kernel::I2C;
+    const UUID: Uuid = known_uuids::kernel::SDMMC;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

From b7c1a2f85f9c30077e89ab6868ba88e996bab663 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sat, 5 Aug 2023 09:00:43 +0200
Subject: [PATCH 05/38] Replace Transfer/Transaction with Command/Response in
 sdmmc service

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 10 +--
 source/kernel/src/services/sdmmc.rs           | 79 +++++++++++++++----
 2 files changed, 67 insertions(+), 22 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 351b4891..dafe9e13 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -19,7 +19,7 @@ use kernel::{
     comms::kchannel::{KChannel, KConsumer},
     mnemos_alloc::containers::FixedVec,
     registry,
-    services::sdmmc::{messages::Transfer, SdmmcService, Transaction},
+    services::sdmmc::{Command, Request, SdmmcService},
     tracing, Kernel,
 };
 
@@ -232,10 +232,10 @@ impl Smhc {
         }
     }
 
-    #[tracing::instrument(level = tracing::Level::DEBUG, skip(self, txn))]
-    async fn transaction(&self, txn: KConsumer<Transfer>) {
-        todo!()
-    }
+    // #[tracing::instrument(level = tracing::Level::DEBUG, skip(self, txn))]
+    // async fn transaction(&self, txn: KConsumer<Transfer>) {
+    //     todo!()
+    // }
 }
 
 impl IsrData {
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 2b1200cc..17d777e3 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -1,8 +1,11 @@
 //! SD/MMC Driver Service
 //!
-//! TODO
-// TODO: #![warn(missing_docs)]
-use self::messages::*;
+//! Driver for SD memory cards, SDIO cards and (e)MMC cards.
+//! This kernel driver will implement the actual protocol
+//! (which commands to send and how to interpret the response),
+//! while the platform driver will implement the device specific part
+//! (how to send and receive the data).
+#![warn(missing_docs)]
 use crate::{
     comms::{
         kchannel::{KChannel, KConsumer, KProducer},
@@ -23,8 +26,8 @@ use uuid::Uuid;
 pub struct SdmmcService;
 
 impl RegisteredDriver for SdmmcService {
-    type Request = StartTransaction;
-    type Response = Transaction;
+    type Request = Command;
+    type Response = Response;
     type Error = core::convert::Infallible;
 
     const UUID: Uuid = known_uuids::kernel::SDMMC;
@@ -34,26 +37,68 @@ impl RegisteredDriver for SdmmcService {
 // Message and Error Types
 ////////////////////////////////////////////////////////////////////////////////
 
+/// Control or data command.
+/// This is the same for SD and MMC (?), but the content should be different.
+/// For data command, should also contain the buffers to read/write?
+pub struct Command {
+    /// The numeric value of the command
+    command: u8,
+    /// Argument value that should be sent on the control line
+    argument: u32,
+    /// The type of command
+    cmd_type: CommandType,
+    /// The expected length of the response
+    rsp_size: ResponseLength,
+    /// Will the card respond with a CRC that needs to be checked
+    crc: bool,
+    /// Incoming or outgoing data
+    buffer: Option<FixedVec<u8>>,
+}
+
 /// TODO
-#[must_use]
-pub struct Transaction {
-    tx: KProducer<Transfer>,
-    rsp_rx: Reusable<Result<FixedVec<u8>, () /* TODO */>>,
-    ended: bool,
+pub enum CommandType {
+    Control,
+    Read,
+    Write,
+}
+
+/// TODO
+pub enum ResponseLength {
+    /// 48-bits
+    Short,
+    /// 136-bits
+    Long,
 }
 
-pub mod messages {
-    use super::*;
+/// Response returned by the card, can be short or long, depending on command.
+/// For read command, you can find the data in previous buffer?
+#[must_use]
+pub struct Response {
+}
 
-    pub struct StartTransaction {}
-    pub struct Transfer {}
+/// TODO
+#[derive(Debug, Eq, PartialEq)]
+pub enum Error {
 }
+
 ////////////////////////////////////////////////////////////////////////////////
 // Client Definition
 ////////////////////////////////////////////////////////////////////////////////
 
-/// A client for the [`SdmmcService`].
-pub struct SdmmcClient {
+/// A client for SD memory cards using the [`SdmmcService`].
+pub struct SdCardClient {
+    handle: KernelHandle<SdmmcService>,
+    reply: Reusable<Envelope<Result<Response, Error>>>,
+}
+
+/// A client for SDIO cards using the [`SdmmcService`].
+pub struct SdioClient {
+    handle: KernelHandle<SdmmcService>,
+    reply: Reusable<Envelope<Result<Response, Error>>>,
+}
+
+/// A client for MMC cards using the [`SdmmcService`].
+pub struct MmcClient {
     handle: KernelHandle<SdmmcService>,
-    reply: Reusable<Envelope<Result<Transaction, Infallible>>>,
+    reply: Reusable<Envelope<Result<Response, Error>>>,
 }

From 95e5daad47765f9104590b1340dae61cb8ad740d Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 3 Sep 2023 18:56:28 +0200
Subject: [PATCH 06/38] sdmmc: rework `Command` and start implementation of
 `SdCardClient`

---
 source/kernel/src/services/sdmmc.rs | 136 +++++++++++++++++++++++-----
 1 file changed, 115 insertions(+), 21 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 17d777e3..f7c00a7a 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -28,7 +28,7 @@ pub struct SdmmcService;
 impl RegisteredDriver for SdmmcService {
     type Request = Command;
     type Response = Response;
-    type Error = core::convert::Infallible;
+    type Error = Error;
 
     const UUID: Uuid = known_uuids::kernel::SDMMC;
 }
@@ -37,48 +37,93 @@ impl RegisteredDriver for SdmmcService {
 // Message and Error Types
 ////////////////////////////////////////////////////////////////////////////////
 
-/// Control or data command.
-/// This is the same for SD and MMC (?), but the content should be different.
-/// For data command, should also contain the buffers to read/write?
+/// Parameters for building a command to be sent to the card.
+///
+/// The command format must follow the SD specification and is sent on the `CMD` line.
+/// It is 48-bit in length, containing a 6-bit command index and 32-bit argument.
+/// Besides that it includes 7-bit CRC and start, transmission and end bit.
+///
+/// The command structure is the same for SD memory, SDIO and MMC (?),
+/// but the content can be very different.
+/// Therefore the content of the commands is decided here in the kernel service,
+/// while the platform driver has the low-level implementation
+/// for how to physically send the necessary bits to the card.
 pub struct Command {
     /// The numeric value of the command
-    command: u8,
-    /// Argument value that should be sent on the control line
-    argument: u32,
+    pub index: u8,
+    /// The argument value for the command
+    pub argument: u32,
     /// The type of command
-    cmd_type: CommandType,
-    /// The expected length of the response
-    rsp_size: ResponseLength,
-    /// Will the card respond with a CRC that needs to be checked
-    crc: bool,
-    /// Incoming or outgoing data
-    buffer: Option<FixedVec<u8>>,
+    pub kind: CommandKind,
+    /// The expected type of the response
+    pub rsp_type: ResponseType,
+    /// Whether the response CRC needs to be checked
+    pub rsp_crc: bool,
+    /// Optional buffer for incoming or outgoing data
+    pub buffer: Option<FixedVec<u8>>,
 }
 
 /// TODO
-pub enum CommandType {
+#[derive(Debug, Eq, PartialEq)]
+pub enum CommandKind {
+    /// Command without data transfer
     Control,
-    Read,
-    Write,
+    /// Command for reading data, contains the number of bytes to read
+    Read(u32),
+    /// Command for writing data, contains the number of bytes to write
+    Write(u32),
 }
 
 /// TODO
-pub enum ResponseLength {
-    /// 48-bits
+#[derive(Debug, Eq, PartialEq)]
+pub enum ResponseType {
+    /// No Response
+    None,
+    /// Response with 48-bit length
     Short,
-    /// 136-bits
+    /// Response with 48-bit length + check *busy* after response
+    ShortWithBusySignal,
+    /// Response with 136-bit length
     Long,
 }
 
 /// Response returned by the card, can be short or long, depending on command.
 /// For read command, you can find the data in previous buffer?
 #[must_use]
-pub struct Response {
+pub enum Response {
+    /// The 32-bit value from the 48-bit response.
+    Short(u32),
+    /// The 128-bit value from the 136-bit response.
+    // TODO: make this `u128`?
+    Long([u32; 4]),
 }
 
 /// TODO
 #[derive(Debug, Eq, PartialEq)]
 pub enum Error {
+    /// TODO
+    Busy,
+    /// TODO
+    Response,
+    /// TODO
+    Data,
+    /// TODO
+    Timeout,
+    /// TODO
+    Other,
+}
+
+impl Default for Command {
+    fn default() -> Self {
+        Command {
+            index: 0,
+            argument: 0,
+            kind: CommandKind::Control,
+            rsp_type: ResponseType::None,
+            rsp_crc: false,
+            buffer: None,
+        }
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -91,6 +136,55 @@ pub struct SdCardClient {
     reply: Reusable<Envelope<Result<Response, Error>>>,
 }
 
+impl SdCardClient {
+    /// Obtain an `SdCardClient`
+    ///
+    /// If the [`SdmmcService`] hasn't been registered yet, we will retry until it
+    /// has been registered.
+    pub async fn from_registry(kernel: &'static Kernel) -> Self {
+        loop {
+            match SdCardClient::from_registry_no_retry(kernel).await {
+                Some(client) => return client,
+                None => {
+                    // SMHC0 probably isn't registered yet. Try again in a bit
+                    kernel.sleep(Duration::from_millis(10)).await;
+                }
+            }
+        }
+    }
+
+    /// Obtain an `SdCardClient`
+    ///
+    /// Does NOT attempt to get an [`SdmmcService`] handle more than once.
+    ///
+    /// Prefer [`SdCardClient::from_registry`] unless you will not be spawning one
+    /// around the same time as obtaining a client.
+    pub async fn from_registry_no_retry(kernel: &'static Kernel) -> Option<Self> {
+        let handle = kernel
+            .with_registry(|reg| reg.get::<SdmmcService>())
+            .await?;
+
+        Some(Self {
+            handle,
+            reply: Reusable::new_async().await,
+        })
+    }
+
+    pub async fn reset(&mut self) {
+        let response = self
+            .handle
+            .request_oneshot(Command::default(), &self.reply)
+            .await
+            .map_err(|error| {
+                tracing::warn!(?error, "failed to send request to SD/MMC service");
+                Error::Other // TODO
+            })
+            .and_then(|resp| resp.body);
+    }
+
+    pub async fn initialize(&mut self) {}
+}
+
 /// A client for SDIO cards using the [`SdmmcService`].
 pub struct SdioClient {
     handle: KernelHandle<SdmmcService>,

From 9d1f933bfd6876d401d2b1812d304ad81318cba6 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 3 Sep 2023 18:57:34 +0200
Subject: [PATCH 07/38] smhc driver: initial implementation of `run` and
 `command` functions

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 179 +++++++++++++++---
 1 file changed, 156 insertions(+), 23 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index dafe9e13..89999070 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -19,7 +19,7 @@ use kernel::{
     comms::kchannel::{KChannel, KConsumer},
     mnemos_alloc::containers::FixedVec,
     registry,
-    services::sdmmc::{Command, Request, SdmmcService},
+    services::sdmmc::{self, SdmmcService},
     tracing, Kernel,
 };
 
@@ -51,15 +51,37 @@ struct SmhcData {
 static SMHC0_ISR: IsrData = IsrData {
     data: UnsafeCell::new(SmhcData {
         state: State::Idle,
-        op: SmhcOp::None,
+        op: SmhcOp::Control,
         err: None,
         waker: None,
     }),
 };
 
 enum SmhcOp {
-    // TODO
-    None,
+    Control,
+    Read {
+        buf: FixedVec<u8>,
+        cnt: u32,
+        auto_stop: bool,
+    },
+    Write {
+        buf: FixedVec<u8>,
+        cnt: u32,
+        auto_stop: bool,
+    },
+}
+
+/// TODO
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[allow(dead_code)]
+enum State {
+    Idle,
+    /// Waiting for command to be completed.
+    WaitForCommand,
+    /// Waiting for data transfer to be completed.
+    WaitForDataTransfer,
+    /// Waiting for the auto-stop command to be sent and completed.
+    WaitForAutoStop,
 }
 
 /// TODO
@@ -95,14 +117,6 @@ pub enum ErrorKind {
     Other,
 }
 
-/// TODO
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-#[allow(dead_code)]
-enum State {
-    Idle,
-    // TODO
-}
-
 impl Smhc {
     /// Initialize SMHC0 for SD cards.
     ///
@@ -136,6 +150,23 @@ impl Smhc {
         });
         ccu.enable_module(&mut smhc);
 
+        // Enable interrupts that are relevant for an SD card
+        smhc.smhc_intmask.write(|w| {
+            w.dee_int_en().set_bit();
+            w.acd_int_en().set_bit();
+            w.dse_bc_int_en().set_bit();
+            w.cb_iw_int_en().set_bit();
+            w.fu_fo_int_en().set_bit();
+            w.dsto_vsd_int_en().set_bit();
+            w.dto_bds_int_en().set_bit();
+            w.dce_int_en().set_bit();
+            w.rce_int_en().set_bit();
+            w.dtc_int_en().set_bit();
+            w.cc_int_en().set_bit();
+            w.re_int_en().set_bit();
+            w
+        });
+
         Self::init(
             unsafe { &*SMHC0::ptr() },
             Interrupt::SMHC0,
@@ -181,7 +212,7 @@ impl Smhc {
         // Set card clock = module clock / 2
         smhc.smhc_clkdiv.modify(|_, w| w.cclk_div().variant(1));
 
-        // Set the sample delay to 0 (also done in Linux and Allwinner BSP)
+        // Set the sample delay to 0 (as done in Linux and Allwinner BSP)
         smhc.smhc_smap_dl.write(|w| {
             w.samp_dl_sw().variant(0);
             w.samp_dl_sw_en().set_bit()
@@ -204,9 +235,10 @@ impl Smhc {
         }
     }
 
-    fn handle_smhc0_interrupt() {
+    pub fn handle_smhc0_interrupt() {
         let _isr = kernel::isr::Isr::enter();
         let smhc = unsafe { &*SMHC0::ptr() };
+        // safety: it's okay to do this since this function can only be called from inside the ISR.
         let data = unsafe { &mut (*SMHC0_ISR.data.get()) };
 
         data.advance_isr(smhc, 0);
@@ -227,15 +259,89 @@ impl Smhc {
     #[tracing::instrument(name = "SMHC", level = tracing::Level::INFO, skip(self, rx))]
     async fn run(self, rx: KConsumer<registry::Message<SdmmcService>>) {
         tracing::info!("starting SMHC driver task");
-        while let Ok(registry::Message { msg, reply }) = rx.dequeue_async().await {
-            todo!()
+        while let Ok(registry::Message { mut msg, reply }) = rx.dequeue_async().await {
+            let response = self.command(msg.body).await;
+            // TODO: we don't need `msg.body` anymore, but since it has been moved
+            // we need to supply another value if we want to use `msg` later to reply.
+            msg.body = sdmmc::Command::default();
+            if let Err(error) = reply.reply_konly(msg.reply_with(response)).await {
+                tracing::warn!(?error, "client hung up...");
+            }
         }
     }
 
-    // #[tracing::instrument(level = tracing::Level::DEBUG, skip(self, txn))]
-    // async fn transaction(&self, txn: KConsumer<Transfer>) {
-    //     todo!()
-    // }
+    #[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);
+        }
+
+        let cmd_idx = params.index;
+        // TODO: naive `auto_stop` selection, probably only works in case of SD memory cards.
+        // Should this (auto_stop select) be part of the params?
+        let (data_trans, trans_dir, auto_stop) = match params.kind {
+            sdmmc::CommandKind::Control => (false, false, false),
+            sdmmc::CommandKind::Read(len) => (true, false, len > 512),
+            sdmmc::CommandKind::Write(len) => (true, true, len > 512),
+        };
+        let chk_resp_crc = params.rsp_crc;
+        let long_resp = params.rsp_type == sdmmc::ResponseType::Long;
+        let resp_rcv = params.rsp_type != sdmmc::ResponseType::None;
+
+        // Configure and start command
+        self.smhc.smhc_cmd.write(|w| {
+            w.cmd_load().set_bit();
+            w.wait_pre_over().wait();
+            w.stop_cmd_flag().bit(auto_stop);
+            w.data_trans().bit(data_trans);
+            w.trans_dir().bit(trans_dir);
+            w.chk_resp_crc().bit(chk_resp_crc);
+            w.long_resp().bit(long_resp);
+            w.resp_rcv().bit(resp_rcv);
+            w.cmd_idx().variant(cmd_idx);
+            w
+        });
+
+        // Now wait for completion or error interrupt
+        let mut guard = self.isr.lock(self.smhc);
+        guard.data.state = State::WaitForCommand;
+        guard.data.op = match (params.kind, params.buffer) {
+            (sdmmc::CommandKind::Control, _) => SmhcOp::Control,
+            (sdmmc::CommandKind::Read(cnt), Some(buf)) => SmhcOp::Read {
+                buf,
+                cnt,
+                auto_stop,
+            },
+            (sdmmc::CommandKind::Write(cnt), Some(buf)) => SmhcOp::Write {
+                buf,
+                cnt,
+                auto_stop,
+            },
+            _ => {
+                tracing::warn!("did not provide a buffer for read/write");
+                return Err(sdmmc::Error::Other);
+            }
+        };
+
+        guard.wait_for_irq().await;
+        tracing::trace!("SMHC operation completed");
+        let res = if let Some(error) = guard.data.err.take() {
+            tracing::warn!(?error, "SMHC error");
+            Err(sdmmc::Error::Other) // TODO
+        } else {
+            if long_resp {
+                Ok(sdmmc::Response::Long([
+                    self.smhc.smhc_resp0.read().bits(),
+                    self.smhc.smhc_resp1.read().bits(),
+                    self.smhc.smhc_resp2.read().bits(),
+                    self.smhc.smhc_resp3.read().bits(),
+                ]))
+            } else {
+                Ok(sdmmc::Response::Short(self.smhc.smhc_resp0.read().bits()))
+            }
+        };
+        res
+    }
 }
 
 impl IsrData {
@@ -256,9 +362,18 @@ impl Drop for SmhcDataGuard<'_> {
 
 impl SmhcDataGuard<'_> {
     async fn wait_for_irq(&mut self) {
+        let mut waiting = false;
         futures::future::poll_fn(|cx| {
-            // TODO
-            Poll::Ready(())
+            if waiting {
+                self.smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
+                return Poll::Ready(());
+            }
+
+            self.data.waker = Some(cx.waker().clone());
+            waiting = true;
+            self.smhc.smhc_ctrl.modify(|_, w| w.ine_enb().enable());
+
+            Poll::Pending
         })
         .await;
     }
@@ -280,6 +395,24 @@ impl DerefMut for SmhcDataGuard<'_> {
 
 impl SmhcData {
     fn advance_isr(&mut self, smhc: &smhc::RegisterBlock, num: u8) {
-        todo!()
+        let mut needs_wake = false;
+        tracing::trace!(state = ?self.state, smhc = num, "SMHC{num} interrupt");
+
+        match self.state {
+            State::Idle => (),
+            State::WaitForCommand => (),
+            State::WaitForDataTransfer => (),
+            State::WaitForAutoStop => (),
+        }
+
+        if needs_wake {
+            if let Some(waker) = self.waker.take() {
+                waker.wake();
+                // If we are waking the driver task, we need to disable interrupts
+                // until the driver can prepare the next phase of the transaction.
+                smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
+            }
+        }
+        // TODO
     }
 }

From cdd8ce799e9dfa9b256250f0da5c3692b494fc81 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 3 Sep 2023 18:58:00 +0200
Subject: [PATCH 08/38] Add smhc in `src/lib.rs`

---
 platforms/allwinner-d1/src/lib.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/platforms/allwinner-d1/src/lib.rs b/platforms/allwinner-d1/src/lib.rs
index b4779555..17f15fc4 100644
--- a/platforms/allwinner-d1/src/lib.rs
+++ b/platforms/allwinner-d1/src/lib.rs
@@ -8,6 +8,7 @@ use self::{
     ccu::Ccu,
     dmac::Dmac,
     drivers::{
+        smhc::Smhc,
         spim::{self, SpiSenderServer},
         twi,
         uart::{self, D1Uart, Uart},
@@ -50,6 +51,7 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
 
     let uart = unsafe { uart::kernel_uart(&mut ccu, &mut p.GPIO, p.UART0) };
     let spim = unsafe { spim::kernel_spim1(p.SPI_DBI, &mut ccu, &mut p.GPIO) };
+    let smhc0 = unsafe { Smhc::new(p.SMHC0, &mut ccu, &mut p.GPIO) };
 
     let i2c0 = match config.platform.i2c {
         d1_config::I2cConfiguration { enabled: false, .. } => None,
@@ -79,6 +81,7 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
         dmac,
         uart,
         spim,
+        smhc0,
         plic,
         i2c0,
         config.kernel,
@@ -183,6 +186,7 @@ impl D1 {
         dmac: Dmac,
         uart: Uart,
         spim: spim::Spim1,
+        smhc: Smhc,
         plic: Plic,
         i2c0: Option<twi::I2c0>,
         kernel_settings: KernelSettings,
@@ -224,6 +228,12 @@ impl D1 {
             i2c0_int
         });
 
+        // Initialize SMHC driver
+        k.initialize(async {
+            smhc.register(k, 4).await.unwrap();
+        })
+        .unwrap();
+
         Self {
             kernel: k,
             _uart: uart,
@@ -320,9 +330,11 @@ impl D1 {
             plic.register(Interrupt::TIMER1, Self::timer1_int);
             plic.register(Interrupt::DMAC_NS, Dmac::handle_interrupt);
             plic.register(Interrupt::UART0, D1Uart::handle_uart0_int);
+            plic.register(Interrupt::SMHC0, Smhc::handle_smhc0_interrupt);
 
             plic.activate(Interrupt::DMAC_NS, Priority::P1).unwrap();
             plic.activate(Interrupt::UART0, Priority::P1).unwrap();
+            plic.activate(Interrupt::SMHC0, Priority::P1).unwrap();
 
             if let Some((i2c0_int, i2c0_isr)) = i2c0_int {
                 plic.register(i2c0_int, i2c0_isr);

From 63e88b04ef0d4a44f13193e020e83f7f24ccc029 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Wed, 20 Sep 2023 11:19:42 +0200
Subject: [PATCH 09/38] Rebase on main

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 21 ++++++++----
 source/kernel/src/services/sdmmc.rs           | 33 ++++++++++---------
 2 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 89999070..22fe63b4 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -244,22 +244,29 @@ impl Smhc {
         data.advance_isr(smhc, 0);
     }
 
-    pub async fn register(self, kernel: &'static Kernel, queued: usize) -> Result<(), ()> {
-        let (tx, rx) = KChannel::new_async(queued).await.split();
+    pub async fn register(
+        self,
+        kernel: &'static Kernel,
+        queued: usize,
+    ) -> Result<(), registry::RegistrationError> {
+        let rx = kernel
+            .registry()
+            .bind_konly::<SdmmcService>(queued)
+            .await?
+            .into_request_stream(queued)
+            .await;
 
         kernel.spawn(self.run(rx)).await;
         tracing::info!("SMHC driver task spawned");
-        kernel
-            .with_registry(move |reg| reg.register_konly::<SdmmcService>(&tx).map_err(drop))
-            .await?;
 
         Ok(())
     }
 
     #[tracing::instrument(name = "SMHC", level = tracing::Level::INFO, skip(self, rx))]
-    async fn run(self, rx: KConsumer<registry::Message<SdmmcService>>) {
+    async fn run(self, rx: registry::listener::RequestStream<SdmmcService>) {
         tracing::info!("starting SMHC driver task");
-        while let Ok(registry::Message { mut msg, reply }) = rx.dequeue_async().await {
+        loop {
+            let registry::Message { mut msg, reply } = rx.next_request().await;
             let response = self.command(msg.body).await;
             // TODO: we don't need `msg.body` anymore, but since it has been moved
             // we need to supply another value if we want to use `msg` later to reply.
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index f7c00a7a..4a052049 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -12,7 +12,7 @@ use crate::{
         oneshot::{self, Reusable},
     },
     mnemos_alloc::containers::FixedVec,
-    registry::{known_uuids, Envelope, KernelHandle, RegisteredDriver},
+    registry::{self, known_uuids, Envelope, KernelHandle, RegisteredDriver},
     Kernel,
 };
 use core::{convert::Infallible, fmt, time::Duration};
@@ -29,6 +29,8 @@ impl RegisteredDriver for SdmmcService {
     type Request = Command;
     type Response = Response;
     type Error = Error;
+    type Hello = ();
+    type ConnectError = core::convert::Infallible;
 
     const UUID: Uuid = known_uuids::kernel::SDMMC;
 }
@@ -141,16 +143,15 @@ impl SdCardClient {
     ///
     /// If the [`SdmmcService`] hasn't been registered yet, we will retry until it
     /// has been registered.
-    pub async fn from_registry(kernel: &'static Kernel) -> Self {
-        loop {
-            match SdCardClient::from_registry_no_retry(kernel).await {
-                Some(client) => return client,
-                None => {
-                    // SMHC0 probably isn't registered yet. Try again in a bit
-                    kernel.sleep(Duration::from_millis(10)).await;
-                }
-            }
-        }
+    pub async fn from_registry(
+        kernel: &'static Kernel,
+    ) -> Result<Self, registry::ConnectError<SdmmcService>> {
+        let handle = kernel.registry().connect::<SdmmcService>(()).await?;
+
+        Ok(Self {
+            handle,
+            reply: Reusable::new_async().await,
+        })
     }
 
     /// Obtain an `SdCardClient`
@@ -159,12 +160,12 @@ impl SdCardClient {
     ///
     /// Prefer [`SdCardClient::from_registry`] unless you will not be spawning one
     /// around the same time as obtaining a client.
-    pub async fn from_registry_no_retry(kernel: &'static Kernel) -> Option<Self> {
-        let handle = kernel
-            .with_registry(|reg| reg.get::<SdmmcService>())
-            .await?;
+    pub async fn from_registry_no_retry(
+        kernel: &'static Kernel,
+    ) -> Result<Self, registry::ConnectError<SdmmcService>> {
+        let handle = kernel.registry().try_connect::<SdmmcService>(()).await?;
 
-        Some(Self {
+        Ok(Self {
             handle,
             reply: Reusable::new_async().await,
         })

From 36663e7bbe49c8a4364a90113413f09375876878 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Wed, 20 Sep 2023 11:20:38 +0200
Subject: [PATCH 10/38] sdmmc: add *data* field to `Response::Short`

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 24 +++++++++++++------
 source/kernel/src/services/sdmmc.rs           |  6 ++++-
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 22fe63b4..68feacf7 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -58,6 +58,7 @@ static SMHC0_ISR: IsrData = IsrData {
 };
 
 enum SmhcOp {
+    None,
     Control,
     Read {
         buf: FixedVec<u8>,
@@ -344,7 +345,13 @@ impl Smhc {
                     self.smhc.smhc_resp3.read().bits(),
                 ]))
             } else {
-                Ok(sdmmc::Response::Short(self.smhc.smhc_resp0.read().bits()))
+                Ok(sdmmc::Response::Short {
+                    value: self.smhc.smhc_resp0.read().bits(),
+                    data: match core::mem::replace(&mut guard.data.op, SmhcOp::None) {
+                        SmhcOp::Read { buf, .. } => Some(buf),
+                        _ => None,
+                    },
+                })
             }
         };
         res
@@ -405,12 +412,15 @@ impl SmhcData {
         let mut needs_wake = false;
         tracing::trace!(state = ?self.state, smhc = num, "SMHC{num} interrupt");
 
-        match self.state {
-            State::Idle => (),
-            State::WaitForCommand => (),
-            State::WaitForDataTransfer => (),
-            State::WaitForAutoStop => (),
-        }
+        let new_state = match self.state {
+            State::Idle => State::Idle,
+            State::WaitForCommand => match self.op {
+                SmhcOp::None | SmhcOp::Control => State::Idle,
+                SmhcOp::Read { .. } | SmhcOp::Write { .. } => State::WaitForCommand,
+            },
+            State::WaitForDataTransfer => State::WaitForAutoStop,
+            State::WaitForAutoStop => State::Idle,
+        };
 
         if needs_wake {
             if let Some(waker) = self.waker.take() {
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 4a052049..8df8188f 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -94,7 +94,11 @@ pub enum ResponseType {
 #[must_use]
 pub enum Response {
     /// The 32-bit value from the 48-bit response.
-    Short(u32),
+    /// Potentially also includes the data vector if this was a read command.
+    Short {
+        value: u32,
+        data: Option<FixedVec<u8>>,
+    },
     /// The 128-bit value from the 136-bit response.
     // TODO: make this `u128`?
     Long([u32; 4]),

From 183dd96c689622ed0d74a4345056de2c3063ed18 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Wed, 20 Sep 2023 11:21:16 +0200
Subject: [PATCH 11/38] smhc driver: implement `advance_isr`

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 97 +++++++++++++++++--
 1 file changed, 87 insertions(+), 10 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 68feacf7..934e572b 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -296,6 +296,11 @@ impl Smhc {
         let long_resp = params.rsp_type == sdmmc::ResponseType::Long;
         let resp_rcv = params.rsp_type != sdmmc::ResponseType::None;
 
+        // Set the argument to be sent
+        self.smhc
+            .smhc_cmdarg
+            .write(|w| unsafe { w.bits(params.argument) });
+
         // Configure and start command
         self.smhc.smhc_cmd.write(|w| {
             w.cmd_load().set_bit();
@@ -408,28 +413,100 @@ impl DerefMut for SmhcDataGuard<'_> {
 }
 
 impl SmhcData {
+    fn error_status(smhc: &smhc::RegisterBlock) -> Option<ErrorKind> {
+        let mint = smhc.smhc_mintsts.read();
+        if mint.m_rto_back_int().bit_is_set() {
+            return Some(ErrorKind::ResponseTimeout);
+        }
+        if mint.m_rce_int().bit_is_set() {
+            return Some(ErrorKind::ResponseCrc);
+        }
+        if mint.m_re_int().bit_is_set() {
+            return Some(ErrorKind::Response);
+        }
+        if mint.m_dsto_vsd_int().bit_is_set() {
+            return Some(ErrorKind::DataStarvationTimeout);
+        }
+        if mint.m_dto_bds_int().bit_is_set() {
+            return Some(ErrorKind::DataTimeout);
+        }
+        if mint.m_dce_int().bit_is_set() {
+            return Some(ErrorKind::DataCrc);
+        }
+        if mint.m_fu_fo_int().bit_is_set() {
+            return Some(ErrorKind::FifoUnderrunOverflow);
+        }
+        if mint.m_cb_iw_int().bit_is_set() {
+            return Some(ErrorKind::CommandBusyIllegalWrite);
+        }
+        if mint.m_dse_bc_int().bit_is_set() {
+            return Some(ErrorKind::DataStart);
+        }
+        if mint.m_dee_int().bit_is_set() {
+            return Some(ErrorKind::DataEnd);
+        }
+
+        None
+    }
+
     fn advance_isr(&mut self, smhc: &smhc::RegisterBlock, num: u8) {
-        let mut needs_wake = false;
         tracing::trace!(state = ?self.state, smhc = num, "SMHC{num} interrupt");
 
-        let new_state = match self.state {
+        self.err = Self::error_status(smhc);
+        let mut needs_wake = self.err.is_some();
+
+        self.state = match self.state {
             State::Idle => State::Idle,
-            State::WaitForCommand => match self.op {
-                SmhcOp::None | SmhcOp::Control => State::Idle,
-                SmhcOp::Read { .. } | SmhcOp::Write { .. } => State::WaitForCommand,
-            },
-            State::WaitForDataTransfer => State::WaitForAutoStop,
-            State::WaitForAutoStop => State::Idle,
+            State::WaitForCommand => {
+                if smhc.smhc_mintsts.read().m_cc_int().bit_is_set() {
+                    match self.op {
+                        SmhcOp::None => State::Idle,
+                        SmhcOp::Control => {
+                            needs_wake = true;
+                            State::Idle
+                        }
+                        SmhcOp::Read { .. } | SmhcOp::Write { .. } => State::WaitForDataTransfer,
+                    }
+                } else {
+                    self.state
+                }
+            }
+            State::WaitForDataTransfer => {
+                if smhc.smhc_mintsts.read().m_dtc_int().bit_is_set() {
+                    match self.op {
+                        SmhcOp::None | SmhcOp::Control => State::Idle,
+                        SmhcOp::Read { auto_stop, .. } | SmhcOp::Write { auto_stop, .. } => {
+                            if auto_stop {
+                                State::WaitForAutoStop
+                            } else {
+                                needs_wake = true;
+                                State::Idle
+                            }
+                        }
+                    }
+                } else {
+                    self.state
+                }
+            }
+            State::WaitForAutoStop => {
+                if smhc.smhc_mintsts.read().m_acd_int().bit_is_set() {
+                    needs_wake = true;
+                    State::Idle
+                } else {
+                    self.state
+                }
+            }
         };
 
         if needs_wake {
             if let Some(waker) = self.waker.take() {
                 waker.wake();
                 // If we are waking the driver task, we need to disable interrupts
-                // until the driver can prepare the next phase of the transaction.
                 smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
             }
         }
-        // TODO
+
+        // Clear all pending interrupts
+        smhc.smhc_rintsts.write(|w| unsafe { w.bits(0xFFFF_FFFF) });
     }
 }

From 391d9c9cb70852a66929cb7409470be0d7d829c1 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Wed, 20 Sep 2023 18:33:38 +0200
Subject: [PATCH 12/38] ccu.rs: fix swapped docs in `BusGatingResetRegister`

+ change *reset* function parameter to be `assert` instead of `deassert`
which makes more sense for a user
---
 platforms/allwinner-d1/d1-core/src/ccu.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/ccu.rs b/platforms/allwinner-d1/d1-core/src/ccu.rs
index 1b88fdea..a6a05f5b 100644
--- a/platforms/allwinner-d1/d1-core/src/ccu.rs
+++ b/platforms/allwinner-d1/d1-core/src/ccu.rs
@@ -20,10 +20,10 @@ pub struct Ccu {
 
 /// Trait to be implemented for module clocks that can be gated and reset
 pub trait BusGatingResetRegister {
-    /// Enable or disable the clock reset bit
-    fn gating(ccu: &mut CCU, pass: bool);
     /// Enable or disable the clock gating bit
-    fn reset(ccu: &mut CCU, deassert: bool);
+    fn gating(ccu: &mut CCU, pass: bool);
+    /// Enable or disable the clock reset bit
+    fn reset(ccu: &mut CCU, assert: bool);
 }
 
 // TODO: should this move into the `Clint`?
@@ -49,7 +49,7 @@ impl Ccu {
 
     /// De-assert the reset bit and enable the clock gating bit for the given module
     pub fn enable_module<MODULE: BusGatingResetRegister>(&mut self, _mod: &mut MODULE) {
-        MODULE::reset(&mut self.ccu, true);
+        MODULE::reset(&mut self.ccu, false);
         sdelay(20);
         MODULE::gating(&mut self.ccu, true);
     }
@@ -58,7 +58,7 @@ impl Ccu {
     pub fn disable_module<MODULE: BusGatingResetRegister>(&mut self, _mod: &mut MODULE) {
         MODULE::gating(&mut self.ccu, false);
         // TODO: delay?
-        MODULE::reset(&mut self.ccu, false);
+        MODULE::reset(&mut self.ccu, true);
     }
 
     /// Allow modules to configure their own clock on a PAC level
@@ -252,8 +252,8 @@ macro_rules! impl_bgr {
                     ccu.$reg.modify(|_, w| w.$gating().bit(pass));
                 }
 
-                fn reset(ccu: &mut CCU, deassert: bool) {
-                    ccu.$reg.modify(|_, w| w.$reset().bit(deassert));
+                fn reset(ccu: &mut CCU, assert: bool) {
+                    ccu.$reg.modify(|_, w| w.$reset().bit(!assert));
                 }
             }
         )+

From 6a39d6e30eb992563ccf5355ba2322588823f866 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 24 Sep 2023 09:26:45 +0200
Subject: [PATCH 13/38] sdmmc: implement `initialize` function

---
 source/kernel/src/services/sdmmc.rs | 105 +++++++++++++++++++++++++---
 1 file changed, 97 insertions(+), 8 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 8df8188f..4a144fd4 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -90,11 +90,10 @@ pub enum ResponseType {
 }
 
 /// Response returned by the card, can be short or long, depending on command.
-/// For read command, you can find the data in previous buffer?
 #[must_use]
 pub enum Response {
     /// The 32-bit value from the 48-bit response.
-    /// Potentially also includes the data vector if this was a read command.
+    /// Potentially also includes the data buffer if this was a read command.
     Short {
         value: u32,
         data: Option<FixedVec<u8>>,
@@ -142,6 +141,14 @@ pub struct SdCardClient {
     reply: Reusable<Envelope<Result<Response, Error>>>,
 }
 
+/// The different types of cards
+#[derive(Debug, PartialEq)]
+pub enum CardType {
+    SD1,
+    SD2,
+    SDHC,
+}
+
 impl SdCardClient {
     /// Obtain an `SdCardClient`
     ///
@@ -175,19 +182,101 @@ impl SdCardClient {
         })
     }
 
-    pub async fn reset(&mut self) {
-        let response = self
-            .handle
-            .request_oneshot(Command::default(), &self.reply)
+    pub async fn cmd(&mut self, cmd: Command) -> Result<Response, Error> {
+        self.handle
+            .request_oneshot(cmd, &self.reply)
             .await
             .map_err(|error| {
                 tracing::warn!(?error, "failed to send request to SD/MMC service");
                 Error::Other // TODO
             })
-            .and_then(|resp| resp.body);
+            .and_then(|resp| resp.body)
+    }
+
+    pub async fn reset(&mut self) -> Result<(), Error> {
+        self.cmd(Command::default()).await.map(|_| ())
     }
 
-    pub async fn initialize(&mut self) {}
+    pub async fn initialize(&mut self) -> Result<CardType, Error> {
+        /// Request switch to 1.8V
+        #[allow(dead_code)]
+        const OCR_S18R: u32 = 0x1000000;
+        /// Host supports high capacity
+        const OCR_HCS: u32 = 0x40000000;
+        /// Card has finished power up routine if bit is high
+        const OCR_NBUSY: u32 = 0x80000000;
+        /// Valid bits for voltage setting
+        const OCR_VOLTAGE_MASK: u32 = 0x007FFF80;
+
+        let mut card_type = CardType::SD1;
+
+        match self
+            .cmd(Command {
+                index: 8,
+                argument: 0x1AA,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?
+        {
+            Response::Short { value, .. } => {
+                tracing::trace!("CMD8 response: {value:#x}");
+                if value == 0x1AA {
+                    card_type = CardType::SD2;
+                }
+            }
+            Response::Long(_) => return Err(Error::Response),
+        }
+
+        // TODO: limit the number of attempts
+        let ocr = loop {
+            // Go to *APP* mode before sending application command
+            self.cmd(Command {
+                index: 55,
+                argument: 0,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?;
+
+            let mut op_cond_arg = OCR_VOLTAGE_MASK & 0x00ff8000;
+            if card_type != CardType::SD1 {
+                op_cond_arg = OCR_HCS | op_cond_arg;
+            }
+            match self
+                .cmd(Command {
+                    index: 41,
+                    argument: op_cond_arg,
+                    kind: CommandKind::Control,
+                    rsp_type: ResponseType::Short,
+                    rsp_crc: false,
+                    buffer: None,
+                })
+                .await?
+            {
+                Response::Short { value, .. } => {
+                    tracing::trace!("ACMD41 response: {value:#x}");
+                    if value & OCR_NBUSY == OCR_NBUSY {
+                        // Card has finished power up, data is valid
+                        break value;
+                    }
+                }
+                Response::Long(_) => return Err(Error::Response),
+            }
+
+            // TODO: wait 1ms
+        };
+
+        if (ocr & OCR_HCS) == OCR_HCS {
+            card_type = CardType::SDHC;
+        }
+
+        Ok(card_type)
+    }
 }
 
 /// A client for SDIO cards using the [`SdmmcService`].

From c32ffc064bdab5b01add0da999d3b99074ebfb05 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 24 Sep 2023 09:28:39 +0200
Subject: [PATCH 14/38] Add small function to test SdCardClient

---
 platforms/allwinner-d1/src/lib.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/platforms/allwinner-d1/src/lib.rs b/platforms/allwinner-d1/src/lib.rs
index 17f15fc4..173462c4 100644
--- a/platforms/allwinner-d1/src/lib.rs
+++ b/platforms/allwinner-d1/src/lib.rs
@@ -25,6 +25,7 @@ use core::{
 use d1_pac::{Interrupt, TIMER};
 use kernel::{
     mnemos_alloc::containers::Box,
+    services::sdmmc::SdCardClient,
     tracing::{self, Instrument},
     Kernel, KernelServiceSettings, KernelSettings,
 };
@@ -158,6 +159,25 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
     #[cfg(feature = "sharp-display")]
     d1.initialize_sharp_display();
 
+    // #[cfg(feature = "sdcard")]
+    d1.kernel
+        .initialize(async move {
+            let mut sdcard = SdCardClient::from_registry(d1.kernel)
+                .await
+                .expect("no sdmmc service running!");
+
+            d1.kernel.sleep(core::time::Duration::from_secs(3)).await;
+            tracing::debug!("resetting the SD card");
+
+            sdcard.reset().await.expect("can't reset the SD card");
+            let card_type = sdcard
+                .initialize()
+                .await
+                .expect("can't initialize the SD card");
+            tracing::debug!("initialized card of type {card_type:?}");
+        })
+        .expect("failed to spawn sdcard client");
+
     d1.run()
 }
 
@@ -230,7 +250,9 @@ impl D1 {
 
         // Initialize SMHC driver
         k.initialize(async {
+            tracing::debug!("initializing SMHC...");
             smhc.register(k, 4).await.unwrap();
+            tracing::debug!("SMHC initialized!");
         })
         .unwrap();
 

From 92438cea3fb9cf648448f0f262f3f44194249ca0 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 25 Sep 2023 21:13:15 +0200
Subject: [PATCH 15/38] sdmmc: implement more functions

- `get_cid`
- `get_rca`
- `select`
- `set_wide_bus`
---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  |  12 ++
 source/kernel/src/services/sdmmc.rs           | 148 +++++++++++++++++-
 2 files changed, 158 insertions(+), 2 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 934e572b..e9f0dc21 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -296,6 +296,18 @@ impl Smhc {
         let long_resp = params.rsp_type == sdmmc::ResponseType::Long;
         let resp_rcv = params.rsp_type != sdmmc::ResponseType::None;
 
+        // Do any required configuration
+        match params.options {
+            sdmmc::HardwareOptions::None => (),
+            sdmmc::HardwareOptions::SetBusWidth(width) => {
+                self.smhc.smhc_ctype.write(|w| match width {
+                    sdmmc::BusWidth::Single => w.card_wid().b1(),
+                    sdmmc::BusWidth::Quad => w.card_wid().b4(),
+                    sdmmc::BusWidth::Octo => w.card_wid().b8(),
+                });
+            }
+        }
+
         // Set the argument to be sent
         self.smhc
             .smhc_cmdarg
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 4a144fd4..760a591b 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -1,6 +1,6 @@
 //! SD/MMC Driver Service
 //!
-//! Driver for SD memory cards, SDIO cards and (e)MMC cards.
+//! Driver for SD memory cards, SDIO cards and (e)MMC.
 //! This kernel driver will implement the actual protocol
 //! (which commands to send and how to interpret the response),
 //! while the platform driver will implement the device specific part
@@ -55,6 +55,8 @@ pub struct Command {
     pub index: u8,
     /// The argument value for the command
     pub argument: u32,
+    /// Hardware specific configuration that should be applied
+    pub options: HardwareOptions,
     /// The type of command
     pub kind: CommandKind,
     /// The expected type of the response
@@ -65,6 +67,26 @@ pub struct Command {
     pub buffer: Option<FixedVec<u8>>,
 }
 
+/// The number of lines that are used for data transfer
+#[derive(Debug, Eq, PartialEq)]
+pub enum BusWidth {
+    /// 1-bit bus width, default after power-up or idle
+    Single,
+    /// 4-bit bus width, also called wide bus operation mode for SD cards
+    Quad,
+    /// 8-bit bus width, only available for MMC
+    Octo,
+}
+
+/// TODO
+#[derive(Debug, Eq, PartialEq)]
+pub enum HardwareOptions {
+    /// No change in configuration
+    None,
+    /// Switch the bus width
+    SetBusWidth(BusWidth),
+}
+
 /// TODO
 #[derive(Debug, Eq, PartialEq)]
 pub enum CommandKind {
@@ -123,6 +145,7 @@ impl Default for Command {
         Command {
             index: 0,
             argument: 0,
+            options: HardwareOptions::None,
             kind: CommandKind::Control,
             rsp_type: ResponseType::None,
             rsp_crc: false,
@@ -149,6 +172,21 @@ pub enum CardType {
     SDHC,
 }
 
+/// Card status in R1 response format
+pub struct CardStatus(u32);
+
+/// Card identification register in R2 response format
+///   Manufacturer ID       [127:120]
+///   OEM/Application ID    [119:104]
+///   Product name          [103:64]
+///   Product revision      [63:56]
+///   Product serial number [55:24]
+///   Reserved              [23:20]
+///   Manufacturing date    [19:8]
+///   CRC7 checksum         [7:1]
+///   Not used, always 1    [0:0]
+pub struct CardIdentification(u128);
+
 impl SdCardClient {
     /// Obtain an `SdCardClient`
     ///
@@ -182,7 +220,7 @@ impl SdCardClient {
         })
     }
 
-    pub async fn cmd(&mut self, cmd: Command) -> Result<Response, Error> {
+    async fn cmd(&mut self, cmd: Command) -> Result<Response, Error> {
         self.handle
             .request_oneshot(cmd, &self.reply)
             .await
@@ -193,10 +231,12 @@ impl SdCardClient {
             .and_then(|resp| resp.body)
     }
 
+    /// TODO
     pub async fn reset(&mut self) -> Result<(), Error> {
         self.cmd(Command::default()).await.map(|_| ())
     }
 
+    /// TODO
     pub async fn initialize(&mut self) -> Result<CardType, Error> {
         /// Request switch to 1.8V
         #[allow(dead_code)]
@@ -214,6 +254,7 @@ impl SdCardClient {
             .cmd(Command {
                 index: 8,
                 argument: 0x1AA,
+                options: HardwareOptions::None,
                 kind: CommandKind::Control,
                 rsp_type: ResponseType::Short,
                 rsp_crc: true,
@@ -236,6 +277,7 @@ impl SdCardClient {
             self.cmd(Command {
                 index: 55,
                 argument: 0,
+                options: HardwareOptions::None,
                 kind: CommandKind::Control,
                 rsp_type: ResponseType::Short,
                 rsp_crc: true,
@@ -251,6 +293,7 @@ impl SdCardClient {
                 .cmd(Command {
                     index: 41,
                     argument: op_cond_arg,
+                    options: HardwareOptions::None,
                     kind: CommandKind::Control,
                     rsp_type: ResponseType::Short,
                     rsp_crc: false,
@@ -277,6 +320,107 @@ impl SdCardClient {
 
         Ok(card_type)
     }
+
+    /// Get the card identification register
+    pub async fn get_cid(&mut self) -> Result<CardIdentification, Error> {
+        match self
+            .cmd(Command {
+                index: 2,
+                argument: 0,
+                options: HardwareOptions::None,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Long,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?
+        {
+            Response::Short { .. } => return Err(Error::Response),
+            Response::Long(value) => {
+                tracing::trace!("CMD2 response: {value:#x}");
+                // TODO: map [u32; 4] value to u128
+                Ok(CardIdentification(0))
+            }
+        }
+    }
+
+    /// Get the relative card address
+    pub async fn get_rca(&mut self) -> Result<u32, Error> {
+        match self
+            .cmd(Command {
+                index: 3,
+                argument: 0,
+                options: HardwareOptions::None,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?
+        {
+            Response::Short { value, .. } => {
+                tracing::trace!("CMD3 response: {value:#x}");
+                Ok(value)
+            }
+            Response::Long(_) => return Err(Error::Response),
+        }
+    }
+
+    /// Toggle the card between stand-by and transfer state
+    pub async fn select(&mut self, rca: u32) -> Result<CardStatus, Error> {
+        match self
+            .cmd(Command {
+                index: 7,
+                argument: rca,
+                options: HardwareOptions::None,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?
+        {
+            Response::Short { value, .. } => {
+                tracing::trace!("CMD7 response: {value:#x}");
+                Ok(CardStatus(value))
+            }
+            Response::Long(_) => return Err(Error::Response),
+        }
+    }
+
+    /// Use 4 data lanes
+    pub async fn set_wide_bus(&mut self, rca: u32) -> Result<CardStatus, Error> {
+        // Go to *APP* mode before sending application command
+        self.cmd(Command {
+            index: 55,
+            argument: rca,
+            options: HardwareOptions::None,
+            kind: CommandKind::Control,
+            rsp_type: ResponseType::Short,
+            rsp_crc: true,
+            buffer: None,
+        })
+        .await?;
+
+        match self
+            .cmd(Command {
+                index: 6,
+                argument: 0b10,
+                options: HardwareOptions::SetBusWidth(BusWidth::Quad),
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?
+        {
+            Response::Short { value, .. } => {
+                tracing::trace!("ACMD6 response: {value:#x}");
+                Ok(CardStatus(value))
+            }
+            Response::Long(_) => Err(Error::Response),
+        }
+    }
 }
 
 /// A client for SDIO cards using the [`SdmmcService`].

From befaf608527e631933fb2cc6301ac2be2e65d28a Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Thu, 26 Oct 2023 22:09:44 +0200
Subject: [PATCH 16/38] sdmmc: implement reading from SD card for allwinner-d1

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 360 +++++++++++++++---
 platforms/allwinner-d1/src/lib.rs             |  20 +-
 source/kernel/src/services/sdmmc.rs           |  43 ++-
 3 files changed, 355 insertions(+), 68 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index e9f0dc21..912b0857 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -16,7 +16,6 @@ use core::{
 use crate::ccu::{BusGatingResetRegister, Ccu};
 use d1_pac::{smhc, Interrupt, GPIO, SMHC0, SMHC1, SMHC2};
 use kernel::{
-    comms::kchannel::{KChannel, KConsumer},
     mnemos_alloc::containers::FixedVec,
     registry,
     services::sdmmc::{self, SdmmcService},
@@ -62,12 +61,12 @@ enum SmhcOp {
     Control,
     Read {
         buf: FixedVec<u8>,
-        cnt: u32,
+        cnt: usize,
         auto_stop: bool,
     },
     Write {
         buf: FixedVec<u8>,
-        cnt: u32,
+        cnt: usize,
         auto_stop: bool,
     },
 }
@@ -79,6 +78,8 @@ enum State {
     Idle,
     /// Waiting for command to be completed.
     WaitForCommand,
+    /// Waiting for the DMA operation to be completed.
+    WaitForDma,
     /// Waiting for data transfer to be completed.
     WaitForDataTransfer,
     /// Waiting for the auto-stop command to be sent and completed.
@@ -167,6 +168,13 @@ impl Smhc {
             w.re_int_en().set_bit();
             w
         });
+        smhc.smhc_idie.write(|w| {
+            w.des_unavl_int_enb().set_bit();
+            w.ferr_int_enb().set_bit();
+            w.rx_int_enb().set_bit();
+            w.tx_int_enb().set_bit();
+            w
+        });
 
         Self::init(
             unsafe { &*SMHC0::ptr() },
@@ -216,7 +224,8 @@ impl Smhc {
         // Set the sample delay to 0 (as done in Linux and Allwinner BSP)
         smhc.smhc_smap_dl.write(|w| {
             w.samp_dl_sw().variant(0);
-            w.samp_dl_sw_en().set_bit()
+            w.samp_dl_sw_en().set_bit();
+            w
         });
 
         // Enable the card clock
@@ -236,6 +245,60 @@ impl Smhc {
         }
     }
 
+    /// # Safety
+    /// The descriptor chain needs to live at least as long as the DMA transfer.
+    /// Additionally, their content (e.g., the validity of the buffers they point to)
+    /// also needs to be verified by the user.
+    unsafe fn prepare_dma(&self, descriptor: &idmac::Descriptor, byte_cnt: u32) {
+        self.smhc.smhc_ctrl.modify(|_, w| {
+            w.dma_enb().set_bit();
+            w.dma_rst().set_bit();
+            w
+        });
+        while self.smhc.smhc_ctrl.read().dma_rst().bit_is_set() {}
+
+        // Configure the address of the first DMA descriptor
+        // Right-shift by 2 because it is a *word-address*.
+        self.smhc
+            .smhc_dlba
+            .write(|w| unsafe { w.bits((descriptor as *const _ as u32) >> 2) });
+
+        // Set number of bytes that will be read or written.
+        self.smhc.smhc_bytcnt.write(|w| unsafe { w.bits(byte_cnt) });
+
+        // Soft reset of DMA controller
+        self.smhc.smhc_idmac.write(|w| w.idmac_rst().set_bit());
+
+        // Configure the burst size and TX/RX trigger level
+        self.smhc.smhc_fifoth.write(|w| {
+            w.tx_tl().variant(8);
+            w.rx_tl().variant(7);
+            w.bsize_of_trans().t8();
+            w
+        });
+
+        // Configure the transfer interrupt, receive interrupt, and abnormal interrupt.
+        self.smhc.smhc_idie.write(|w| {
+            w.rx_int_enb().set_bit();
+            w.tx_int_enb().set_bit();
+            w.err_sum_int_enb().set_bit();
+            w
+        });
+
+        // Enable the IDMAC and configure burst transfers
+        self.smhc.smhc_idmac.write(|w| {
+            w.idmac_enb().set_bit();
+            w.fix_bust_ctrl().set_bit();
+            w
+        });
+
+        // Reset the FIFO
+        self.smhc.smhc_ctrl.modify(|_, w| w.fifo_rst().reset());
+        while self.smhc.smhc_ctrl.read().fifo_rst().is_reset() {
+            core::hint::spin_loop();
+        }
+    }
+
     pub fn handle_smhc0_interrupt() {
         let _isr = kernel::isr::Isr::enter();
         let smhc = unsafe { &*SMHC0::ptr() };
@@ -308,6 +371,75 @@ impl Smhc {
             }
         }
 
+        let mut guard = self.isr.lock(self.smhc);
+        guard.data.op = match (params.kind, params.buffer) {
+            (sdmmc::CommandKind::Control, _) => SmhcOp::Control,
+            (sdmmc::CommandKind::Read(cnt), Some(buf)) => SmhcOp::Read {
+                buf,
+                cnt,
+                auto_stop,
+            },
+            (sdmmc::CommandKind::Write(cnt), Some(buf)) => SmhcOp::Write {
+                buf,
+                cnt,
+                auto_stop,
+            },
+            _ => {
+                tracing::error!("did not provide a buffer for read/write");
+                return Err(sdmmc::Error::Other);
+            }
+        };
+
+        // Statically allocate space for 16 DMA descriptors.
+        // Each descriptor can do a transfer of 4KB, giving a max total transfer of 64KB.
+        // By declaring them in this scope the descriptor memory will live long enough,
+        // however currently this is up to the user to guarantuee.
+        // Safety: a zero'ed descriptor is valid and will simply be ignored by the IDMAC.
+        let mut descriptors: [idmac::Descriptor; 16] = unsafe { core::mem::zeroed() };
+
+        // Perform checks on arguments to make sure we won't overflow the buffer
+        match &guard.data.op {
+            SmhcOp::Read { buf, cnt, .. } | SmhcOp::Write { buf, cnt, .. } => {
+                // Currently we limit the number of data that can be read at once
+                if *cnt > 0x10000 || buf.capacity() < *cnt {
+                    return Err(sdmmc::Error::Data);
+                }
+
+                tracing::debug!("Creating descriptor chain from buffer");
+                let buf_ptr = buf.as_slice().as_ptr();
+                let mut remaining = *cnt;
+                let mut index = 0;
+                while remaining > 0x1000 {
+                    descriptors[index] = idmac::Descriptor::try_from(idmac::DescriptorConfig {
+                        disable_int_on_complete: true,
+                        first: index == 0,
+                        last: false,
+                        buff_size: 0x1000,
+                        buff_addr: unsafe { buf_ptr.add(index * 0x1000) },
+                        next_desc: Some(&descriptors[index + 1] as *const _ as _),
+                    })
+                    .map_err(|_| sdmmc::Error::Other)?;
+                    remaining -= 0x1000;
+                    index += 1;
+                }
+
+                descriptors[index] = idmac::Descriptor::try_from(idmac::DescriptorConfig {
+                    disable_int_on_complete: false,
+                    first: index == 0,
+                    last: true,
+                    buff_size: remaining as u16,
+                    buff_addr: unsafe { buf_ptr.add(index * 0x1000) },
+                    next_desc: None,
+                })
+                .map_err(|_| sdmmc::Error::Other)?;
+
+                // TODO: should this return some kind of guard, which needs to be used later
+                // to make sure the descriptor has a long enough lifetime?
+                unsafe { self.prepare_dma(&descriptors[0], *cnt as u32) };
+            }
+            _ => (),
+        }
+
         // Set the argument to be sent
         self.smhc
             .smhc_cmdarg
@@ -327,27 +459,8 @@ impl Smhc {
             w
         });
 
-        // Now wait for completion or error interrupt
-        let mut guard = self.isr.lock(self.smhc);
         guard.data.state = State::WaitForCommand;
-        guard.data.op = match (params.kind, params.buffer) {
-            (sdmmc::CommandKind::Control, _) => SmhcOp::Control,
-            (sdmmc::CommandKind::Read(cnt), Some(buf)) => SmhcOp::Read {
-                buf,
-                cnt,
-                auto_stop,
-            },
-            (sdmmc::CommandKind::Write(cnt), Some(buf)) => SmhcOp::Write {
-                buf,
-                cnt,
-                auto_stop,
-            },
-            _ => {
-                tracing::warn!("did not provide a buffer for read/write");
-                return Err(sdmmc::Error::Other);
-            }
-        };
-
+        // Now wait for completion or error interrupt
         guard.wait_for_irq().await;
         tracing::trace!("SMHC operation completed");
         let res = if let Some(error) = guard.data.err.take() {
@@ -425,70 +538,103 @@ impl DerefMut for SmhcDataGuard<'_> {
 }
 
 impl SmhcData {
+    /// Check for error bits in the interrupt status registers.
     fn error_status(smhc: &smhc::RegisterBlock) -> Option<ErrorKind> {
         let mint = smhc.smhc_mintsts.read();
-        if mint.m_rto_back_int().bit_is_set() {
-            return Some(ErrorKind::ResponseTimeout);
-        }
-        if mint.m_rce_int().bit_is_set() {
-            return Some(ErrorKind::ResponseCrc);
-        }
-        if mint.m_re_int().bit_is_set() {
-            return Some(ErrorKind::Response);
-        }
+        let idst = smhc.smhc_idst.read();
+
         if mint.m_dsto_vsd_int().bit_is_set() {
-            return Some(ErrorKind::DataStarvationTimeout);
-        }
-        if mint.m_dto_bds_int().bit_is_set() {
-            return Some(ErrorKind::DataTimeout);
-        }
-        if mint.m_dce_int().bit_is_set() {
-            return Some(ErrorKind::DataCrc);
-        }
-        if mint.m_fu_fo_int().bit_is_set() {
-            return Some(ErrorKind::FifoUnderrunOverflow);
-        }
-        if mint.m_cb_iw_int().bit_is_set() {
-            return Some(ErrorKind::CommandBusyIllegalWrite);
-        }
-        if mint.m_dse_bc_int().bit_is_set() {
-            return Some(ErrorKind::DataStart);
-        }
-        if mint.m_dee_int().bit_is_set() {
-            return Some(ErrorKind::DataEnd);
+            Some(ErrorKind::DataStarvationTimeout)
+        } else if mint.m_fu_fo_int().bit_is_set() {
+            Some(ErrorKind::FifoUnderrunOverflow)
+        } else if mint.m_cb_iw_int().bit_is_set() {
+            Some(ErrorKind::CommandBusyIllegalWrite)
+        } else if mint.m_rto_back_int().bit_is_set() {
+            Some(ErrorKind::ResponseTimeout)
+        } else if mint.m_rce_int().bit_is_set() {
+            Some(ErrorKind::ResponseCrc)
+        } else if mint.m_re_int().bit_is_set() {
+            Some(ErrorKind::Response)
+        } else if mint.m_dto_bds_int().bit_is_set() {
+            Some(ErrorKind::DataTimeout)
+        } else if mint.m_dce_int().bit_is_set() {
+            Some(ErrorKind::DataCrc)
+        } else if mint.m_dse_bc_int().bit_is_set() {
+            Some(ErrorKind::DataStart)
+        } else if mint.m_dee_int().bit_is_set() {
+            Some(ErrorKind::DataEnd)
+        } else if idst.des_unavl_int().bit_is_set() || idst.fatal_berr_int().bit_is_set() {
+            Some(ErrorKind::Dma)
+        } else {
+            None
         }
-
-        None
     }
 
     fn advance_isr(&mut self, smhc: &smhc::RegisterBlock, num: u8) {
         tracing::trace!(state = ?self.state, smhc = num, "SMHC{num} interrupt");
 
+        let mut needs_wake = false;
+
         self.err = Self::error_status(smhc);
-        let mut needs_wake = self.err.is_some();
+        if let Some(err) = self.err {
+            // Clear all interrupt bits
+            smhc.smhc_rintsts.write(|w| unsafe { w.bits(0xFFFF_FFFF) });
+            smhc.smhc_idst.write(|w| unsafe { w.bits(0x3FF) });
+
+            needs_wake = true;
+        }
 
         self.state = match self.state {
             State::Idle => State::Idle,
             State::WaitForCommand => {
                 if smhc.smhc_mintsts.read().m_cc_int().bit_is_set() {
+                    smhc.smhc_rintsts.write(|w| w.cc().set_bit());
                     match self.op {
                         SmhcOp::None => State::Idle,
                         SmhcOp::Control => {
                             needs_wake = true;
                             State::Idle
                         }
-                        SmhcOp::Read { .. } | SmhcOp::Write { .. } => State::WaitForDataTransfer,
+                        SmhcOp::Read { .. } | SmhcOp::Write { .. } => State::WaitForDma,
                     }
                 } else {
                     self.state
                 }
             }
+            State::WaitForDma => {
+                // TODO: better way to check for RX and TX,
+                // *normal interrupt summary* does not seem to work
+                if smhc.smhc_idst.read().bits() != 0 {
+                    smhc.smhc_idst.write(|w| {
+                        w.nor_int_sum().set_bit();
+                        w.rx_int().set_bit();
+                        w.tx_int().set_bit();
+                        w
+                    });
+                    State::WaitForDataTransfer
+                } else {
+                    self.state
+                }
+            }
             State::WaitForDataTransfer => {
                 if smhc.smhc_mintsts.read().m_dtc_int().bit_is_set() {
-                    match self.op {
+                    smhc.smhc_rintsts.write(|w| w.dtc().set_bit());
+                    match &mut self.op {
                         SmhcOp::None | SmhcOp::Control => State::Idle,
-                        SmhcOp::Read { auto_stop, .. } | SmhcOp::Write { auto_stop, .. } => {
-                            if auto_stop {
+                        SmhcOp::Read {
+                            auto_stop,
+                            buf,
+                            cnt,
+                        }
+                        | SmhcOp::Write {
+                            auto_stop,
+                            buf,
+                            cnt,
+                        } => {
+                            tracing::trace!("setting buf len: {cnt}");
+                            // TODO: Safety?
+                            unsafe { buf.as_vec_mut().set_len(*cnt) };
+                            if *auto_stop {
                                 State::WaitForAutoStop
                             } else {
                                 needs_wake = true;
@@ -502,6 +648,7 @@ impl SmhcData {
             }
             State::WaitForAutoStop => {
                 if smhc.smhc_mintsts.read().m_acd_int().bit_is_set() {
+                    smhc.smhc_rintsts.write(|w| w.acd().set_bit());
                     needs_wake = true;
                     State::Idle
                 } else {
@@ -517,8 +664,99 @@ impl SmhcData {
                 smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
             }
         }
+    }
+}
+
+/// Internal DMA controller
+mod idmac {
+    #[repr(C, align(4))]
+    #[derive(Default, Debug)]
+    pub struct Descriptor {
+        des0: u32,
+        des1: u32,
+        des2: u32,
+        des3: u32,
+    }
+
+    impl Descriptor {
+        /// Indicates whether the descriptor is currently owned by the IDMAC.
+        pub fn is_owned(&self) -> bool {
+            self.des0 & (1 << 31) != 0
+        }
+
+        /// Indicates whether an error happened during transfer.
+        pub fn is_err(&self) -> bool {
+            self.des0 & (1 << 30) != 0
+        }
+    }
+
+    pub struct DescriptorConfig {
+        /// When true, this will prevent the setting of the TX/RX interrupt
+        /// bit of the IDMAC status register for data that ends in the buffer the
+        /// descriptor points to.
+        pub disable_int_on_complete: bool,
+        /// If this is the first descriptor.
+        pub first: bool,
+        /// If this is the last descriptor.
+        pub last: bool,
+        /// Buffer data size in bytes, must be a multiple of 4.
+        pub buff_size: u16,
+        /// The physical address of the data buffer.
+        pub buff_addr: *const u8,
+        /// The physical address of the next descriptor.
+        pub next_desc: Option<*const ()>,
+    }
+
+    #[derive(Debug)]
+    pub enum Error {
+        InvalidBufferAddr,
+        InvalidBufferSize,
+    }
+
+    impl TryFrom<DescriptorConfig> for Descriptor {
+        type Error = Error;
 
-        // Clear all pending interrupts
-        smhc.smhc_rintsts.write(|w| unsafe { w.bits(0xFFFF_FFFF) });
+        fn try_from(value: DescriptorConfig) -> Result<Self, Self::Error> {
+            let mut descriptor = Descriptor {
+                des0: 0,
+                des1: 0,
+                des2: 0,
+                des3: 0,
+            };
+
+            if value.disable_int_on_complete {
+                descriptor.des0 |= 1 << 1;
+            }
+            if value.last {
+                descriptor.des0 |= 1 << 2;
+            }
+            if value.first {
+                descriptor.des0 |= 1 << 3;
+            }
+
+            // These always need to be set
+            descriptor.des0 |= 1 << 4;
+            descriptor.des0 |= 1 << 31;
+
+            if value.buff_size < (1 << 13) && (value.buff_size & 0b11) == 0 {
+                descriptor.des1 = value.buff_size as u32;
+            } else {
+                return Err(Error::InvalidBufferSize);
+            }
+
+            if !value.buff_addr.is_null() {
+                // Right-shift by 2 because it is a *word-address*.
+                descriptor.des2 = (value.buff_addr as u32) >> 2;
+            } else {
+                return Err(Error::InvalidBufferAddr);
+            }
+
+            if let Some(next) = value.next_desc {
+                // Right-shift by 2 because it is a *word-address*.
+                descriptor.des3 = (next as u32) >> 2;
+            }
+
+            Ok(descriptor)
+        }
     }
 }
diff --git a/platforms/allwinner-d1/src/lib.rs b/platforms/allwinner-d1/src/lib.rs
index 173462c4..a01563d8 100644
--- a/platforms/allwinner-d1/src/lib.rs
+++ b/platforms/allwinner-d1/src/lib.rs
@@ -24,7 +24,7 @@ use core::{
 };
 use d1_pac::{Interrupt, TIMER};
 use kernel::{
-    mnemos_alloc::containers::Box,
+    mnemos_alloc::containers::{Box, FixedVec},
     services::sdmmc::SdCardClient,
     tracing::{self, Instrument},
     Kernel, KernelServiceSettings, KernelSettings,
@@ -175,6 +175,24 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
                 .await
                 .expect("can't initialize the SD card");
             tracing::debug!("initialized card of type {card_type:?}");
+
+            sdcard.get_cid().await.expect("can't get CID");
+            let rca = sdcard.get_rca().await.expect("can't get RCA");
+            let card_status = sdcard.select(rca).await.expect("can't select card");
+            tracing::debug!("card status: {card_status:?}");
+            sdcard
+                .set_wide_bus(rca)
+                .await
+                .expect("can't set wide bus mode");
+
+            let buffer = FixedVec::new(512).await;
+            match sdcard.read(80, 1, buffer).await {
+                Ok(buffer) => {
+                    let tmp = &buffer.as_slice()[0..16];
+                    tracing::info!("Read data from SD card: {tmp:?}")
+                }
+                Err(_) => tracing::error!("Failed to read SD card"),
+            }
         })
         .expect("failed to spawn sdcard client");
 
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 760a591b..04a73ba5 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -93,9 +93,9 @@ pub enum CommandKind {
     /// Command without data transfer
     Control,
     /// Command for reading data, contains the number of bytes to read
-    Read(u32),
+    Read(usize),
     /// Command for writing data, contains the number of bytes to write
-    Write(u32),
+    Write(usize),
 }
 
 /// TODO
@@ -173,6 +173,7 @@ pub enum CardType {
 }
 
 /// Card status in R1 response format
+#[derive(Debug, PartialEq)]
 pub struct CardStatus(u32);
 
 /// Card identification register in R2 response format
@@ -231,12 +232,12 @@ impl SdCardClient {
             .and_then(|resp| resp.body)
     }
 
-    /// TODO
+    /// Reset the card
     pub async fn reset(&mut self) -> Result<(), Error> {
         self.cmd(Command::default()).await.map(|_| ())
     }
 
-    /// TODO
+    /// Initialize the card
     pub async fn initialize(&mut self) -> Result<CardType, Error> {
         /// Request switch to 1.8V
         #[allow(dead_code)]
@@ -311,7 +312,7 @@ impl SdCardClient {
                 Response::Long(_) => return Err(Error::Response),
             }
 
-            // TODO: wait 1ms
+            // TODO: wait some time (e.g., 1ms?)
         };
 
         if (ocr & OCR_HCS) == OCR_HCS {
@@ -337,7 +338,7 @@ impl SdCardClient {
         {
             Response::Short { .. } => return Err(Error::Response),
             Response::Long(value) => {
-                tracing::trace!("CMD2 response: {value:#x}");
+                tracing::trace!("CMD2 response: {value:?}");
                 // TODO: map [u32; 4] value to u128
                 Ok(CardIdentification(0))
             }
@@ -421,6 +422,36 @@ impl SdCardClient {
             Response::Long(_) => Err(Error::Response),
         }
     }
+
+    /// Read the desired number of data blocks into the provided buffer, starting at the given sector.
+    pub async fn read(
+        &mut self,
+        sector: u32,
+        blocks: usize,
+        buf: FixedVec<u8>,
+    ) -> Result<FixedVec<u8>, Error> {
+        match self
+            .cmd(Command {
+                index: 18,
+                argument: sector,
+                options: HardwareOptions::None,
+                kind: CommandKind::Read(512 * blocks),
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: Some(buf),
+            })
+            .await?
+        {
+            Response::Short {
+                value,
+                data: Some(res),
+            } => {
+                tracing::trace!("CMD18 response: {value:#x}");
+                Ok(res)
+            }
+            _ => Err(Error::Response),
+        }
+    }
 }
 
 /// A client for SDIO cards using the [`SdmmcService`].

From abaf683e170e4019b602c1126cbf310a1a72dc86 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Thu, 2 Nov 2023 22:12:19 +0100
Subject: [PATCH 17/38] Apply some PR feedback

---
 platforms/allwinner-d1/d1-core/src/ccu.rs     | 44 ++++++++---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 78 +++++++++----------
 platforms/allwinner-d1/src/lib.rs             |  6 +-
 source/kernel/src/services/sdmmc.rs           | 49 +++++++-----
 4 files changed, 102 insertions(+), 75 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/ccu.rs b/platforms/allwinner-d1/d1-core/src/ccu.rs
index a6a05f5b..85f3450d 100644
--- a/platforms/allwinner-d1/d1-core/src/ccu.rs
+++ b/platforms/allwinner-d1/d1-core/src/ccu.rs
@@ -18,12 +18,24 @@ pub struct Ccu {
     ccu: CCU,
 }
 
+#[derive(PartialEq)]
+pub enum BusGating {
+    Mask,
+    Pass,
+}
+
+#[derive(PartialEq)]
+pub enum BusReset {
+    Assert,
+    Deassert,
+}
+
 /// Trait to be implemented for module clocks that can be gated and reset
 pub trait BusGatingResetRegister {
     /// Enable or disable the clock gating bit
-    fn gating(ccu: &mut CCU, pass: bool);
+    fn gating(ccu: &mut CCU, gating: BusGating);
     /// Enable or disable the clock reset bit
-    fn reset(ccu: &mut CCU, assert: bool);
+    fn reset(ccu: &mut CCU, reset: BusReset);
 }
 
 // TODO: should this move into the `Clint`?
@@ -49,16 +61,16 @@ impl Ccu {
 
     /// De-assert the reset bit and enable the clock gating bit for the given module
     pub fn enable_module<MODULE: BusGatingResetRegister>(&mut self, _mod: &mut MODULE) {
-        MODULE::reset(&mut self.ccu, false);
+        MODULE::reset(&mut self.ccu, BusReset::Deassert);
         sdelay(20);
-        MODULE::gating(&mut self.ccu, true);
+        MODULE::gating(&mut self.ccu, BusGating::Pass);
     }
 
     /// Disable the clock gating bit and assert the reset bit for the given module
     pub fn disable_module<MODULE: BusGatingResetRegister>(&mut self, _mod: &mut MODULE) {
-        MODULE::gating(&mut self.ccu, false);
+        MODULE::gating(&mut self.ccu, BusGating::Mask);
         // TODO: delay?
-        MODULE::reset(&mut self.ccu, true);
+        MODULE::reset(&mut self.ccu, BusReset::Assert);
     }
 
     /// Allow modules to configure their own clock on a PAC level
@@ -248,12 +260,24 @@ macro_rules! impl_bgr {
     ($($MODULE:ident : ($reg:ident, $gating:ident, $reset:ident),)+) => {
         $(
             impl BusGatingResetRegister for $MODULE {
-                fn gating(ccu: &mut CCU, pass: bool) {
-                    ccu.$reg.modify(|_, w| w.$gating().bit(pass));
+                fn gating(ccu: &mut CCU, gating: BusGating) {
+                    ccu.$reg.modify(|_, w| {
+                        if gating == BusGating::Mask {
+                            w.$gating().clear_bit()
+                        } else {
+                            w.$gating().set_bit()
+                        }
+                    });
                 }
 
-                fn reset(ccu: &mut CCU, assert: bool) {
-                    ccu.$reg.modify(|_, w| w.$reset().bit(!assert));
+                fn reset(ccu: &mut CCU, reset: BusReset) {
+                    ccu.$reg.modify(|_, w| {
+                        if reset == BusReset::Assert {
+                            w.$reset().clear_bit()
+                        } else {
+                            w.$reset().set_bit()
+                        }
+                    });
                 }
             }
         )+
diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 912b0857..953e401c 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -124,7 +124,7 @@ impl Smhc {
     ///
     /// # Safety
     /// TODO
-    pub unsafe fn new(mut smhc: SMHC0, ccu: &mut Ccu, gpio: &mut GPIO) -> Self {
+    pub unsafe fn smhc0(mut smhc: SMHC0, ccu: &mut Ccu, gpio: &mut GPIO) -> Self {
         // Configure default pin mapping for TF (micro SD) card socket.
         // This is valid for the Lichee RV SOM and Mango Pi MQ Pro.
         gpio.pf_cfg0.modify(|_, w| {
@@ -236,7 +236,7 @@ impl Smhc {
         // Default bus width after power up or idle is 1-bit
         smhc.smhc_ctype.write(|w| w.card_wid().b1());
         // Blocksize is fixed to 512 bytes
-        smhc.smhc_blksiz.write(|w| unsafe { w.bits(0x200) });
+        smhc.smhc_blksiz.write(|w| unsafe { w.bits(512) });
 
         Self {
             smhc,
@@ -348,12 +348,12 @@ impl Smhc {
         }
 
         let cmd_idx = params.index;
-        // TODO: naive `auto_stop` selection, probably only works in case of SD memory cards.
+        // TODO: naive `auto_stop` selection, this depends on the command (argument) that is used.
         // Should this (auto_stop select) be part of the params?
         let (data_trans, trans_dir, auto_stop) = match params.kind {
             sdmmc::CommandKind::Control => (false, false, false),
-            sdmmc::CommandKind::Read(len) => (true, false, len > 512),
-            sdmmc::CommandKind::Write(len) => (true, true, len > 512),
+            sdmmc::CommandKind::Read(len) => (true, false, true),
+            sdmmc::CommandKind::Write(len) => (true, true, true),
         };
         let chk_resp_crc = params.rsp_crc;
         let long_resp = params.rsp_type == sdmmc::ResponseType::Long;
@@ -400,8 +400,10 @@ impl Smhc {
         // Perform checks on arguments to make sure we won't overflow the buffer
         match &guard.data.op {
             SmhcOp::Read { buf, cnt, .. } | SmhcOp::Write { buf, cnt, .. } => {
+                const DESCR_BUFF_SIZE: usize = 0x1000;
+
                 // Currently we limit the number of data that can be read at once
-                if *cnt > 0x10000 || buf.capacity() < *cnt {
+                if *cnt > DESCR_BUFF_SIZE * descriptors.len() || buf.capacity() < *cnt {
                     return Err(sdmmc::Error::Data);
                 }
 
@@ -409,30 +411,23 @@ impl Smhc {
                 let buf_ptr = buf.as_slice().as_ptr();
                 let mut remaining = *cnt;
                 let mut index = 0;
-                while remaining > 0x1000 {
+                while remaining > 0 {
+                    let buff_size = core::cmp::min(DESCR_BUFF_SIZE, remaining) as u16;
+                    remaining = remaining.saturating_sub(DESCR_BUFF_SIZE);
+                    let first = index == 0;
+                    let last = remaining == 0;
                     descriptors[index] = idmac::Descriptor::try_from(idmac::DescriptorConfig {
-                        disable_int_on_complete: true,
-                        first: index == 0,
-                        last: false,
-                        buff_size: 0x1000,
-                        buff_addr: unsafe { buf_ptr.add(index * 0x1000) },
-                        next_desc: Some(&descriptors[index + 1] as *const _ as _),
+                        disable_int_on_complete: !last,
+                        first,
+                        last,
+                        buff_size,
+                        buff_addr: unsafe { buf_ptr.add(index * DESCR_BUFF_SIZE) },
+                        next_desc: (!last).then_some(&descriptors[index + 1] as *const _ as _),
                     })
                     .map_err(|_| sdmmc::Error::Other)?;
-                    remaining -= 0x1000;
                     index += 1;
                 }
 
-                descriptors[index] = idmac::Descriptor::try_from(idmac::DescriptorConfig {
-                    disable_int_on_complete: false,
-                    first: index == 0,
-                    last: true,
-                    buff_size: remaining as u16,
-                    buff_addr: unsafe { buf_ptr.add(index * 0x1000) },
-                    next_desc: None,
-                })
-                .map_err(|_| sdmmc::Error::Other)?;
-
                 // TODO: should this return some kind of guard, which needs to be used later
                 // to make sure the descriptor has a long enough lifetime?
                 unsafe { self.prepare_dma(&descriptors[0], *cnt as u32) };
@@ -463,28 +458,25 @@ impl Smhc {
         // Now wait for completion or error interrupt
         guard.wait_for_irq().await;
         tracing::trace!("SMHC operation completed");
-        let res = if let Some(error) = guard.data.err.take() {
+        if let Some(error) = guard.data.err.take() {
             tracing::warn!(?error, "SMHC error");
             Err(sdmmc::Error::Other) // TODO
+        } else if long_resp {
+            Ok(sdmmc::Response::Long([
+                self.smhc.smhc_resp0.read().bits(),
+                self.smhc.smhc_resp1.read().bits(),
+                self.smhc.smhc_resp2.read().bits(),
+                self.smhc.smhc_resp3.read().bits(),
+            ]))
         } else {
-            if long_resp {
-                Ok(sdmmc::Response::Long([
-                    self.smhc.smhc_resp0.read().bits(),
-                    self.smhc.smhc_resp1.read().bits(),
-                    self.smhc.smhc_resp2.read().bits(),
-                    self.smhc.smhc_resp3.read().bits(),
-                ]))
-            } else {
-                Ok(sdmmc::Response::Short {
-                    value: self.smhc.smhc_resp0.read().bits(),
-                    data: match core::mem::replace(&mut guard.data.op, SmhcOp::None) {
-                        SmhcOp::Read { buf, .. } => Some(buf),
-                        _ => None,
-                    },
-                })
-            }
-        };
-        res
+            Ok(sdmmc::Response::Short {
+                value: self.smhc.smhc_resp0.read().bits(),
+                data: match core::mem::replace(&mut guard.data.op, SmhcOp::None) {
+                    SmhcOp::Read { buf, .. } => Some(buf),
+                    _ => None,
+                },
+            })
+        }
     }
 }
 
diff --git a/platforms/allwinner-d1/src/lib.rs b/platforms/allwinner-d1/src/lib.rs
index a01563d8..f7ab95c9 100644
--- a/platforms/allwinner-d1/src/lib.rs
+++ b/platforms/allwinner-d1/src/lib.rs
@@ -52,7 +52,7 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
 
     let uart = unsafe { uart::kernel_uart(&mut ccu, &mut p.GPIO, p.UART0) };
     let spim = unsafe { spim::kernel_spim1(p.SPI_DBI, &mut ccu, &mut p.GPIO) };
-    let smhc0 = unsafe { Smhc::new(p.SMHC0, &mut ccu, &mut p.GPIO) };
+    let smhc0 = unsafe { Smhc::smhc0(p.SMHC0, &mut ccu, &mut p.GPIO) };
 
     let i2c0 = match config.platform.i2c {
         d1_config::I2cConfiguration { enabled: false, .. } => None,
@@ -185,8 +185,8 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
                 .await
                 .expect("can't set wide bus mode");
 
-            let buffer = FixedVec::new(512).await;
-            match sdcard.read(80, 1, buffer).await {
+            let buffer = FixedVec::new(16 * 512).await;
+            match sdcard.read(2048, 16, buffer).await {
                 Ok(buffer) => {
                     let tmp = &buffer.as_slice()[0..16];
                     tracing::info!("Read data from SD card: {tmp:?}")
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 04a73ba5..7eca71e0 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -7,10 +7,7 @@
 //! (how to send and receive the data).
 #![warn(missing_docs)]
 use crate::{
-    comms::{
-        kchannel::{KChannel, KConsumer, KProducer},
-        oneshot::{self, Reusable},
-    },
+    comms::oneshot::Reusable,
     mnemos_alloc::containers::FixedVec,
     registry::{self, known_uuids, Envelope, KernelHandle, RegisteredDriver},
     Kernel,
@@ -177,17 +174,24 @@ pub enum CardType {
 pub struct CardStatus(u32);
 
 /// Card identification register in R2 response format
-///   Manufacturer ID       [127:120]
-///   OEM/Application ID    [119:104]
-///   Product name          [103:64]
-///   Product revision      [63:56]
-///   Product serial number [55:24]
-///   Reserved              [23:20]
-///   Manufacturing date    [19:8]
-///   CRC7 checksum         [7:1]
-///   Not used, always 1    [0:0]
+///
+/// | Field                 | Bits        |
+/// |:----------------------|------------:|
+/// | Manufacturer ID       | `[127:120]` |
+/// | OEM/Application ID    | `[119:104]` |
+/// | Product name          | `[103:64]`  |
+/// | Product revision      | `[63:56]`   |
+/// | Product serial number | `[55:24]`   |
+/// | Reserved              | `[23:20]`   |
+/// | Manufacturing date    | `[19:8]`    |
+/// | CRC7 checksum         | `[7:1]`     |
+/// | Not used, always 1    | `[0:0]`     |
 pub struct CardIdentification(u128);
 
+/// Published RCA in R6 response format
+#[derive(Debug, PartialEq, Clone, Copy)]
+pub struct RelativeCardAddress(u32);
+
 impl SdCardClient {
     /// Obtain an `SdCardClient`
     ///
@@ -346,7 +350,7 @@ impl SdCardClient {
     }
 
     /// Get the relative card address
-    pub async fn get_rca(&mut self) -> Result<u32, Error> {
+    pub async fn get_rca(&mut self) -> Result<RelativeCardAddress, Error> {
         match self
             .cmd(Command {
                 index: 3,
@@ -361,18 +365,18 @@ impl SdCardClient {
         {
             Response::Short { value, .. } => {
                 tracing::trace!("CMD3 response: {value:#x}");
-                Ok(value)
+                Ok(RelativeCardAddress(value))
             }
             Response::Long(_) => return Err(Error::Response),
         }
     }
 
     /// Toggle the card between stand-by and transfer state
-    pub async fn select(&mut self, rca: u32) -> Result<CardStatus, Error> {
+    pub async fn select(&mut self, rca: RelativeCardAddress) -> Result<CardStatus, Error> {
         match self
             .cmd(Command {
                 index: 7,
-                argument: rca,
+                argument: rca.0,
                 options: HardwareOptions::None,
                 kind: CommandKind::Control,
                 rsp_type: ResponseType::Short,
@@ -390,11 +394,11 @@ impl SdCardClient {
     }
 
     /// Use 4 data lanes
-    pub async fn set_wide_bus(&mut self, rca: u32) -> Result<CardStatus, Error> {
+    pub async fn set_wide_bus(&mut self, rca: RelativeCardAddress) -> Result<CardStatus, Error> {
         // Go to *APP* mode before sending application command
         self.cmd(Command {
             index: 55,
-            argument: rca,
+            argument: rca.0,
             options: HardwareOptions::None,
             kind: CommandKind::Control,
             rsp_type: ResponseType::Short,
@@ -430,6 +434,11 @@ impl SdCardClient {
         blocks: usize,
         buf: FixedVec<u8>,
     ) -> Result<FixedVec<u8>, Error> {
+        // The provider buffer should have space for the requested amount of data
+        if buf.capacity() < 512 * blocks {
+            return Err(Error::Data);
+        }
+
         match self
             .cmd(Command {
                 index: 18,
@@ -455,12 +464,14 @@ impl SdCardClient {
 }
 
 /// A client for SDIO cards using the [`SdmmcService`].
+#[allow(dead_code)]
 pub struct SdioClient {
     handle: KernelHandle<SdmmcService>,
     reply: Reusable<Envelope<Result<Response, Error>>>,
 }
 
 /// A client for MMC cards using the [`SdmmcService`].
+#[allow(dead_code)]
 pub struct MmcClient {
     handle: KernelHandle<SdmmcService>,
     reply: Reusable<Envelope<Result<Response, Error>>>,

From 3722c51a48374b9731fe86dd364d1f519e90db12 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Thu, 2 Nov 2023 22:43:35 +0100
Subject: [PATCH 18/38] smhc: reduce module clock rate to 100 MHz

---
 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 953e401c..6df7f422 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -141,11 +141,11 @@ impl Smhc {
         smhc.smhc_clkdiv.write(|w| w.cclk_enb().off());
 
         ccu.disable_module(&mut smhc);
-        // Set module clock rate to 200 MHz
+        // Set module clock rate to 100 MHz
         // TODO: ccu should provide a higher-level abstraction for this
         ccu.borrow_raw().smhc0_clk.write(|w| {
             w.clk_src_sel().pll_peri_1x();
-            w.factor_n().variant(d1_pac::ccu::smhc0_clk::FACTOR_N_A::N1);
+            w.factor_n().variant(d1_pac::ccu::smhc0_clk::FACTOR_N_A::N2);
             w.factor_m().variant(2);
             w.clk_gating().set_bit();
             w

From 5a5c468d6eee896a0a0e62bb943d10293126d627 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 22:36:03 +0100
Subject: [PATCH 19/38] ccu: set gating and reset in single line

---
 platforms/allwinner-d1/d1-core/src/ccu.rs | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/ccu.rs b/platforms/allwinner-d1/d1-core/src/ccu.rs
index 85f3450d..485f5b69 100644
--- a/platforms/allwinner-d1/d1-core/src/ccu.rs
+++ b/platforms/allwinner-d1/d1-core/src/ccu.rs
@@ -262,21 +262,13 @@ macro_rules! impl_bgr {
             impl BusGatingResetRegister for $MODULE {
                 fn gating(ccu: &mut CCU, gating: BusGating) {
                     ccu.$reg.modify(|_, w| {
-                        if gating == BusGating::Mask {
-                            w.$gating().clear_bit()
-                        } else {
-                            w.$gating().set_bit()
-                        }
+                        w.$gating().bit(gating == BusGating::Pass)
                     });
                 }
 
                 fn reset(ccu: &mut CCU, reset: BusReset) {
                     ccu.$reg.modify(|_, w| {
-                        if reset == BusReset::Assert {
-                            w.$reset().clear_bit()
-                        } else {
-                            w.$reset().set_bit()
-                        }
+                        w.$reset().bit(reset == BusReset::Deassert)
                     });
                 }
             }

From 597a9b75d6881d42513ad431ef3f329381a99bd3 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 22:36:35 +0100
Subject: [PATCH 20/38] smhc: extract `reset_fifo` function

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 6df7f422..645ab604 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -207,11 +207,7 @@ impl Smhc {
             core::hint::spin_loop();
         }
 
-        // Reset the FIFO
-        smhc.smhc_ctrl.modify(|_, w| w.fifo_rst().reset());
-        while smhc.smhc_ctrl.read().fifo_rst().is_reset() {
-            core::hint::spin_loop();
-        }
+        Self::reset_fifo(smhc);
 
         // Global interrupt disable
         smhc.smhc_ctrl.modify(|_, w| w.ine_enb().disable());
@@ -245,6 +241,14 @@ impl Smhc {
         }
     }
 
+    #[inline(always)]
+    fn reset_fifo(regs: &smhc::RegisterBlock) {
+        regs.smhc_ctrl.modify(|_, w| w.fifo_rst().reset());
+        while regs.smhc_ctrl.read().fifo_rst().is_reset() {
+            core::hint::spin_loop();
+        }
+    }
+
     /// # Safety
     /// The descriptor chain needs to live at least as long as the DMA transfer.
     /// Additionally, their content (e.g., the validity of the buffers they point to)
@@ -292,11 +296,7 @@ impl Smhc {
             w
         });
 
-        // Reset the FIFO
-        self.smhc.smhc_ctrl.modify(|_, w| w.fifo_rst().reset());
-        while self.smhc.smhc_ctrl.read().fifo_rst().is_reset() {
-            core::hint::spin_loop();
-        }
+        Self::reset_fifo(self.smhc);
     }
 
     pub fn handle_smhc0_interrupt() {

From 4a45088cbe7836ebebae02077074afc1deb44096 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 22:38:26 +0100
Subject: [PATCH 21/38] smhc: refactor `idmac` descriptors to be inline with
 `dmac` implementation

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 247 ++++++++++++------
 1 file changed, 170 insertions(+), 77 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 645ab604..1286af7d 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -398,7 +398,7 @@ impl Smhc {
         let mut descriptors: [idmac::Descriptor; 16] = unsafe { core::mem::zeroed() };
 
         // Perform checks on arguments to make sure we won't overflow the buffer
-        match &guard.data.op {
+        match &mut guard.data.op {
             SmhcOp::Read { buf, cnt, .. } | SmhcOp::Write { buf, cnt, .. } => {
                 const DESCR_BUFF_SIZE: usize = 0x1000;
 
@@ -407,24 +407,28 @@ impl Smhc {
                     return Err(sdmmc::Error::Data);
                 }
 
-                tracing::debug!("Creating descriptor chain from buffer");
-                let buf_ptr = buf.as_slice().as_ptr();
+                tracing::debug!(cnt, "Creating descriptor chain from buffer");
+                let buf_ptr = buf.as_slice_mut().as_mut_ptr();
                 let mut remaining = *cnt;
                 let mut index = 0;
                 while remaining > 0 {
-                    let buff_size = core::cmp::min(DESCR_BUFF_SIZE, remaining) as u16;
+                    let buff_size = core::cmp::min(DESCR_BUFF_SIZE, remaining);
+                    let buff_addr = unsafe { buf_ptr.add(index * DESCR_BUFF_SIZE) };
+                    // Having to construct the slice in this manual way is not ideal,
+                    // but it allows us to use `&mut [u8]` in the DescriptorBuilder.
+                    let slice = unsafe { core::slice::from_raw_parts_mut(buff_addr, buff_size) };
                     remaining = remaining.saturating_sub(DESCR_BUFF_SIZE);
                     let first = index == 0;
                     let last = remaining == 0;
-                    descriptors[index] = idmac::Descriptor::try_from(idmac::DescriptorConfig {
-                        disable_int_on_complete: !last,
-                        first,
-                        last,
-                        buff_size,
-                        buff_addr: unsafe { buf_ptr.add(index * DESCR_BUFF_SIZE) },
-                        next_desc: (!last).then_some(&descriptors[index + 1] as *const _ as _),
-                    })
-                    .map_err(|_| sdmmc::Error::Other)?;
+                    descriptors[index] = idmac::DescriptorBuilder::new()
+                        .disable_irq(!last)
+                        .first(first)
+                        .last(last)
+                        .link((!last).then(|| (&descriptors[index + 1]).into()))
+                        .expect("Should be able to link to next descriptor")
+                        .buff_slice(slice)
+                        .expect("Should be able to configure data buffer with slice")
+                        .build();
                     index += 1;
                 }
 
@@ -661,94 +665,183 @@ impl SmhcData {
 
 /// Internal DMA controller
 mod idmac {
+    use core::mem;
+    use core::ptr::NonNull;
+    use mycelium_bitfield::{bitfield, enum_from_bits};
+
+    /// A descriptor that describes how memory needs to transfer data
+    /// between the SMHC port and host memory. Multiple DMA transfers
+    /// can be configured by creating a descriptor *chain* (linked list).
+    #[derive(Clone, Debug)]
     #[repr(C, align(4))]
-    #[derive(Default, Debug)]
-    pub struct Descriptor {
-        des0: u32,
-        des1: u32,
-        des2: u32,
-        des3: u32,
+    pub(super) struct Descriptor {
+        /// The descriptor configuration.
+        configuration: Cfg,
+        /// The size of the data buffer.
+        buff_size: BuffSize,
+        /// The (*word*) address of the data buffer.
+        buff_addr: u32,
+        /// The (*word*) address of the next descriptor, for creating a descriptor chain.
+        next_desc: u32,
     }
 
-    impl Descriptor {
-        /// Indicates whether the descriptor is currently owned by the IDMAC.
-        pub fn is_owned(&self) -> bool {
-            self.des0 & (1 << 31) != 0
-        }
-
-        /// Indicates whether an error happened during transfer.
-        pub fn is_err(&self) -> bool {
-            self.des0 & (1 << 30) != 0
-        }
-    }
-
-    pub struct DescriptorConfig {
-        /// When true, this will prevent the setting of the TX/RX interrupt
-        /// bit of the IDMAC status register for data that ends in the buffer the
-        /// descriptor points to.
-        pub disable_int_on_complete: bool,
-        /// If this is the first descriptor.
-        pub first: bool,
-        /// If this is the last descriptor.
-        pub last: bool,
-        /// Buffer data size in bytes, must be a multiple of 4.
-        pub buff_size: u16,
-        /// The physical address of the data buffer.
-        pub buff_addr: *const u8,
-        /// The physical address of the next descriptor.
-        pub next_desc: Option<*const ()>,
+    /// A builder for constructing an IDMAC [`Descriptor`].
+    #[derive(Copy, Clone, Debug)]
+    #[must_use = "a `DescriptorBuilder` does nothing unless `DescriptorBuilder::build()` is called"]
+    pub(super) struct DescriptorBuilder<B = ()> {
+        cfg: Cfg,
+        buff: B,
+        link: u32,
     }
 
     #[derive(Debug)]
-    pub enum Error {
+    pub(super) enum Error {
         InvalidBufferAddr,
         InvalidBufferSize,
+        InvalidLink,
+    }
+
+    bitfield! {
+        /// The first 32-bit word of an IDMAC descriptor, containing configuration data.
+        struct Cfg<u32> {
+            // Bit 0 is reserved.
+            const _RESERVED_0 = 1;
+
+            /// Disable interrupts on completion.
+            const CUR_TXRX_OVER_INT_DIS = 1;
+
+            /// When set to 1, this bit indicates that the buffers this descriptor points to
+            /// are the last data buffer.
+            const LAST_FLAG = 1;
+
+            /// When set to 1, this bit indicates that this descriptor contains
+            /// the first buffer of data. It must be set to 1 in the first DES.
+            const FIRST_FLAG = 1;
+
+            /// When set to 1, this bit indicates that the second address in the descriptor
+            /// is the next descriptor address. It must be set to 1.
+            const CHAIN_MOD = 1;
+
+            /// Bits 29:5 are reserved.
+            const _RESERVED_1 = 25;
+
+            /// When some errors happen in transfer, this bit will be set to 1 by the IDMAC.
+            const ERR_FLAG = 1;
+
+            /// When set to 1, this bit indicates that the descriptor is owned by the IDMAC.
+            /// When this bit is reset, it indicates that the descriptor is owned by the host.
+            /// This bit is cleared when the transfer is over.
+            const DES_OWN_FLAG = 1;
+        }
     }
 
-    impl TryFrom<DescriptorConfig> for Descriptor {
-        type Error = Error;
+    bitfield! {
+        /// The second 32-bit word of an IDMAC descriptor, containing data buffer size.
+        struct BuffSize<u32> {
+            /// The data buffer byte size, which must be a multiple of 4 bytes.
+            /// If this field is 0, the DMA ignores this buffer and proceeds to the next descriptor.
+            const SIZE = 13;
 
-        fn try_from(value: DescriptorConfig) -> Result<Self, Self::Error> {
-            let mut descriptor = Descriptor {
-                des0: 0,
-                des1: 0,
-                des2: 0,
-                des3: 0,
-            };
+            /// Bits 31:13 are reserved.
+            const _RESERVED_0 = 19;
+        }
+    }
 
-            if value.disable_int_on_complete {
-                descriptor.des0 |= 1 << 1;
+    impl DescriptorBuilder {
+        pub const fn new() -> Self {
+            Self {
+                cfg: Cfg::new(),
+                buff: (),
+                link: 0,
             }
-            if value.last {
-                descriptor.des0 |= 1 << 2;
+        }
+
+        pub fn disable_irq(self, val: bool) -> Self {
+            Self {
+                cfg: self.cfg.with(Cfg::CUR_TXRX_OVER_INT_DIS, val as u32),
+                ..self
             }
-            if value.first {
-                descriptor.des0 |= 1 << 3;
+        }
+
+        pub fn first(self, val: bool) -> Self {
+            Self {
+                cfg: self.cfg.with(Cfg::FIRST_FLAG, val as u32),
+                ..self
             }
+        }
 
-            // These always need to be set
-            descriptor.des0 |= 1 << 4;
-            descriptor.des0 |= 1 << 31;
+        pub fn last(self, val: bool) -> Self {
+            Self {
+                cfg: self.cfg.with(Cfg::LAST_FLAG, val as u32),
+                ..self
+            }
+        }
 
-            if value.buff_size < (1 << 13) && (value.buff_size & 0b11) == 0 {
-                descriptor.des1 = value.buff_size as u32;
-            } else {
+        pub fn buff_slice(
+            self,
+            buff: &'_ mut [u8],
+        ) -> Result<DescriptorBuilder<&'_ mut [u8]>, Error> {
+            if buff.len() > Descriptor::MAX_LEN as usize {
                 return Err(Error::InvalidBufferSize);
             }
 
-            if !value.buff_addr.is_null() {
-                // Right-shift by 2 because it is a *word-address*.
-                descriptor.des2 = (value.buff_addr as u32) >> 2;
-            } else {
-                return Err(Error::InvalidBufferAddr);
+            if (buff.len() & 0b11) > 0 {
+                return Err(Error::InvalidBufferSize);
+            }
+
+            Ok(DescriptorBuilder {
+                cfg: self.cfg,
+                buff,
+                link: self.link,
+            })
+        }
+
+        pub fn link(self, link: impl Into<Option<NonNull<Descriptor>>>) -> Result<Self, Error> {
+            let link = link
+                .into()
+                .map(Descriptor::addr_to_link)
+                .transpose()?
+                .unwrap_or(0);
+            Ok(Self { link, ..self })
+        }
+    }
+
+    impl DescriptorBuilder<&'_ mut [u8]> {
+        pub fn build(self) -> Descriptor {
+            let buff_size = BuffSize::new().with(BuffSize::SIZE, self.buff.len() as u32);
+            let buff_addr = (self.buff.as_mut_ptr() as *mut _ as u32) >> 2;
+
+            Descriptor {
+                configuration: self.cfg.with(Cfg::CHAIN_MOD, 1).with(Cfg::DES_OWN_FLAG, 1),
+                buff_size,
+                buff_addr,
+                next_desc: self.link,
             }
+        }
+    }
+
+    impl Descriptor {
+        /// Maximum length for arguments to [`DescriptorBuilder::buff_slice`].
+        /// Must be 13 bits wide or less.
+        pub const MAX_LEN: u32 = (1 << 13) - 1;
+
+        /// Indicates whether the descriptor is currently owned by the IDMAC.
+        pub fn is_owned(&self) -> bool {
+            self.configuration.get(Cfg::DES_OWN_FLAG) != 0
+        }
+
+        /// Indicates whether an error happened during transfer.
+        pub fn is_err(&self) -> bool {
+            self.configuration.get(Cfg::ERR_FLAG) != 0
+        }
 
-            if let Some(next) = value.next_desc {
-                // Right-shift by 2 because it is a *word-address*.
-                descriptor.des3 = (next as u32) >> 2;
+        fn addr_to_link(link: NonNull<Self>) -> Result<u32, Error> {
+            let addr = link.as_ptr() as usize;
+            if addr & (mem::align_of::<Self>() - 1) > 0 {
+                return Err(Error::InvalidLink);
             }
 
-            Ok(descriptor)
+            Ok((addr as u32) >> 2)
         }
     }
 }

From 24cb3cb9c1cca32219fe2454b564455a385eb438 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 22:38:48 +0100
Subject: [PATCH 22/38] smhc: apply more PR feedback

---
 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 1286af7d..56e0041b 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -259,7 +259,9 @@ impl Smhc {
             w.dma_rst().set_bit();
             w
         });
-        while self.smhc.smhc_ctrl.read().dma_rst().bit_is_set() {}
+        while self.smhc.smhc_ctrl.read().dma_rst().bit_is_set() {
+            core::hint::spin_loop();
+        }
 
         // Configure the address of the first DMA descriptor
         // Right-shift by 2 because it is a *word-address*.
@@ -274,6 +276,8 @@ impl Smhc {
         self.smhc.smhc_idmac.write(|w| w.idmac_rst().set_bit());
 
         // Configure the burst size and TX/RX trigger level
+        // to the same values as used in the Linux implementation:
+        // Burst size = 8, RX_TL = 7, TX_TL = 8
         self.smhc.smhc_fifoth.write(|w| {
             w.tx_tl().variant(8);
             w.rx_tl().variant(7);
@@ -571,6 +575,9 @@ impl SmhcData {
 
         let mut needs_wake = false;
 
+        // NOTE: to clear bits in the interrupt status registers,
+        // you have to *write* a 1 to their location (W1C = write 1 to clear).
+
         self.err = Self::error_status(smhc);
         if let Some(err) = self.err {
             // Clear all interrupt bits

From d53a8072467b3fb5479e15c2fc5e72dcbf98ec0e Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 22:39:08 +0100
Subject: [PATCH 23/38] dmac: some spelling fixes

---
 platforms/allwinner-d1/d1-core/src/dmac/mod.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/dmac/mod.rs b/platforms/allwinner-d1/d1-core/src/dmac/mod.rs
index 9a9c89bf..4eb8e501 100644
--- a/platforms/allwinner-d1/d1-core/src/dmac/mod.rs
+++ b/platforms/allwinner-d1/d1-core/src/dmac/mod.rs
@@ -36,7 +36,7 @@ pub mod descriptor;
 /// This struct is constructed using [`Dmac::new`], which initializes the DMA
 /// controller and returns a `Dmac`. Since this struct is essentially a token
 /// representing that the DMAC has been initialized, it may be freely copied
-/// into any driver that wishes to perform DMA oeprations.
+/// into any driver that wishes to perform DMA operations.
 #[derive(Copy, Clone)]
 pub struct Dmac {
     // this struct is essentially used as a "yes, the DMAC is initialized now" token...
@@ -89,7 +89,7 @@ pub enum ChannelMode {
     /// transfer completes, and waits for the peripheral to pull the DMA
     /// Active signal low before starting the next transfer.
     ///
-    /// The Allwinner documentationh for the D1 describes this mode as follows:
+    /// The Allwinner documentation for the D1 describes this mode as follows:
     ///
     /// > * When the DMAC detects a valid external request signal, the DMAC
     /// >   starts to operate the peripheral device. The internal DRQ always
@@ -222,7 +222,7 @@ impl Dmac {
     /// some of the data it wanted. If we were reading from a device, reads may
     /// have side effects and incomplete reads may leave the device in a weird
     /// state. Cancelling an incomplete transfer may result in, for example,
-    /// writing out half of a string to the UART, or only part  of a structured
+    /// writing out half of a string to the UART, or only part of a structured
     /// message over SPI, and so on. But, at least we don't have abandoned DMA
     /// transfers running around in random parts of the heap you probably wanted
     /// to use for normal stuff like having strings, or whatever it is that
@@ -398,6 +398,7 @@ impl Channel {
     /// dropped, the descriptor and its associated memory region may also be
     /// dropped safely.
     ///
+    /// Of course, the transfer may still have completed partially. If we
     /// were writing to a device, the device may be unhappy to have only gotten
     /// some of the data it wanted. If we were reading from a device, reads may
     /// have side effects and incomplete reads may leave the device in a weird

From 4853c4641643c6fc35a1596b025c0546ffc494aa Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 19 Nov 2023 23:06:23 +0100
Subject: [PATCH 24/38] smhc: fix some check + clippy lints

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 40 +++++++++----------
 source/kernel/src/services/sdmmc.rs           | 18 +++++----
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 56e0041b..e6d23014 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -1,3 +1,6 @@
+// Note: We sometimes force a pass by ref mut to enforce exclusive access
+#![allow(clippy::needless_pass_by_ref_mut)]
+
 //! Driver for the Allwinner D1's SMHC peripheral.
 //!
 //! The D1 contains three separate SD/MMC Host Controllers: [`SMHC0`], [`SMHC1`] and [`SMHC2`].
@@ -13,7 +16,7 @@ use core::{
     task::{Poll, Waker},
 };
 
-use crate::ccu::{BusGatingResetRegister, Ccu};
+use crate::ccu::Ccu;
 use d1_pac::{smhc, Interrupt, GPIO, SMHC0, SMHC1, SMHC2};
 use kernel::{
     mnemos_alloc::containers::FixedVec,
@@ -356,8 +359,8 @@ impl Smhc {
         // Should this (auto_stop select) be part of the params?
         let (data_trans, trans_dir, auto_stop) = match params.kind {
             sdmmc::CommandKind::Control => (false, false, false),
-            sdmmc::CommandKind::Read(len) => (true, false, true),
-            sdmmc::CommandKind::Write(len) => (true, true, true),
+            sdmmc::CommandKind::Read(_len) => (true, false, true),
+            sdmmc::CommandKind::Write(_len) => (true, true, true),
         };
         let chk_resp_crc = params.rsp_crc;
         let long_resp = params.rsp_type == sdmmc::ResponseType::Long;
@@ -579,7 +582,7 @@ impl SmhcData {
         // you have to *write* a 1 to their location (W1C = write 1 to clear).
 
         self.err = Self::error_status(smhc);
-        if let Some(err) = self.err {
+        if self.err.is_some() {
             // Clear all interrupt bits
             smhc.smhc_rintsts.write(|w| unsafe { w.bits(0xFFFF_FFFF) });
             smhc.smhc_idst.write(|w| unsafe { w.bits(0x3FF) });
@@ -674,7 +677,7 @@ impl SmhcData {
 mod idmac {
     use core::mem;
     use core::ptr::NonNull;
-    use mycelium_bitfield::{bitfield, enum_from_bits};
+    use mycelium_bitfield::bitfield;
 
     /// A descriptor that describes how memory needs to transfer data
     /// between the SMHC port and host memory. Multiple DMA transfers
@@ -703,9 +706,9 @@ mod idmac {
 
     #[derive(Debug)]
     pub(super) enum Error {
-        InvalidBufferAddr,
-        InvalidBufferSize,
-        InvalidLink,
+        BufferAddr,
+        BufferSize,
+        Link,
     }
 
     bitfield! {
@@ -789,11 +792,16 @@ mod idmac {
             buff: &'_ mut [u8],
         ) -> Result<DescriptorBuilder<&'_ mut [u8]>, Error> {
             if buff.len() > Descriptor::MAX_LEN as usize {
-                return Err(Error::InvalidBufferSize);
+                return Err(Error::BufferSize);
             }
 
             if (buff.len() & 0b11) > 0 {
-                return Err(Error::InvalidBufferSize);
+                return Err(Error::BufferSize);
+            }
+
+            let buff_addr = buff.as_mut_ptr() as *mut _ as u32;
+            if (buff_addr & 0b11) > 0 {
+                return Err(Error::BufferAddr);
             }
 
             Ok(DescriptorBuilder {
@@ -832,20 +840,10 @@ mod idmac {
         /// Must be 13 bits wide or less.
         pub const MAX_LEN: u32 = (1 << 13) - 1;
 
-        /// Indicates whether the descriptor is currently owned by the IDMAC.
-        pub fn is_owned(&self) -> bool {
-            self.configuration.get(Cfg::DES_OWN_FLAG) != 0
-        }
-
-        /// Indicates whether an error happened during transfer.
-        pub fn is_err(&self) -> bool {
-            self.configuration.get(Cfg::ERR_FLAG) != 0
-        }
-
         fn addr_to_link(link: NonNull<Self>) -> Result<u32, Error> {
             let addr = link.as_ptr() as usize;
             if addr & (mem::align_of::<Self>() - 1) > 0 {
-                return Err(Error::InvalidLink);
+                return Err(Error::Link);
             }
 
             Ok((addr as u32) >> 2)
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 7eca71e0..936f61e9 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -12,7 +12,6 @@ use crate::{
     registry::{self, known_uuids, Envelope, KernelHandle, RegisteredDriver},
     Kernel,
 };
-use core::{convert::Infallible, fmt, time::Duration};
 use uuid::Uuid;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -114,7 +113,9 @@ pub enum Response {
     /// The 32-bit value from the 48-bit response.
     /// Potentially also includes the data buffer if this was a read command.
     Short {
+        /// The response on the command line.
         value: u32,
+        /// The received data, in case of a read command.
         data: Option<FixedVec<u8>>,
     },
     /// The 128-bit value from the 136-bit response.
@@ -164,8 +165,11 @@ pub struct SdCardClient {
 /// The different types of cards
 #[derive(Debug, PartialEq)]
 pub enum CardType {
+    /// Standard capacity v1
     SD1,
+    /// Standard capacity v2 (or later)
     SD2,
+    /// High capacity
     SDHC,
 }
 
@@ -279,7 +283,7 @@ impl SdCardClient {
         // TODO: limit the number of attempts
         let ocr = loop {
             // Go to *APP* mode before sending application command
-            self.cmd(Command {
+            let _ = self.cmd(Command {
                 index: 55,
                 argument: 0,
                 options: HardwareOptions::None,
@@ -292,7 +296,7 @@ impl SdCardClient {
 
             let mut op_cond_arg = OCR_VOLTAGE_MASK & 0x00ff8000;
             if card_type != CardType::SD1 {
-                op_cond_arg = OCR_HCS | op_cond_arg;
+                op_cond_arg |= OCR_HCS;
             }
             match self
                 .cmd(Command {
@@ -340,7 +344,7 @@ impl SdCardClient {
             })
             .await?
         {
-            Response::Short { .. } => return Err(Error::Response),
+            Response::Short { .. } => Err(Error::Response),
             Response::Long(value) => {
                 tracing::trace!("CMD2 response: {value:?}");
                 // TODO: map [u32; 4] value to u128
@@ -367,7 +371,7 @@ impl SdCardClient {
                 tracing::trace!("CMD3 response: {value:#x}");
                 Ok(RelativeCardAddress(value))
             }
-            Response::Long(_) => return Err(Error::Response),
+            Response::Long(_) => Err(Error::Response),
         }
     }
 
@@ -389,14 +393,14 @@ impl SdCardClient {
                 tracing::trace!("CMD7 response: {value:#x}");
                 Ok(CardStatus(value))
             }
-            Response::Long(_) => return Err(Error::Response),
+            Response::Long(_) => Err(Error::Response),
         }
     }
 
     /// Use 4 data lanes
     pub async fn set_wide_bus(&mut self, rca: RelativeCardAddress) -> Result<CardStatus, Error> {
         // Go to *APP* mode before sending application command
-        self.cmd(Command {
+        let _ = self.cmd(Command {
             index: 55,
             argument: rca.0,
             options: HardwareOptions::None,

From 49103032ebd41a2f9c9c916878b441cb4d87248b Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 20 Nov 2023 14:39:50 +0100
Subject: [PATCH 25/38] smhc: fix lints + formatting

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 22 ++++++++++
 source/kernel/src/services/sdmmc.rs           | 42 ++++++++++---------
 2 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index e6d23014..655f8ab5 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -186,6 +186,22 @@ impl Smhc {
         )
     }
 
+    /// Initialize SMHC1 for SDIO cards.
+    ///
+    /// # Safety
+    /// TODO
+    pub unsafe fn smhc1(_smhc: SMHC1, _ccu: &mut Ccu, _gpio: &mut GPIO) -> Self {
+        todo!()
+    }
+
+    /// Initialize SMHC2 for MMC cards.
+    ///
+    /// # Safety
+    /// TODO
+    pub unsafe fn smhc2(_smhc: SMHC2, _ccu: &mut Ccu, _gpio: &mut GPIO) -> Self {
+        todo!()
+    }
+
     /// This assumes the GPIO pin mappings and module clock are already configured.
     unsafe fn init(smhc: &'static smhc::RegisterBlock, int: Interrupt, isr: fn()) -> Self {
         // Closure to change the card clock
@@ -306,6 +322,12 @@ impl Smhc {
         Self::reset_fifo(self.smhc);
     }
 
+    /// Returns the interrupt and ISR for this SMHC.
+    pub fn interrupt(&self) -> (Interrupt, fn()) {
+        self.int
+    }
+
+    /// Handle an interrupt for SMHC0
     pub fn handle_smhc0_interrupt() {
         let _isr = kernel::isr::Isr::enter();
         let smhc = unsafe { &*SMHC0::ptr() };
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 936f61e9..26d70bb7 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -283,16 +283,17 @@ impl SdCardClient {
         // TODO: limit the number of attempts
         let ocr = loop {
             // Go to *APP* mode before sending application command
-            let _ = self.cmd(Command {
-                index: 55,
-                argument: 0,
-                options: HardwareOptions::None,
-                kind: CommandKind::Control,
-                rsp_type: ResponseType::Short,
-                rsp_crc: true,
-                buffer: None,
-            })
-            .await?;
+            let _ = self
+                .cmd(Command {
+                    index: 55,
+                    argument: 0,
+                    options: HardwareOptions::None,
+                    kind: CommandKind::Control,
+                    rsp_type: ResponseType::Short,
+                    rsp_crc: true,
+                    buffer: None,
+                })
+                .await?;
 
             let mut op_cond_arg = OCR_VOLTAGE_MASK & 0x00ff8000;
             if card_type != CardType::SD1 {
@@ -400,16 +401,17 @@ impl SdCardClient {
     /// Use 4 data lanes
     pub async fn set_wide_bus(&mut self, rca: RelativeCardAddress) -> Result<CardStatus, Error> {
         // Go to *APP* mode before sending application command
-        let _ = self.cmd(Command {
-            index: 55,
-            argument: rca.0,
-            options: HardwareOptions::None,
-            kind: CommandKind::Control,
-            rsp_type: ResponseType::Short,
-            rsp_crc: true,
-            buffer: None,
-        })
-        .await?;
+        let _ = self
+            .cmd(Command {
+                index: 55,
+                argument: rca.0,
+                options: HardwareOptions::None,
+                kind: CommandKind::Control,
+                rsp_type: ResponseType::Short,
+                rsp_crc: true,
+                buffer: None,
+            })
+            .await?;
 
         match self
             .cmd(Command {

From a0a3a9bb7fa7e693c839d6cfd0108f64ac38bd6b Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 20 Nov 2023 14:43:31 +0100
Subject: [PATCH 26/38] kernel/src/services/mod: fix rustfmt

---
 source/kernel/src/services/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source/kernel/src/services/mod.rs b/source/kernel/src/services/mod.rs
index 9f97a4a0..cee7e319 100644
--- a/source/kernel/src/services/mod.rs
+++ b/source/kernel/src/services/mod.rs
@@ -21,7 +21,7 @@
 pub mod emb_display;
 pub mod forth_spawnulator;
 pub mod i2c;
-pub mod sdmmc;
 pub mod keyboard;
+pub mod sdmmc;
 pub mod serial_mux;
 pub mod simple_serial;

From 8de15613a7b5aa2fcf5e56e5ec70f7a73abd66e3 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 20 Nov 2023 22:02:53 +0100
Subject: [PATCH 27/38] smhc+sdmmc: improve error handling + doc comments

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 21 +++++++++++++----
 source/kernel/src/services/sdmmc.rs           | 23 ++++++++++---------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 655f8ab5..d048b05f 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -433,7 +433,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::Data);
+                    return Err(sdmmc::Error::Buffer);
                 }
 
                 tracing::debug!(cnt, "Creating descriptor chain from buffer");
@@ -456,7 +456,7 @@ impl Smhc {
                         .link((!last).then(|| (&descriptors[index + 1]).into()))
                         .expect("Should be able to link to next descriptor")
                         .buff_slice(slice)
-                        .expect("Should be able to configure data buffer with slice")
+                        .map_err(|_| sdmmc::Error::Buffer)?
                         .build();
                     index += 1;
                 }
@@ -493,7 +493,20 @@ impl Smhc {
         tracing::trace!("SMHC operation completed");
         if let Some(error) = guard.data.err.take() {
             tracing::warn!(?error, "SMHC error");
-            Err(sdmmc::Error::Other) // TODO
+            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,
+            })
         } else if long_resp {
             Ok(sdmmc::Response::Long([
                 self.smhc.smhc_resp0.read().bits(),
@@ -660,7 +673,7 @@ impl SmhcData {
                             cnt,
                         } => {
                             tracing::trace!("setting buf len: {cnt}");
-                            // TODO: Safety?
+                            // Safety: we have already checked that cnt <= buf capacity
                             unsafe { buf.as_vec_mut().set_len(*cnt) };
                             if *auto_stop {
                                 State::WaitForAutoStop
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 26d70bb7..b062bfa2 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -74,7 +74,7 @@ pub enum BusWidth {
     Octo,
 }
 
-/// TODO
+/// Hardware configuration that should be applied as part of the command
 #[derive(Debug, Eq, PartialEq)]
 pub enum HardwareOptions {
     /// No change in configuration
@@ -83,7 +83,7 @@ pub enum HardwareOptions {
     SetBusWidth(BusWidth),
 }
 
-/// TODO
+/// The different types of commands that can be sent to the card
 #[derive(Debug, Eq, PartialEq)]
 pub enum CommandKind {
     /// Command without data transfer
@@ -94,7 +94,7 @@ pub enum CommandKind {
     Write(usize),
 }
 
-/// TODO
+/// The different types of responses that can be sent by the card
 #[derive(Debug, Eq, PartialEq)]
 pub enum ResponseType {
     /// No Response
@@ -119,22 +119,23 @@ pub enum Response {
         data: Option<FixedVec<u8>>,
     },
     /// The 128-bit value from the 136-bit response.
-    // TODO: make this `u128`?
     Long([u32; 4]),
 }
 
-/// TODO
+/// Errors returned by the [`SdmmcService`]
 #[derive(Debug, Eq, PartialEq)]
 pub enum Error {
-    /// TODO
+    /// The service is currently busy and cannot handle the request
     Busy,
-    /// TODO
+    /// Invalid or unexpected response was received
     Response,
-    /// TODO
+    /// Invalid or unexpected data was received
     Data,
-    /// TODO
+    /// The provided buffer does not meet the requirements
+    Buffer,
+    /// A timeout occurred
     Timeout,
-    /// TODO
+    /// A different error has occurred
     Other,
 }
 
@@ -442,7 +443,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::Data);
+            return Err(Error::Buffer);
         }
 
         match self

From 1b01fa5f73c2241f0d4eb78a0086b0f095462b25 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 20 Nov 2023 22:03:30 +0100
Subject: [PATCH 28/38] sdmmc: map [u32;4] to u128 for card identification
 register

---
 source/kernel/src/services/sdmmc.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index b062bfa2..b4d6ef3d 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -349,8 +349,12 @@ impl SdCardClient {
             Response::Short { .. } => Err(Error::Response),
             Response::Long(value) => {
                 tracing::trace!("CMD2 response: {value:?}");
-                // TODO: map [u32; 4] value to u128
-                Ok(CardIdentification(0))
+                let mut bytes = [0u8; 16];
+                bytes[0..4].copy_from_slice(&value[0].to_le_bytes());
+                bytes[4..8].copy_from_slice(&value[1].to_le_bytes());
+                bytes[8..12].copy_from_slice(&value[2].to_le_bytes());
+                bytes[12..16].copy_from_slice(&value[3].to_le_bytes());
+                Ok(CardIdentification(u128::from_le_bytes(bytes)))
             }
         }
     }

From 18d2757dfc0294e27b31461c77fcffbabb028056 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 21 Nov 2023 22:36:06 +0100
Subject: [PATCH 29/38] smhc: add `num` field to `Smhc` struct

---
 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index d048b05f..5680ffa4 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -30,6 +30,7 @@ pub struct Smhc {
     isr: &'static IsrData,
     smhc: &'static smhc::RegisterBlock,
     int: (Interrupt, fn()),
+    num: u8,
 }
 
 /// TODO
@@ -183,6 +184,7 @@ impl Smhc {
             unsafe { &*SMHC0::ptr() },
             Interrupt::SMHC0,
             Self::handle_smhc0_interrupt,
+            0,
         )
     }
 
@@ -203,7 +205,7 @@ impl Smhc {
     }
 
     /// This assumes the GPIO pin mappings and module clock are already configured.
-    unsafe fn init(smhc: &'static smhc::RegisterBlock, int: Interrupt, isr: fn()) -> Self {
+    unsafe fn init(smhc: &'static smhc::RegisterBlock, int: Interrupt, isr: fn(), num: u8) -> Self {
         // Closure to change the card clock
         let prg_clk = || {
             smhc.smhc_cmd.write(|w| {
@@ -257,6 +259,7 @@ impl Smhc {
             smhc,
             isr: &SMHC0_ISR,
             int: (int, isr),
+            num,
         }
     }
 
@@ -355,7 +358,7 @@ impl Smhc {
         Ok(())
     }
 
-    #[tracing::instrument(name = "SMHC", level = tracing::Level::INFO, skip(self, rx))]
+    #[tracing::instrument(name = "SMHC", fields(num = self.num), level = tracing::Level::INFO, skip(self, rx))]
     async fn run(self, rx: registry::listener::RequestStream<SdmmcService>) {
         tracing::info!("starting SMHC driver task");
         loop {

From c517354a5c8c8dcae7d657165a36b202a1044024 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 21 Nov 2023 22:37:32 +0100
Subject: [PATCH 30/38] smhc: use `bool` for single-bit fields in `Cfg`
 bitfield

---
 .../allwinner-d1/d1-core/src/drivers/smhc.rs  | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 5680ffa4..8faf2c99 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -756,30 +756,30 @@ mod idmac {
             const _RESERVED_0 = 1;
 
             /// Disable interrupts on completion.
-            const CUR_TXRX_OVER_INT_DIS = 1;
+            const CUR_TXRX_OVER_INT_DIS: bool;
 
-            /// When set to 1, this bit indicates that the buffers this descriptor points to
-            /// are the last data buffer.
-            const LAST_FLAG = 1;
+            /// When set to 1, this bit indicates that the buffer this descriptor points to
+            /// is the last data buffer.
+            const LAST: bool;
 
             /// When set to 1, this bit indicates that this descriptor contains
             /// the first buffer of data. It must be set to 1 in the first DES.
-            const FIRST_FLAG = 1;
+            const FIRST: bool;
 
             /// When set to 1, this bit indicates that the second address in the descriptor
             /// is the next descriptor address. It must be set to 1.
-            const CHAIN_MOD = 1;
+            const CHAIN_MOD: bool;
 
             /// Bits 29:5 are reserved.
             const _RESERVED_1 = 25;
 
             /// When some errors happen in transfer, this bit will be set to 1 by the IDMAC.
-            const ERR_FLAG = 1;
+            const ERR: bool;
 
             /// When set to 1, this bit indicates that the descriptor is owned by the IDMAC.
             /// When this bit is reset, it indicates that the descriptor is owned by the host.
             /// This bit is cleared when the transfer is over.
-            const DES_OWN_FLAG = 1;
+            const DES_OWN: bool;
         }
     }
 
@@ -806,21 +806,21 @@ mod idmac {
 
         pub fn disable_irq(self, val: bool) -> Self {
             Self {
-                cfg: self.cfg.with(Cfg::CUR_TXRX_OVER_INT_DIS, val as u32),
+                cfg: self.cfg.with(Cfg::CUR_TXRX_OVER_INT_DIS, val),
                 ..self
             }
         }
 
         pub fn first(self, val: bool) -> Self {
             Self {
-                cfg: self.cfg.with(Cfg::FIRST_FLAG, val as u32),
+                cfg: self.cfg.with(Cfg::FIRST, val),
                 ..self
             }
         }
 
         pub fn last(self, val: bool) -> Self {
             Self {
-                cfg: self.cfg.with(Cfg::LAST_FLAG, val as u32),
+                cfg: self.cfg.with(Cfg::LAST, val),
                 ..self
             }
         }
@@ -865,7 +865,7 @@ mod idmac {
             let buff_addr = (self.buff.as_mut_ptr() as *mut _ as u32) >> 2;
 
             Descriptor {
-                configuration: self.cfg.with(Cfg::CHAIN_MOD, 1).with(Cfg::DES_OWN_FLAG, 1),
+                configuration: self.cfg.with(Cfg::CHAIN_MOD, true).with(Cfg::DES_OWN, true),
                 buff_size,
                 buff_addr,
                 next_desc: self.link,

From d676e44b7015129d3c3d2d07fbe1d5c546c01013 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 21 Nov 2023 22:38:08 +0100
Subject: [PATCH 31/38] smhc: update some doc comments

---
 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 8faf2c99..1ad56f53 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -25,7 +25,6 @@ use kernel::{
     tracing, Kernel,
 };
 
-/// TODO
 pub struct Smhc {
     isr: &'static IsrData,
     smhc: &'static smhc::RegisterBlock,
@@ -33,7 +32,7 @@ pub struct Smhc {
     num: u8,
 }
 
-/// TODO
+/// Data used by a SMHC interrupt.
 struct IsrData {
     data: UnsafeCell<SmhcData>,
 }
@@ -75,7 +74,7 @@ enum SmhcOp {
     },
 }
 
-/// TODO
+/// SMHC state machine
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[allow(dead_code)]
 enum State {
@@ -90,7 +89,7 @@ enum State {
     WaitForAutoStop,
 }
 
-/// TODO
+/// The different errors that can occur in this module.
 #[derive(Debug, Copy, Clone)]
 #[non_exhaustive]
 pub enum ErrorKind {
@@ -127,7 +126,8 @@ impl Smhc {
     /// Initialize SMHC0 for SD cards.
     ///
     /// # Safety
-    /// TODO
+    /// - The `SMHC0` register block must not be concurrently written to.
+    /// - This function should be called only while running on an Allwinner D1.
     pub unsafe fn smhc0(mut smhc: SMHC0, ccu: &mut Ccu, gpio: &mut GPIO) -> Self {
         // Configure default pin mapping for TF (micro SD) card socket.
         // This is valid for the Lichee RV SOM and Mango Pi MQ Pro.

From a8478c7ebe8a88a781d65e199d70f8a67be332b3 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 26 Nov 2023 19:21:07 +0100
Subject: [PATCH 32/38] allwinner-d1/src/lib: remove sdcard test code

---
 platforms/allwinner-d1/src/lib.rs | 40 +------------------------------
 1 file changed, 1 insertion(+), 39 deletions(-)

diff --git a/platforms/allwinner-d1/src/lib.rs b/platforms/allwinner-d1/src/lib.rs
index f7ab95c9..88521139 100644
--- a/platforms/allwinner-d1/src/lib.rs
+++ b/platforms/allwinner-d1/src/lib.rs
@@ -24,8 +24,7 @@ use core::{
 };
 use d1_pac::{Interrupt, TIMER};
 use kernel::{
-    mnemos_alloc::containers::{Box, FixedVec},
-    services::sdmmc::SdCardClient,
+    mnemos_alloc::containers::Box,
     tracing::{self, Instrument},
     Kernel, KernelServiceSettings, KernelSettings,
 };
@@ -159,43 +158,6 @@ pub fn kernel_entry(config: mnemos_config::MnemosConfig<PlatformConfig>) -> ! {
     #[cfg(feature = "sharp-display")]
     d1.initialize_sharp_display();
 
-    // #[cfg(feature = "sdcard")]
-    d1.kernel
-        .initialize(async move {
-            let mut sdcard = SdCardClient::from_registry(d1.kernel)
-                .await
-                .expect("no sdmmc service running!");
-
-            d1.kernel.sleep(core::time::Duration::from_secs(3)).await;
-            tracing::debug!("resetting the SD card");
-
-            sdcard.reset().await.expect("can't reset the SD card");
-            let card_type = sdcard
-                .initialize()
-                .await
-                .expect("can't initialize the SD card");
-            tracing::debug!("initialized card of type {card_type:?}");
-
-            sdcard.get_cid().await.expect("can't get CID");
-            let rca = sdcard.get_rca().await.expect("can't get RCA");
-            let card_status = sdcard.select(rca).await.expect("can't select card");
-            tracing::debug!("card status: {card_status:?}");
-            sdcard
-                .set_wide_bus(rca)
-                .await
-                .expect("can't set wide bus mode");
-
-            let buffer = FixedVec::new(16 * 512).await;
-            match sdcard.read(2048, 16, buffer).await {
-                Ok(buffer) => {
-                    let tmp = &buffer.as_slice()[0..16];
-                    tracing::info!("Read data from SD card: {tmp:?}")
-                }
-                Err(_) => tracing::error!("Failed to read SD card"),
-            }
-        })
-        .expect("failed to spawn sdcard client");
-
     d1.run()
 }
 

From 0f14a11e0dfcaf94c350a0963cb3254b8dadd9db Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Mon, 27 Nov 2023 18:51:41 +0100
Subject: [PATCH 33/38] sdmmc: update mapping of [u32;4] to u128

---
 source/kernel/src/services/sdmmc.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index b4d6ef3d..0eb4dd29 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -349,12 +349,12 @@ impl SdCardClient {
             Response::Short { .. } => Err(Error::Response),
             Response::Long(value) => {
                 tracing::trace!("CMD2 response: {value:?}");
-                let mut bytes = [0u8; 16];
-                bytes[0..4].copy_from_slice(&value[0].to_le_bytes());
-                bytes[4..8].copy_from_slice(&value[1].to_le_bytes());
-                bytes[8..12].copy_from_slice(&value[2].to_le_bytes());
-                bytes[12..16].copy_from_slice(&value[3].to_le_bytes());
-                Ok(CardIdentification(u128::from_le_bytes(bytes)))
+                union CidResponse {
+                    value: [u32; 4],
+                    cid: u128,
+                }
+                let rsp = CidResponse { value };
+                Ok(CardIdentification(unsafe { rsp.cid }))
             }
         }
     }

From 86c6b8def6147ef2d43ff310d2a7aeec4b17bc20 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 5 Dec 2023 21:24:54 +0100
Subject: [PATCH 34/38] sdmmc: use `maitake::time` to sleep in
 `SdCardClient::initialize`

---
 source/kernel/src/services/sdmmc.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 0eb4dd29..bdb1d5d2 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -12,6 +12,7 @@ use crate::{
     registry::{self, known_uuids, Envelope, KernelHandle, RegisteredDriver},
     Kernel,
 };
+use maitake::time::{self, Duration};
 use uuid::Uuid;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -322,7 +323,7 @@ impl SdCardClient {
                 Response::Long(_) => return Err(Error::Response),
             }
 
-            // TODO: wait some time (e.g., 1ms?)
+            time::sleep(Duration::from_millis(1)).await;
         };
 
         if (ocr & OCR_HCS) == OCR_HCS {

From 29c1dd1428de0fbde427d2e60d7d00a99bf5ea6c Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 5 Dec 2023 21:27:06 +0100
Subject: [PATCH 35/38] sdmmc: `Response::Long` contains u128

+ smhc: use transmute to convert [u32;4] to u128
---
 platforms/allwinner-d1/d1-core/src/drivers/smhc.rs |  5 +++--
 source/kernel/src/services/sdmmc.rs                | 12 ++----------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
index 1ad56f53..0a356164 100644
--- a/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
+++ b/platforms/allwinner-d1/d1-core/src/drivers/smhc.rs
@@ -511,12 +511,13 @@ impl Smhc {
                 ErrorKind::Other => sdmmc::Error::Other,
             })
         } else if long_resp {
-            Ok(sdmmc::Response::Long([
+            let rsp: [u32; 4] = [
                 self.smhc.smhc_resp0.read().bits(),
                 self.smhc.smhc_resp1.read().bits(),
                 self.smhc.smhc_resp2.read().bits(),
                 self.smhc.smhc_resp3.read().bits(),
-            ]))
+            ];
+            Ok(sdmmc::Response::Long(unsafe { core::mem::transmute(rsp) }))
         } else {
             Ok(sdmmc::Response::Short {
                 value: self.smhc.smhc_resp0.read().bits(),
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index bdb1d5d2..ea3f2e51 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -120,7 +120,7 @@ pub enum Response {
         data: Option<FixedVec<u8>>,
     },
     /// The 128-bit value from the 136-bit response.
-    Long([u32; 4]),
+    Long(u128),
 }
 
 /// Errors returned by the [`SdmmcService`]
@@ -348,15 +348,7 @@ impl SdCardClient {
             .await?
         {
             Response::Short { .. } => Err(Error::Response),
-            Response::Long(value) => {
-                tracing::trace!("CMD2 response: {value:?}");
-                union CidResponse {
-                    value: [u32; 4],
-                    cid: u128,
-                }
-                let rsp = CidResponse { value };
-                Ok(CardIdentification(unsafe { rsp.cid }))
-            }
+            Response::Long(value) => Ok(CardIdentification(value)),
         }
     }
 

From 15c49d5cd20270356b193b3853a88ce7a8486ecd Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Tue, 5 Dec 2023 21:31:44 +0100
Subject: [PATCH 36/38] sdmmc: move response tracing line to
 `SdCardClient::cmd`

---
 source/kernel/src/services/sdmmc.rs | 42 ++++++++++++++---------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index ea3f2e51..10a03ab4 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -154,6 +154,15 @@ impl Default for Command {
     }
 }
 
+impl core::fmt::Debug for Response {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Response::Short { value, data: _ } => write!(f, "{:#x}", value),
+            Response::Long(value) => write!(f, "{:#x}", value),
+        }
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Client Definition
 ////////////////////////////////////////////////////////////////////////////////
@@ -232,14 +241,18 @@ impl SdCardClient {
     }
 
     async fn cmd(&mut self, cmd: Command) -> Result<Response, Error> {
-        self.handle
+        let index = cmd.index;
+        let result = self
+            .handle
             .request_oneshot(cmd, &self.reply)
             .await
             .map_err(|error| {
                 tracing::warn!(?error, "failed to send request to SD/MMC service");
                 Error::Other // TODO
             })
-            .and_then(|resp| resp.body)
+            .and_then(|resp| resp.body);
+        tracing::trace!("CMD{} response: {:?}", index, result);
+        result
     }
 
     /// Reset the card
@@ -274,7 +287,6 @@ impl SdCardClient {
             .await?
         {
             Response::Short { value, .. } => {
-                tracing::trace!("CMD8 response: {value:#x}");
                 if value == 0x1AA {
                     card_type = CardType::SD2;
                 }
@@ -314,7 +326,6 @@ impl SdCardClient {
                 .await?
             {
                 Response::Short { value, .. } => {
-                    tracing::trace!("ACMD41 response: {value:#x}");
                     if value & OCR_NBUSY == OCR_NBUSY {
                         // Card has finished power up, data is valid
                         break value;
@@ -366,10 +377,7 @@ impl SdCardClient {
             })
             .await?
         {
-            Response::Short { value, .. } => {
-                tracing::trace!("CMD3 response: {value:#x}");
-                Ok(RelativeCardAddress(value))
-            }
+            Response::Short { value, .. } => Ok(RelativeCardAddress(value)),
             Response::Long(_) => Err(Error::Response),
         }
     }
@@ -388,10 +396,7 @@ impl SdCardClient {
             })
             .await?
         {
-            Response::Short { value, .. } => {
-                tracing::trace!("CMD7 response: {value:#x}");
-                Ok(CardStatus(value))
-            }
+            Response::Short { value, .. } => Ok(CardStatus(value)),
             Response::Long(_) => Err(Error::Response),
         }
     }
@@ -423,10 +428,7 @@ impl SdCardClient {
             })
             .await?
         {
-            Response::Short { value, .. } => {
-                tracing::trace!("ACMD6 response: {value:#x}");
-                Ok(CardStatus(value))
-            }
+            Response::Short { value, .. } => Ok(CardStatus(value)),
             Response::Long(_) => Err(Error::Response),
         }
     }
@@ -456,12 +458,8 @@ impl SdCardClient {
             .await?
         {
             Response::Short {
-                value,
-                data: Some(res),
-            } => {
-                tracing::trace!("CMD18 response: {value:#x}");
-                Ok(res)
-            }
+                data: Some(res), ..
+            } => Ok(res),
             _ => Err(Error::Response),
         }
     }

From 091961b6e5046481ff9ce7d48f84f2dee5cecfa4 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Fri, 8 Dec 2023 20:43:06 +0100
Subject: [PATCH 37/38] 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<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(),
diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index 10a03ab4..b1eca5d5 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<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(|| Ok(()), |msg| msg.fmt(f))
+    }
+}
+
 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)),
         }
     }
 }

From 3649feda17f44204bd2441b4e709744967fc9a81 Mon Sep 17 00:00:00 2001
From: Jonas Spanoghe <jonas@spanoghe.net>
Date: Sun, 10 Dec 2023 11:11:46 +0100
Subject: [PATCH 38/38] sdmmc: small style improvements

---
 source/kernel/src/services/sdmmc.rs | 36 ++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/source/kernel/src/services/sdmmc.rs b/source/kernel/src/services/sdmmc.rs
index b1eca5d5..1128811a 100644
--- a/source/kernel/src/services/sdmmc.rs
+++ b/source/kernel/src/services/sdmmc.rs
@@ -185,12 +185,12 @@ 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 "),
+            ErrorKind::Busy => f.write_str("busy "),
+            ErrorKind::Response => f.write_str("response "),
+            ErrorKind::Data => f.write_str("data "),
+            ErrorKind::Buffer => f.write_str("buffer "),
+            ErrorKind::Timeout => f.write_str("timeout "),
+            ErrorKind::Other => f.write_str("other "),
         }?;
 
         self.message.map_or_else(|| Ok(()), |msg| msg.fmt(f))
@@ -200,8 +200,8 @@ impl core::fmt::Display for Error {
 impl core::fmt::Debug for Response {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         match self {
-            Response::Short { value, data: _ } => write!(f, "{:#x}", value),
-            Response::Long(value) => write!(f, "{:#x}", value),
+            Response::Short { value, data: _ } => write!(f, "{value:#x}"),
+            Response::Long(value) => write!(f, "{value:#x}"),
         }
     }
 }
@@ -290,11 +290,14 @@ impl SdCardClient {
             .request_oneshot(cmd, &self.reply)
             .await
             .map_err(|error| {
-                tracing::warn!(?error, "failed to send request to SD/MMC service");
+                tracing::warn!(
+                    ?error,
+                    "failed to send CMD{index} request to SD/MMC service"
+                );
                 Error::from(ErrorKind::Other) // TODO
             })
             .and_then(|resp| resp.body);
-        tracing::trace!("CMD{} response: {:?}", index, result);
+        tracing::trace!("CMD{index} response: {result:?}");
         result
     }
 
@@ -314,13 +317,15 @@ impl SdCardClient {
         const OCR_NBUSY: u32 = 0x80000000;
         /// Valid bits for voltage setting
         const OCR_VOLTAGE_MASK: u32 = 0x007FFF80;
+        // Set 'voltage supplied' to 2.7-3.6V and 'check pattern' to 0xAA
+        const CMD8_ARG: u32 = 0x1AA;
 
         let mut card_type = CardType::SD1;
 
         match self
             .cmd(Command {
                 index: 8,
-                argument: 0x1AA,
+                argument: CMD8_ARG,
                 options: HardwareOptions::None,
                 kind: CommandKind::Control,
                 rsp_type: ResponseType::Short,
@@ -330,7 +335,7 @@ impl SdCardClient {
             .await?
         {
             Response::Short { value, .. } => {
-                if value == 0x1AA {
+                if value == CMD8_ARG {
                     card_type = CardType::SD2;
                 }
             }
@@ -483,8 +488,11 @@ impl SdCardClient {
         blocks: usize,
         buf: FixedVec<u8>,
     ) -> Result<FixedVec<u8>, Error> {
+        const BLOCK_SIZE: usize = 512;
+
+        let bytes = BLOCK_SIZE * blocks;
         // The provider buffer should have space for the requested amount of data
-        if buf.capacity() < 512 * blocks {
+        if buf.capacity() < bytes {
             return Err(Error::from(ErrorKind::Buffer));
         }
 
@@ -493,7 +501,7 @@ impl SdCardClient {
                 index: 18,
                 argument: sector,
                 options: HardwareOptions::None,
-                kind: CommandKind::Read(512 * blocks),
+                kind: CommandKind::Read(bytes),
                 rsp_type: ResponseType::Short,
                 rsp_crc: true,
                 buffer: Some(buf),