embedded-sdmmc via SPI broke with v0.20.2

[fko-kuptec]

We have a hardware with ATSAMD51P20A and an SD card slot here. I just realized, that the SD card functionality via embedded-sdmmc stopped working in our latest firmware. Since an old firmware version was still working, I took that old version and upgraded the hal step by step. When upgrading from 0.20.1 to 0.20.2, writing and then closing a file on the SD cards fails. That leads me to the conclusion, that something about the async changes in 0.20.2 seems to have broke or at least altered the behaviour of the synchronous SPI driver.

The following code is extracted from the firmware and should work for the Grand Central M4 board from Adafruit with the older embedded-sdmmc version 0.7.

// Define time source
struct TimeProvider;

impl embedded_sdmmc::TimeSource for TimeProvider {
    fn get_timestamp(&self) -> embedded_sdmmc::Timestamp {
        embedded_sdmmc::Timestamp {
            year_since_1970: 0,
            zero_indexed_month: 0,
            zero_indexed_day: 0,
            hours: 0,
            minutes: 0,
            seconds: 0,
        }
    }
}

// Initialize SPI for the SD card
let sercom = peripherals.sercom2;
let gclk_120mhz = clocks.gclk0();
let sercom_clock = clocks.sercom2_core(&gclk_120mhz).unwrap();
let pads = spi::Pads::default()
    .data_in(pins.pb29)
    .data_out(pins.pb26)
    .sclk(pins.pb27);
let sd_spi = spi::Config::new(&peripherals.mclk, sercom, pads, sercom_clock.freq())
    .baud(2_000_000.Hz())
    .spi_mode(spi::MODE_0)
    .enable();
let sd_cs = pins.pb28.into();

// Init SD card
let sd_spi = embedded_hal_bus::spi::ExclusiveDevice::new_no_delay(
    sd_spi,
    embedded_sdmmc::sdcard::DummyCsPin,
)
.unwrap();
let sdcard = embedded_sdmmc::SdCard::new(sd_spi, sd_cs, delay);
let volume_manager = embedded_sdmmc::VolumeManager::<_, _, 1, 1, 1>::new_with_limits(
    sdcard,
    TimeProvider,
    0,
);

// Log to the SD card
let mut volume = volume_manager
    .open_volume(embedded_sdmmc::VolumeIdx(0))
    .unwrap();
let mut root = volume.open_root_dir().unwrap();
let mut file = root
    .open_file_in_dir("log.txt", embedded_sdmmc::Mode::ReadWriteCreateOrAppend)
    .unwrap();
file.write(b"Hello, world!\n").unwrap();
core::mem::drop(file);

[fko-kuptec]

(Edit: Updated the code and affected version information)

[fko-kuptec]

The problematic commit is 8243da6, which removes the TX flushing. As temporary fix, I reverted the part of the commit affecting the synchronous, non-DMA SPI driver, which seems to make embedded-sdmmc work again.

[ianrrees]

I haven't yet had a chance to look closely at this, but wanted to mention that we had issues with SPI flushes in some other areas when updating to the current set of embedded_hal traits, specifically the LCD driver used for PyGamer springs to mind. PyGamer is a T1 BSP, so uses the latest HAL, and also has an SD card slot though I think our examples only read from it.

The embedded_hal traits specify the flush behaviour, both the HAL and the driver need to conform to those.

[fko-kuptec]

Sorry, that it took me almost a year to respond 😅

I had now some time to dig a little deeper into this issue. I am currently at the conclusion, that the issue is still within atsamd-hal. The implementation of embedded_hal_bus::spi::ExclusiveDevice already takes care of all the necessary flushing, but in case of our specific HAL implementation, externally applied flushing does not seem to be sufficient.

I tested a little more with the explicit flushes that were removed with 8243da6. I could remove all flushes as in this commit, except for this one:

fn write_word_by_word(&mut self, words: &[Word<C>]) -> Result<(), Error> {
    // Ignore RX buffer overflows by disabling the receiver
    self.config.as_mut().regs.rx_disable();

    for word in words {
        self.block_on_flags(Flags::DRE)?;
        unsafe {
            self.write_data(word.as_());
        }
    }
    ///////////////////
    self.flush_tx(); // <---
    ///////////////////

    // Reenable receiver only if necessary
    if D::RX_ENABLE {
        self.config.as_mut().regs.rx_enable();
    }
    Ok(())
}

When I removed this flush, the embedded-sdmmc-rs driver stopped working in my test case. Then, instead of removing this flush, I moved it after the re-enabling of the receiver, which also caused the driver to stop working.

This leads me to the conclusion, that enabling the receiver seems to have some unwanted side-effects on either the transmission or the TXC flag.