From a53f525f510de07e8c35d38ecc575cb8ea929dd9 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Sat, 18 Feb 2023 01:35:35 +0200 Subject: [PATCH] stm32/sdmmc: Fix SDIOv1 writes --- embassy-stm32/src/dma/bdma.rs | 1 + embassy-stm32/src/dma/dma.rs | 23 +++++++++++++++++- embassy-stm32/src/dma/gpdma.rs | 10 +++++++- embassy-stm32/src/dma/mod.rs | 16 +++++++++++++ embassy-stm32/src/sdmmc/mod.rs | 40 ++++++++++++++++++++++++++++++- examples/stm32f4/src/bin/sdmmc.rs | 39 ++++++++++++++++++++++++++++-- 6 files changed, 124 insertions(+), 5 deletions(-) diff --git a/embassy-stm32/src/dma/bdma.rs b/embassy-stm32/src/dma/bdma.rs index 6160b9f40..5a7a408bb 100644 --- a/embassy-stm32/src/dma/bdma.rs +++ b/embassy-stm32/src/dma/bdma.rs @@ -192,6 +192,7 @@ mod low_level_api { options.flow_ctrl == crate::dma::FlowControl::Dma, "Peripheral flow control not supported" ); + assert!(options.fifo_threshold.is_none(), "FIFO mode not supported"); let ch = dma.ch(channel_number as _); diff --git a/embassy-stm32/src/dma/dma.rs b/embassy-stm32/src/dma/dma.rs index fec60f708..385a833f7 100644 --- a/embassy-stm32/src/dma/dma.rs +++ b/embassy-stm32/src/dma/dma.rs @@ -4,7 +4,7 @@ use core::task::Waker; use embassy_cortex_m::interrupt::Priority; use embassy_sync::waitqueue::AtomicWaker; -use super::{Burst, FlowControl, Request, TransferOptions, Word, WordSize}; +use super::{Burst, FifoThreshold, FlowControl, Request, TransferOptions, Word, WordSize}; use crate::_generated::DMA_CHANNEL_COUNT; use crate::interrupt::{Interrupt, InterruptExt}; use crate::pac::dma::{regs, vals}; @@ -40,6 +40,17 @@ impl From for vals::Pfctrl { } } +impl From for vals::Fth { + fn from(value: FifoThreshold) -> Self { + match value { + FifoThreshold::Quarter => vals::Fth::QUARTER, + FifoThreshold::Half => vals::Fth::HALF, + FifoThreshold::ThreeQuarters => vals::Fth::THREEQUARTERS, + FifoThreshold::Full => vals::Fth::FULL, + } + } +} + struct ChannelState { waker: AtomicWaker, } @@ -236,6 +247,16 @@ mod low_level_api { ch.par().write_value(peri_addr as u32); ch.m0ar().write_value(mem_addr as u32); ch.ndtr().write_value(regs::Ndtr(mem_len as _)); + ch.fcr().write(|w| { + if let Some(fth) = options.fifo_threshold { + // FIFO mode + w.set_dmdis(vals::Dmdis::DISABLED); + w.set_fth(fth.into()); + } else { + // Direct mode + w.set_dmdis(vals::Dmdis::ENABLED); + } + }); ch.cr().write(|w| { w.set_dir(dir); w.set_msize(data_size); diff --git a/embassy-stm32/src/dma/gpdma.rs b/embassy-stm32/src/dma/gpdma.rs index d252cef3e..442fee48e 100644 --- a/embassy-stm32/src/dma/gpdma.rs +++ b/embassy-stm32/src/dma/gpdma.rs @@ -176,8 +176,16 @@ mod low_level_api { mem_len: usize, incr_mem: bool, data_size: WordSize, - _options: TransferOptions, + options: TransferOptions, ) { + assert!(options.mburst == crate::dma::Burst::Single, "Burst mode not supported"); + assert!(options.pburst == crate::dma::Burst::Single, "Burst mode not supported"); + assert!( + options.flow_ctrl == crate::dma::FlowControl::Dma, + "Peripheral flow control not supported" + ); + assert!(options.fifo_threshold.is_none(), "FIFO mode not supported"); + // "Preceding reads and writes cannot be moved past subsequent writes." fence(Ordering::SeqCst); diff --git a/embassy-stm32/src/dma/mod.rs b/embassy-stm32/src/dma/mod.rs index b51e0d40b..f5a82fb7a 100644 --- a/embassy-stm32/src/dma/mod.rs +++ b/embassy-stm32/src/dma/mod.rs @@ -186,6 +186,19 @@ pub enum FlowControl { Peripheral, } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum FifoThreshold { + /// 1/4 full FIFO + Quarter, + /// 1/2 full FIFO + Half, + /// 3/4 full FIFO + ThreeQuarters, + /// Full FIFO + Full, +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct TransferOptions { @@ -195,6 +208,8 @@ pub struct TransferOptions { pub mburst: Burst, /// Flow control configuration pub flow_ctrl: FlowControl, + /// FIFO threshold for DMA FIFO mode. If none, direct mode is used. + pub fifo_threshold: Option, } impl Default for TransferOptions { @@ -203,6 +218,7 @@ impl Default for TransferOptions { pburst: Burst::Single, mburst: Burst::Single, flow_ctrl: FlowControl::Dma, + fifo_threshold: None, } } } diff --git a/embassy-stm32/src/sdmmc/mod.rs b/embassy-stm32/src/sdmmc/mod.rs index 3f07e0c07..3c99a0b6c 100644 --- a/embassy-stm32/src/sdmmc/mod.rs +++ b/embassy-stm32/src/sdmmc/mod.rs @@ -2,6 +2,7 @@ use core::default::Default; use core::future::poll_fn; +use core::ops::{Deref, DerefMut}; use core::task::Poll; use embassy_hal_common::drop::OnDrop; @@ -40,7 +41,23 @@ impl Default for Signalling { } #[repr(align(4))] -pub struct DataBlock([u8; 512]); +#[derive(Debug, Clone)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct DataBlock(pub [u8; 512]); + +impl Deref for DataBlock { + type Target = [u8; 512]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for DataBlock { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} /// Errors #[non_exhaustive] @@ -570,6 +587,12 @@ impl SdmmcInner { regs.clkcr().write(|w| { w.set_pwrsav(false); w.set_negedge(false); + + // Hardware flow control is broken on SDIOv1 and causes clock glitches, which result in CRC errors. + // See chip erratas for more details. + #[cfg(sdmmc_v1)] + w.set_hwfc_en(false); + #[cfg(sdmmc_v2)] w.set_hwfc_en(true); #[cfg(sdmmc_v1)] @@ -807,10 +830,16 @@ impl SdmmcInner { let regs = self.0; let on_drop = OnDrop::new(|| unsafe { self.on_drop() }); + // sdmmc_v1 uses different cmd/dma order than v2, but only for writes + #[cfg(sdmmc_v1)] + self.cmd(Cmd::write_single_block(address), true)?; + unsafe { self.prepare_datapath_write(buffer as *const [u32; 128], 512, 9, data_transfer_timeout, dma); self.data_interrupts(true); } + + #[cfg(sdmmc_v2)] self.cmd(Cmd::write_single_block(address), true)?; let res = poll_fn(|cx| { @@ -922,7 +951,9 @@ impl SdmmcInner { let request = dma.request(); dma.start_read(request, regs.fifor().ptr() as *const u32, buffer, crate::dma::TransferOptions { pburst: crate::dma::Burst::Incr4, + mburst: crate::dma::Burst::Incr4, flow_ctrl: crate::dma::FlowControl::Peripheral, + fifo_threshold: Some(crate::dma::FifoThreshold::Full), ..Default::default() }); } else if #[cfg(sdmmc_v2)] { @@ -970,7 +1001,9 @@ impl SdmmcInner { let request = dma.request(); dma.start_write(request, buffer, regs.fifor().ptr() as *mut u32, crate::dma::TransferOptions { pburst: crate::dma::Burst::Incr4, + mburst: crate::dma::Burst::Incr4, flow_ctrl: crate::dma::FlowControl::Peripheral, + fifo_threshold: Some(crate::dma::FifoThreshold::Full), ..Default::default() }); } else if #[cfg(sdmmc_v2)] { @@ -982,6 +1015,11 @@ impl SdmmcInner { regs.dctrl().modify(|w| { w.set_dblocksize(block_size); w.set_dtdir(false); + #[cfg(sdmmc_v1)] + { + w.set_dmaen(true); + w.set_dten(true); + } }); } diff --git a/examples/stm32f4/src/bin/sdmmc.rs b/examples/stm32f4/src/bin/sdmmc.rs index 0edd8a61a..b57e955f6 100644 --- a/examples/stm32f4/src/bin/sdmmc.rs +++ b/examples/stm32f4/src/bin/sdmmc.rs @@ -4,11 +4,15 @@ use defmt::*; use embassy_executor::Spawner; -use embassy_stm32::sdmmc::Sdmmc; +use embassy_stm32::sdmmc::{DataBlock, Sdmmc}; use embassy_stm32::time::mhz; use embassy_stm32::{interrupt, Config}; use {defmt_rtt as _, panic_probe as _}; +/// This is a safeguard to not overwrite any data on the SD card. +/// If you don't care about SD card contents, set this to `true` to test writes. +const ALLOW_WRITES: bool = false; + #[embassy_executor::main] async fn main(_spawner: Spawner) -> ! { let mut config = Config::default(); @@ -34,11 +38,42 @@ async fn main(_spawner: Spawner) -> ! { // Should print 400kHz for initialization info!("Configured clock: {}", sdmmc.clock().0); - unwrap!(sdmmc.init_card(mhz(25)).await); + unwrap!(sdmmc.init_card(mhz(24)).await); let card = unwrap!(sdmmc.card()); info!("Card: {:#?}", Debug2Format(card)); + info!("Clock: {}", sdmmc.clock()); + + // Arbitrary block index + let block_idx = 16; + + // SDMMC uses `DataBlock` instead of `&[u8]` to ensure 4 byte alignment required by the hardware. + let mut block = DataBlock([0u8; 512]); + + sdmmc.read_block(block_idx, &mut block).await.unwrap(); + info!("Read: {=[u8]:X}...{=[u8]:X}", block[..8], block[512 - 8..]); + + if !ALLOW_WRITES { + info!("Writing is disabled."); + loop {} + } + + info!("Filling block with 0x55"); + block.fill(0x55); + sdmmc.write_block(block_idx, &block).await.unwrap(); + info!("Write done"); + + sdmmc.read_block(block_idx, &mut block).await.unwrap(); + info!("Read: {=[u8]:X}...{=[u8]:X}", block[..8], block[512 - 8..]); + + info!("Filling block with 0xAA"); + block.fill(0xAA); + sdmmc.write_block(block_idx, &block).await.unwrap(); + info!("Write done"); + + sdmmc.read_block(block_idx, &mut block).await.unwrap(); + info!("Read: {=[u8]:X}...{=[u8]:X}", block[..8], block[512 - 8..]); loop {} }