Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async SD Card SPI #481

Closed
DaneSlattery opened this issue Sep 18, 2024 · 8 comments
Closed

Async SD Card SPI #481

DaneSlattery opened this issue Sep 18, 2024 · 8 comments

Comments

@DaneSlattery
Copy link
Contributor

In a project of mine I am using an SD card and an ethernet chip on the same SPI bus. I want to use the async ethernet driver, but this limitation has caught my eye from the spi.rs docs:

//! * True non-blocking async possible only when all devices attached to the SPI bus are used in async mode (i.e. calling methods xxx_async()
//! instead of their blocking xxx() counterparts)

For this, we would need to have an async driver for the spi bus, but I don't see a clear path to implementing one right now. With some guidance, I would be happy to contribute this feature.

@ivmarkov
Copy link
Collaborator

In a project of mine I am using an SD card and an ethernet chip on the same SPI bus. I want to use the async ethernet driver, but this limitation has caught my eye from the spi.rs docs:

//! * True non-blocking async possible only when all devices attached to the SPI bus are used in async mode (i.e. calling methods xxx_async()
//! instead of their blocking xxx() counterparts)

For this, we would need to have an async driver for the spi bus, but I don't see a clear path to implementing one right now. With some guidance, I would be happy to contribute this feature.

Don't worry. The async Ethernet driver internally uses the blocking SPI, and this is something we cannot change, but it is not a problem either.

The only "async"-ness of the async Ethernet driver (and for that matter, of the Wifi and possibly of the upcoming Thread one) is when dealing with events from the system event loop when the user asynchronously awaits for a driver operation to complete, where the completeness of the operation arrives on the event loop (i.e. the driver to start, stop and so on).

@ivmarkov
Copy link
Collaborator

Ditto for the SD card - if you use the higher-level APIs, like FatFS (via Rust's std::fs namespace) it is blocking, and there is no other way, because Rust's std::fs API is blocking.

Now, the only place where eventually you'll want "true" async with the SD card is the (not yet existing) lower-level sector-based API. For this API, having an async variant would be nice, because then you can layer on top some of the pure-Rust crates for e.g. FatFS or other filesystem.

The thing is, I'm not even sure it is possible to implement an async layer on top of the blocking SD card ESP-IDF driver anyway (that is, without "cheating" by running a hidden thread). So for that case, you might also need a "pure Rust" async SD card SPI driver, which can be finally layered on top of the "true" async SPI driver we have.

With that said, I'm also currently not sure how much of a use case a sector-based SD card API would have anyway, given that ESP IDF provides FATFS out of the box... and I don't imagine running any other file system on an SD card with a measly MCU...

@ivmarkov
Copy link
Collaborator

OK one final thing: the fact that the Ethernet driver is internally using SPI in a blocking way does not mean that the APIs all the way app through LwIP and up to the async-io TCP/UDP sockets & stuff you use automatically become blocking. They are truly async.

Complex, I know...

@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 18, 2024

Final, final comment:
or to put it another way - if something is having - async methods (as in the AsyncEth driver) - even if it internally somehow uses a blocking API (like the blocking ESP IDF SPI C driver), from your POV the thing still operates asynchronously. No need to worry about the internal blocking details, as the implementor of the driver should have assured, that the thing is async regardless.

@ivmarkov
Copy link
Collaborator

I think it would be useful to have a sector based SD card API for https://docs.rs/embedded-storage/latest/embedded_storage/ , and then also to build towards https://github.com/MabezDev/embedded-fatfs

Without complete research, I see: https://docs.rs/spi-memory-async/latest/spi_memory_async/index.html and https://github.com/rust-embedded-community/embedded-sdmmc-rs and https://github.com/kusstas/sdmmc-spi and https://github.com/ninjasource/embedded-sdmmc-rs/tree/add-async-support (very promising) https://github.com/peterkrull/embedded-sdmmc-rs/blob/develop-async/src/sdcard/mod.rs

Sure it can be done, but as I said I fail to see the point? The point is, there must be something async-only which does not have an ESP IDF equivalent that you really want to use on top of the ESP IDF's raw SD Card API and hence the latter needs to be async.

What would that thing be, for the case of SD cards?

@DaneSlattery
Copy link
Contributor Author

DaneSlattery commented Sep 18, 2024

In my case, I'm midway through this TODO:

impl<T> SdCardDriver<T> {
        /// Get a reference to the SD-Card native structure.
        pub fn card(&self) -> &sdmmc_card_t {
            &self.card
        }

        pub fn read_sectors(
            &mut self,
            dst: &mut [u8],
            start_sector: usize,
            sector_count: usize,
        ) -> Result<(), EspError> {
            if sector_count == 0 {
                return Ok(());
            }
            esp!(unsafe {
                sdmmc_read_sectors(
                    &mut *self.card as *mut _,
                    dst.as_mut_ptr().cast(),
                    start_sector,
                    sector_count,
                )
            })
        }

        pub fn write_sectors(
            &mut self,
            src: &[u8],
            start_sector: usize,
            sector_count: usize,
        ) -> Result<(), EspError> {
            if sector_count == 0 {
                return Ok(());
            }
            esp!(unsafe {
                sdmmc_write_sectors(
                    &mut *self.card as *mut _,
                    src.as_ptr().cast(),
                    start_sector,
                    sector_count,
                )
            })
        }

        pub fn erase_sectors(
            &mut self,
            start_sector: usize,
            sector_count: usize,
            operation: EraseOperation,
        ) -> Result<(), EspError> {
            if sector_count == 0 {
                return Ok(());
            }
            esp!(unsafe {
                sdmmc_erase_sectors(
                    &mut *self.card as *mut _,
                    start_sector,
                    sector_count,
                    operation.into(),
                )
            })
        }

        pub fn read_bytes(&mut self,) {}

        pub fn write_bytes() {}


        // TODO: Implement the SD-Card API here, i.e. read/write/erase sectors, as well as
        // read/write of arbitrary-length bytes.
    }

I don't necessarily need it to be async for this use case(more on that later). You have abated my concerns about the true "async" nature of the SPI driver, especially for Ethernet. The primary benefits I see for an async port are as you said:

Now, the only place where eventually you'll want "true" async with the SD card is the (not yet existing) lower-level sector-based API. For this API, having an async variant would be nice, because then you can layer on top some of the pure-Rust crates for e.g. FatFS or other filesystem.

but also for the opportunity to speed up the read/write process (users in the above libraries reported 8x speedup) while also allowing other processes to run concurrently (without threading).

All of that aside: my motivation to implement the read_sectors/write_sectors/read_bytes/write_bytes is two fold:

  1. I previously used embedded_svc::storage::RawStorage trait, which is supported by esp-idf-svc::nvs::EspNvs to store key-value data. I will accumulate a lot of data over a devices life time, and I probably won't have enough flash space (the ESP-IDF NVS C-library doesn't target sd/sdio/mmc ... yet). So I've added an SD card, and would like to implement embedded_svc::storage::RawStorage so that my raw key-value pairs can be written directly to the SD card.

  2. I would like to implement embedded_storage::Storage for this SdCardDriver.

Looking at the above, I worry about so many different traits that promise similar things 👀 , they seem to split the esp embedded ecosystem. Embassy on top is a whole other ball game, but that's all an aside.

@ivmarkov
Copy link
Collaborator

If you feel you need it, just open a PR and finish it! I'll surely merge.

I don't necessarily need it to be async for this use case(more on that later). You have abated my concerns about the true "async" nature of the SPI driver, especially for Ethernet. The primary benefits I see for an async port are as you said:

Now, the only place where eventually you'll want "true" async with the SD card is the (not yet existing) lower-level sector-based API. For this API, having an async variant would be nice, because then you can layer on top some of the pure-Rust crates for e.g. FatFS or other filesystem.

Sure. It is nice in that it might give us a "warm fuzzy feeling" that "more of my stack is async", but objectively speaking, we barely achieve something useful, if any. ESP IDF's FATFS is here, it works, so why bother? If you need async file IO, you can always offload these to a separate thread of your own and then use a queue between the async and sync world. Or even something as simple as smol's unblock! macro or a variation of it thereof.

but also for the opportunity to speed up the read/write process (users in the above libraries reported 8x speedup) while also allowing other processes to run concurrently (without threading).

But you already do have threads with the ESP IDF. So this 8x speedup won't apply to you at all.
For me, async on ESP IDF should be all about "less user-spawned threads", but not necessarily zero user-spawned threads.

All of that aside: my motivation to implement the read_sectors/write_sectors/read_bytes/write_bytes is two fold:

  1. I previously used embedded_svc::storage::RawStorage trait, which is supported by esp-idf-svc::nvs::EspNvs to store key-value data. I will accumulate a lot of data over a devices life time, and I probably won't have enough flash space (the ESP-IDF NVS C-library doesn't target sd/sdio/mmc ... yet). So I've added an SD card, and would like to implement embedded_svc::storage::RawStorage so that my raw key-value pairs can be written directly to the SD card.

Just use a regular Rust file or directory on the SD-card and store the "keys" there. Why do you need to go low level?

In the end all up to you of course, and whatever raw API you implement for the SD card driver, I'll merge as it enriches our crates. I'm just trying to be objective with you as to - you know - YAGNI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants