drivers/periph_sdmmc: define a High-level SDIO/SD/MMC API and low-level SDMMC periperal driver interface #19539

gschorcht · 2023-05-02T22:11:57Z

Contribution description

This PR provides a SDIO/SD/MMC Device API (SDMMC). It implements a SD host controller driver that provides a high-level functions using a low-level SDIO/SD/MMC peripheral driver for accessing

MultiMediaCards (MMC) and Embedded MultiMediaCards (eMMC)
SD Memory Cards (SD Cards) with Standard Capacity (SDSC), High Capacity (SDHC) or Extended Capacity (SDXC).

It supports:

1-bit, 4-bit and 8-bit data bus width
Default Speed and High Speed
Auto-CLK

The SDIO/SD/MMC device API (SDMMC) is divided into two parts:

The high-level API that implements the SD Host Controller driver and allows
- to inititialize and identify different types of cards,
- to access them either blockwise or bytewise,
- to get information about the used card, and
- to send single commands or application specific commands to the card.
The low-level SDIO/SD/MMC peripheral driver implements the low-level functions required by the high-level device API. It has to be implemented for each MCU.

Limitations:

Only one card per SDIO/SD/MMC device is supported.
eMMCs specific features are not supported.
UHS-I, UHS-II and UHS-III are not supported.

Testing procedure

PR #19540, PR #19760 or PR #19786 is needed to test this PR.

Issues/PRs references

Prerequisite for PR #19540
Prerequisite for PR #19760
Prerequisite for PR #19786

maribu

I only took a brief look, but I like the API a lot. It makes the impression that the API was well thought through :)

Feel free to squash the inline nitpicks at will

drivers/include/periph/sdmmc.h

drivers/include/mtd_sdcard.h

drivers/mtd_sdcard/mtd_sdcard.c

drivers/include/sdmmc/sdmmc.h

benpicco

This looks really nice already!
How much do you think is missing to get this in?

drivers/include/sdmmc/sdmmc.h

drivers/periph_common/init.c

drivers/sdmmc/sdmmc.c

gschorcht · 2023-06-01T16:37:03Z

@maribu @benpicco Thanks for starting the review. However, the PR is intentionally marked as WIP as I will provide fundamental changes to the API to be able to handle MMCs and even multiple cards in case of MMCs in some weeks when Im back from vacation. I apologize for the inconvenience. I will let you know when the PR is ready for review. However, I will already take your comments into account when providing these changes.

gschorcht · 2023-06-09T07:50:19Z

This looks really nice already! How much do you think is missing to get this in?

In principle it works for STM32. My goal was to port the SAM0 SDHC peripheral driver as a second low-level driver to make sure the API is complete.

I'm still a bit unsure whether to provide the multiple-card-per-sdmmc-peripheral version (which is also working) or to stay with the single-card-per-sdmmc-peripheral version. ~~Furthermore, I'm a bit unsure how to deal with the sdcard_spi driver and the mtd_sdcard module.~~

gschorcht · 2023-08-07T16:25:39Z

or is there some way to share the command layer?

The long-term vision was also to use the low-level driver for SPI mode as a backend under the hood of the high-level API. This will of course require some additional modifications in the implementation, but should hopefully be possible without any changes to the API.

Whether we will do it in the end is another question.

gschorcht · 2023-08-08T05:54:27Z

@benpicco I think it's better for now to merge the PR as is, since it's already pretty large, and handle the devices in an XFA as a follow-on PR. As long as this is done before the fall release, a small change to the API shouldn't be a problem.

Therefore, I have squashed it.

maribu · 2023-08-08T06:36:35Z

You could declare the API as experimental for now in the API doc to avoid the usual deprecation cycle on API changes. Once the API is complete and stable, you can drop that experimental declaration.

gschorcht · 2023-08-08T07:11:23Z

You could declare the API as experimental for now in the API doc

I squashed this one-line change directly.

RIOT/drivers/include/sdmmc/sdmmc.h

Lines 14 to 15 in 8e9e3bf

    
            * @experimental This API is experimental and in an early state - expect 
        
            *               changes!

@maribu Thanks for this suggestion.

gschorcht · 2023-08-08T08:40:39Z

Do we have any idea why every second CI compilation or more fails due to tests/net/gcoap_fileserver? This is quite annoying.

gschorcht · 2023-08-08T08:44:04Z

Hm 🤔 I'm still thinking about to place sdmmc devices into an XFA.

By the way, I already have a working version locally that uses this XFA approach. XFAs are pretty cool. It just costs one pointer variable in ROM per SDMMC peripheral.

benpicco · 2023-08-23T16:54:48Z

bors merge

bors · 2023-08-23T20:30:30Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

benpicco · 2023-08-23T20:37:52Z

I think it makes sense to merge this first

I already have a working version locally that uses this XFA approach.

If you think this will improve things, feel free to make a follow-up PR

gschorcht · 2023-08-23T21:49:35Z

I think it makes sense to merge this first

Thanks

If you think this will improve things, feel free to make a follow-up PR

The changes on the driver are pretty small, but it will affect the board configuration in #19540, #19760 and #19786. The question is what we should do first. Providing the XFA approach first and fixing #19540, #19760 and #19786? Or merging #19540, #19760 and #19786 first and then providing the XFA approach and changing the board configuration again?

gschorcht · 2023-08-23T21:51:18Z

Probably, providing the small change for the XFA approach and fixing #19540, #19760 and #19786? makes the life easier.

benpicco · 2023-08-23T22:03:08Z

Whatever works best for you

19760: cpu/sam0_common/periph: add low-level SDMMC peripheral driver for SDHC r=benpicco a=gschorcht ### Contribution description This PR implements the low-level SDIO/SDMMC peripheral driver for SAM0 SDHC according to the definition in #19539. ### Testing procedure ``` BOARD=same54-xpro make -C tests/drivers/sdmmc ``` ``` BOARD=same54-xpro make -C tests/sys/vfs_default ``` ### Issues/PRs references ~Depends on PR #19539~ Depends on PR #19899 19942: pkg/littlefs2: bump to v2.8 r=benpicco a=benpicco Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]>

19760: cpu/sam0_common/periph: add low-level SDMMC peripheral driver for SDHC r=benpicco a=gschorcht ### Contribution description This PR implements the low-level SDIO/SDMMC peripheral driver for SAM0 SDHC according to the definition in #19539. ### Testing procedure ``` BOARD=same54-xpro make -C tests/drivers/sdmmc ``` ``` BOARD=same54-xpro make -C tests/sys/vfs_default ``` ### Issues/PRs references ~Depends on PR #19539~ Depends on PR #19899 Co-authored-by: Gunar Schorcht <[email protected]>

19760: cpu/sam0_common/periph: add low-level SDMMC peripheral driver for SDHC r=benpicco a=gschorcht ### Contribution description This PR implements the low-level SDIO/SDMMC peripheral driver for SAM0 SDHC according to the definition in #19539. ### Testing procedure ``` BOARD=same54-xpro make -C tests/drivers/sdmmc ``` ``` BOARD=same54-xpro make -C tests/sys/vfs_default ``` ### Issues/PRs references ~Depends on PR #19539~ Depends on PR #19899 19946: posix_sockets.c: Fix 2 byte int compilation errors r=benpicco a=mrdeep1 19956: cpu/esp32: fix heap definition for ESP32-S2 and ESP32-S3 r=benpicco a=gschorcht ### Contribution description For ESP32-S2 and ESP32-S3 the symbol `_heap_end` must not be used as `_eheap` for the newlibc `malloc` and function `sbrk`. `_heap_end` is used by the ESP-IDF heap implementation `esp-idf-heap` and points to the highest possible address (0x40000000) that could be used for the heap in ESP-IDF. It doesn't point to the top address of the unused SRAM area that can be used in newlibc `malloc` and function `sbrk`. Instead, the origin and the length of `dram0_0_seg` must be used to calculate the end of the heap `_eheap`. The problem only occurs for the newlibc `malloc` when the `sbrk` function is used but not for the ESP-IDF heap implementation `esp_idf_heap`. ### Testing procedure Use any ESP32-S2 or ESP32-S3 board and flash `tests/sys/malloc`, e.g. ``` CFLAGS='-DCHUNK_SIZE=16384' USEMODULE='stdio_uart' BOARD=esp32s3-pros3 make -j8 -C tests/sys/malloc flash ``` Without the PR the `nm` command will give the wrong address ``` nm -s tests/sys/malloc/bin/esp32s3-pros3/tests_malloc.elf | grep _eheap 40000000 A _eheap ``` The test will stuck, i.e. the allocation of memory stops when the top of unused SRAM is reached and the board restarts when the watchdog timer expires. With the PR it should work as expected ``` Help: Press s to start test, r to print it is ready START main(): This is RIOT! (Version: 2023.10-devel-309-g4669e) calloc(zu, zu) = 0x10000000 CHUNK_SIZE: 16384 NUMBER_OF_TESTS: 3 Allocated 16384 Bytes at 0x3fc8c4b0, total 16384 ... Allocated 16384 Bytes at 0x3fcec6f0, total 409792 ESP-ROM:esp32s3-20210327 Build:Mar 27 2021 rst:0x7 (TG0WDT_SYS_RST),boot:0x8 (SPI_FAST_FLASH_BOOT) Saved PC:0x403763e3 ``` With this PR the `nm` command should give a address in unused SRAM address space ``` nm -s tests/sys/malloc/bin/esp32s3-pros3/tests_malloc.elf | grep _eheap 3fcca000 A _eheap ``` and the test should pass. ### Issues/PRs references 19957: cpu/esp32: fix Octal SPI RAM for ESP32-S3 r=benpicco a=gschorcht ### Contribution description This PR fixes Octal SPI RAM handling for ESP32-S3. Functions that are used during the initialization of the Octal SPI RAM must reside in IRAM instead of Flash. Otherwise, the system stucks during boot once the Octal SPI RAM is enabled. The reason is that the Flash is not available during the initialization of the Octal SPI RAM and the functions that are called during that initialization can't be accessed in Flash. As a result the call of such a function leads to code that is messed up and the system crashes. The PR also includes the documentation fixe for the `esp32s3-box`. It also includes a small documentation fix regarding the SPI RAM for the `esp32s3-pros3` board. ### Testing procedure Use a board that has Octal SPI RAM and flash `tests/sys/malloc`, e.g.: ``` CFLAGS='-DCHUNK_SIZE=16384' USEMODULE='stdio_uart esp_spi_ram esp_log_startup' \ BOARD=esp32s3-box make -C tests/sys/malloc ``` Without the PR, the system stuck during boot once the information for the Octal SPI RAM is print ``` ESP-ROM:esp32s3-20210327 ... I (133) boot: Loaded app from partition at offset 0x10000 I (134) boot: Disabling RNG early entropy source... vendor id : 0x0d (AP) dev id : 0x02 (generation 3) density : 0x03 (64 Mbit) good-die : 0x01 (Pass) Latency : 0x01 (Fixed) VCC : 0x01 (3V) SRF : 0x01 (Fast Refresh) BurstType : 0x01 (Hybrid Wrap) BurstLen : 0x01 (32 Byte) Readlatency : 0x02 (10 cycles@Fixed) DriveStrength: 0x00 (1/1) ``` and the board restarts when the watchdog timer expires. With this PR, the system starts as expected. ``` ESP-ROM:esp32s3-20210327 ... I (132) boot: Loaded app from partition at offset 0x10000 I (133) boot: Disabling RNG early entropy source... vendor id : 0x0d (AP) dev id : 0x02 (generation 3) density : 0x03 (64 Mbit) good-die : 0x01 (Pass) Latency : 0x01 (Fixed) VCC : 0x01 (3V) SRF : 0x01 (Fast Refresh) BurstType : 0x01 (Hybrid Wrap) BurstLen : 0x01 (32 Byte) Readlatency : 0x02 (10 cycles@Fixed) DriveStrength: 0x00 (1/1) Found 64MBit SPI RAM device SPI RAM mode: sram 40m PSRAM initialized, cache is in normal (1-core) mode. Pro cpu up. Single core mode SPI SRAM memory test OK Initializing. RAM available for dynamic allocation: At 3FC8C150 len 00053EB0 (335 KiB): D/IRAM At 3FCE0000 len 0000EE34 (59 KiB): STACK/DRAM At 3FCF0000 len 00008000 (32 KiB): DRAM Starting ESP32x with ID: f412fafd0f8c ESP-IDF SDK Version v4.4.1 Current clocks in Hz: CPU=80000000 APB=80000000 XTAL=40000000 SLOW=150000 PRO cpu is up (single core mode, only PRO cpu is used) PRO cpu starts user code Adding pool of 8192K of external SPI memory to heap allocator Used clocks in Hz: CPU=80000000 APB=80000000 XTAL=40000000 FAST=8000000 SLOW=150000 XTAL calibration value: 3643448 Heap free: 8754851 bytes Board configuration: UART_DEV(0) txd=43 rxd=44 LED pins=[ ] BUTTONS pins=[ 0 ] Starting RIOT kernel on PRO cpu Help: Press s to start test, r to print it is ready ``` ### Issues/PRs references Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Jon Shallow <[email protected]>

benpicco · 2023-12-13T16:06:05Z

drivers/mtd_sdmmc/mtd_sdmmc.c

+    (void)sector;
+    (void)count;
+    mtd_sdmmc_t *mtd_sd = (mtd_sdmmc_t*)dev;
+    if (IS_ACTIVE(CONFIG_MTD_SDMMC_ERASE)) {


Why is this conditional?
Always just calling sdmmc_erase_blocks() works, we don't even need the special case for when mtd_write_page is implemented.

See comment #20180 (comment)

gschorcht requested review from smlng, leandrolanzieri, aabadie, MichelRottleuthner, fjmolinas, MrKevinWeiss, DipSwitch and vincent-d as code owners May 2, 2023 22:11

gschorcht force-pushed the drivers/periph_sdmmc branch from 32db3e0 to 2bd8aa7 Compare May 2, 2023 22:12

github-actions bot added Area: drivers Area: Device drivers Area: tests Area: tests and testing framework labels May 2, 2023

gschorcht mentioned this pull request May 2, 2023

cpu/stm32/periph: add low-level SDMMC peripheral driver #19540

Merged

gschorcht force-pushed the drivers/periph_sdmmc branch 2 times, most recently from 2fa729d to b7e5456 Compare May 3, 2023 14:04

gschorcht added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label May 4, 2023

maribu reviewed May 5, 2023

View reviewed changes

gschorcht force-pushed the drivers/periph_sdmmc branch from b7e5456 to 5921b43 Compare May 10, 2023 12:09

gschorcht mentioned this pull request May 12, 2023

board: add Adafruit Grand Central M4 Express support #19580

Merged

gschorcht force-pushed the drivers/periph_sdmmc branch from 5921b43 to 17e47b8 Compare May 15, 2023 12:32