-
Notifications
You must be signed in to change notification settings - Fork 356
sdio: initial SDIO HAL implementation #3503
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
base: main
Are you sure you want to change the base?
Conversation
b7eeb4c to
2142d4d
Compare
dddf388 to
c3a7b0d
Compare
|
Running the As I get closer to a full implementation, I may need to bring in the What's strange is that running I'll keep debugging. |
|
It would be best to not block this PR on embedded-hal, as it'll likely take months to stabilize a new set of traits. You should instead demonstrate new functionality in a new crate, like That way you wouldn't have to patch around the dependency issues with multiple embedded-hal versions in the same project. |
|
To expand on @bugadani's comment, we can't merge this with git dependencies, so I would take the suggestion to split this into a separate crate and then work on upstreaming to eh proper. Feel free to iterate here in the mean time, but if you want this merged it won't be with a fork of embedded-hal. |
This sounds like a good plan. I'll bring it up in next week's meeting, and see what the rest of the HAL team thinks.
Right, I had no intention of trying to get this PR merged with the git dependency. My original thought process was to prove the implementation here and in However, the solution discussed above sounds like a better plan. We could do a Without trying to bikeshed too much, we would probably need to call it |
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
|
I added the remaining functions needed for the SDIO Not sure about the best way to approach SOC-specific methods (i.e. the One design I though of was to add a @Dominaezzz @MabezDev @bugadani thoughts? |
Uses the experimental `embedded-hal-sdmmc` crate for SD/MMC traits. See discussion for reasoning: - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)> - <esp-rs#3503 (comment)>
6ce6b2f to
69ec4b9
Compare
|
Regarding the low level stuff, see #1282 . Whilst esp-idf serves as a (mostly reliable) working reference, its design decisions (which were made for a C library) shouldn't be copied directly (into a Rust hal). Check out what the stable(izing) drivers (SPI, I2C, UART, GPIO) are doing to get an idea of what to do. Also the dev guidelines is another place to check. imo, for chip-based differences, |
Agreed, I kept the relative structure of the code the same. It should hopefully make reviewing the code easier, since the mapping from the Rust back to the
This is the approach I ended up going with, which can hopefully be extended pretty easily as more SDIO-having SOCs are added. |
f075cd5 to
bc0c396
Compare
Adds the missing `SDIO_*` GPIO signals for the ESP32-C6 device.
Refactors the `sdio::Pins` type to use the `gpio::PinGuard` +
`gpio::interconnect::{InputSignal, OutputSignal}` types instead of the
lower-level `Pin` types.
Adds DMA descriptor types based on the main DMA engine descriptors. The main difference is the bitfield layout used in the descriptor flags.
Adds the `DescriptorFlagFields` trait to generalize field accessor functions for descriptor flag types. Enables using a generic parameter for `DmaDescriptor`.
Implements the `DescriptorFlagFields` trait for the relevant types. Small refactors to handle function naming differences.
Adds some methods to the `Owner` type for better ergonomics.
Refactors the `DmaDescriptor` type to `DmaDescriptorGeneric` taking a generic parameter for the flags field. Creates convenience aliases for the concrete types used in the general + SDIO dedicated DMA descriptors.
Makes the `Preparation` type generic over the `DescriptorFlagFields` trait.
Maks `DmaLoopBuf` generic over the `DescriptorFlagFields` trait.
Makes the `DmaRxStreamingBuf` type generic over the `DescriptorFieldFlags` trait.
Makes the `DescriptorSet` type generic over the `DescriptorFlagFields` trait.
Makes the `DmaTxBuf` generic over the `DescriptorFlagFields` trait.
Makes the `DmaRxBuf` generic over the `DescriptorFlagFields` trait.
Makes the `DmaRxTxBuf` type generic over the `DescriptorFlagFields` trait.
Makes the `ChannelTx` struct generic over the `DescriptorFlagFields` trait.
Adds a generic paramter for the descriptor flags for the `ChannelRx` type.
Makes the `DmaTxBuffer` generic over the `DescriptorFlagFields` trait.
Makes the `DmaRxBuffer` trait generic over the `DescriptorFlagFields` trait.
Adds the `AtomicDmaDescriptor` type to enable safely storing DMA descriptors in static variables.
Adds the `AtomicBuffer` type to allow storing byte buffers safely in static variables.
Adds the `bitfielder` library as a more performant and ergonomic bitfield library.
Adds SDIO commands supported by ESP SDIO controllers.
Adds the SD and SPI mode `R5` response types, and the corresponding field types.
Adds `Sdio::read_command_raw` + `Sdio::write_response_raw` functions to read and write raw data on the `CMD` line. TODO: add SDIO mode implementations NOTE: these functions may be unecessary, the controller seems to handle command/response in hardware (at least in SPI mode).
Adds a split `qa-test` SPI-mode SDIO integration test. The tests are split with the intention to flash each half of the test to separate devices, allowing for concurrent execution.
e5c9931 to
bdb0c1e
Compare
Initializes the SDIO device in the integration test. Removes the `CMD5` sequence from the SDIO host integration test. TODO: add interrupt handler for SDIO interrupts
|
Rebased on the latest Cleaned up the commit history to remove The Should I use It's also unclear to me if the SDIO device sends the interrupt over the wire, e.g. in SPI mode. The |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
Provides the initial skeleton for the
embedded_hal::mmc::MmcOpsimplementation for the SDIO peripherals on the ESP32C6 microcontroller.Testing
Doc tests for now.
Integration tests via
examplesprogram is WIP.