drivers: sdio: Support SDIO driver for STM32.

[ExaltZephyr]

This PR introduces support for the SDHC driver on STM32, enabling functionality APIs for SDHC host controllers.
We used Portenta H7 but we faced some issues with end-to-end testing with airoc-wifi module, which will be addressed later.

Supported commands

• SD_GO_IDLE_STATE
• SDIO_SEND_OP_COND
• SD_SEND_RELATIVE_ADDR
• SD_SELECT_CARD
• SD_VOL_SWITCH (could not test on Portenta)
• SDIO_RW_DIRECT
• SDIO_RW_EXTENDED

Supported Modes

• polling mode

Known Issues

• During card identification process, specifically when issuing cmd opcode#3 asking for card relative address, both zephyr subsys and HAL shift the relative address coming from the response.
• ✔️ this issue has been addressed in this driver "sdhc_stm32"

-HAL_SDIO_Init currently failing if we skip the card identification sequence, a fix from HAL is needed.
✔️ this issue has been addressed in this PR #281.

• Integration with the Airoc driver for Murata 1DX .

[pillo79]

First of all, thanks for this! Tried building the new sample, but it fails with the error

devicetree error: 'disk-name' is marked as required in 'properties:' in zephyr/dts/bindings/sdhc/st,stm32-sdhc.yaml, but does not appear in <Node /soc/sdmmc@52007000 in 'zephyr/misc/empty_file.c'>

(AFAICS CI did not build it).

I think this is caused by a very recently merged commit (f9e5eeb from #89086) being included. The name can be defined of course, but I am not sure if this has additional implications.

[ExaltZephyr]

First of all, thanks for this! Tried building the new sample, but it fails with the error

devicetree error: 'disk-name' is marked as required in 'properties:' in zephyr/dts/bindings/sdhc/st,stm32-sdhc.yaml, but does not appear in <Node /soc/sdmmc@52007000 in 'zephyr/misc/empty_file.c'>

(AFAICS CI did not build it).

I think this is caused by a very recently merged commit (f9e5eeb from #89086) being included. The name can be defined of course, but I am not sure if this has additional implications.

I see, will push a fix now

[ExaltZephyr]

card identification stack
@facchinm
ok, we can skip the card identification sequence in HAL_SDIO_Init, which may require updating HAL implementation (in a new PR).
regarding second issue, can you please attach logs/output. Do you know which step has timed out?

[facchinm]

@ExaltZephyr I think it's better if you try to reproduce independently, the log is not very informative and probably most of the readings are failing anyway even before the timeout since I get 0 from the chip identification routine.

If you need it, here's the patch I'm using to bypass the missing CMD8.

diff --git a/subsys/sd/sd.c b/subsys/sd/sd.c
index d8c28914380..2bc70fa6624 100644
--- a/subsys/sd/sd.c
+++ b/subsys/sd/sd.c
@@ -58,6 +58,9 @@ static int sd_send_interface_condition(struct sd_card *card)
        cmd.retries = CONFIG_SD_CMD_RETRIES;
        cmd.timeout_ms = CONFIG_SD_CMD_TIMEOUT;
        ret = sdhc_request(card->sdhc, &cmd, NULL);
+       if (ret == -ENOTSUP) {
+               return ret;
+       }
        if (ret) {
                LOG_DBG("SD CMD8 failed with error %d", ret);
                /* Retry */
@@ -200,7 +202,8 @@ static int sd_command_init(struct sd_card *card)
         * specification and expect something like this too.
         */
        ret = sd_common_init(card);
-       if (ret) {
+       if (ret && (ret != -ENOTSUP)) {
                return ret;
        }
 
diff --git a/subsys/sd/sdio.c b/subsys/sd/sdio.c
index a81403c5f82..8ec475e7f78 100644
--- a/subsys/sd/sdio.c
+++ b/subsys/sd/sdio.c
@@ -38,6 +38,9 @@ static int sdio_send_ocr(struct sd_card *card, uint32_t ocr)
        /* Send OCR5 to initialize card */
        for (retries = 0; retries < CONFIG_SD_OCR_RETRY_COUNT; retries++) {
                ret = sdhc_request(card->sdhc, &cmd, NULL);
+               if (ret == -ENOTSUP) {
+                       return 0;
+               }
                if (ret) {
                        if (ocr == 0) {
                                /* Just probing card, likely not SDIO */
@@ -553,6 +556,10 @@ int sdio_card_init(struct sd_card *card)
        /* Probe card with SDIO OCR CM5 */
        ret = sdio_send_ocr(card, ocr_arg);
        if (ret) {
+               LOG_ERR("Card did not respond to CMD5");
+               if (ret == -ENOTSUP) {
+                       return 0;
+               }
                return ret;
        }
        /* Card responded to CMD5, type is SDIO */

[erwango]

Description edited.

[github-actions]

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_stm32	zephyrproject-rtos/hal_stm32@5cbc642	zephyrproject-rtos/hal_stm32#281	zephyrproject-rtos/hal_stm32#281/files

⛔ DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

[danieldegrasse]

@danieldegrasse Unfortunately, newly available HAL SDIO doesn't replace previous HAL SDMMC in terms of functionalities. It has not been designed to support SD card transactions use case and won't allow to replace existing Zephyr STM32 SDMMC driver. It's only meant to support SDIO Devices. So, both drivers would have to co-exist (unless the situation evolves, but there are no plans atm).

I have concerns about this moving forwards without any plan to port the STM32 SDMMC driver. We are already in a situation where users or developers can get confused by the existence of the STM32 disk driver, and assume that is a supported method for using SD devices in Zephyr. If we also have an SDIO only driver for STM32, I worry that users are going to be more confused. There is also the issue that ST's SDIO layer appears to duplicate some of the SDIO initialization functionality that should be the responsibility of the SDIO stack- which is not how SDHC drivers should function.

How difficult would it be (realistically) to use the SDIO layer with SDMMC? If it isn't possible I really feel that this driver needs to be implemented bare metal- we should not have stack-specific code in SDHC drivers

[ExaltZephyr]

Description edited.

[ExaltZephyr]

@ExaltZephyr @erwango first of all, thanks for the contribution; I'm currently trying the integration with the Airoc driver for Murata 1DX (currently on Giga but should apply to any board). Here's the snippet of the dts modifications and configs I'm enabling

CONFIG_WIFI=y
CONFIG_SDHC=y
CONFIG_SDIO_STACK=y

diff --git a/boards/arduino/giga_r1/Kconfig.defconfig b/boards/arduino/giga_r1/Kconfig.defconfig
index 0d20cf3c341..cad3ab9c8d1 100644
--- a/boards/arduino/giga_r1/Kconfig.defconfig
+++ b/boards/arduino/giga_r1/Kconfig.defconfig
@@ -3,7 +3,7 @@
 
 if BOARD_ARDUINO_GIGA_R1
 
-if BT
+if BT || WIFI
 
 choice AIROC_PART
        default CYW4343W
diff --git a/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts b/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts
index 1d5df851e53..9cb6795ec83 100644
--- a/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts
+++ b/boards/arduino/giga_r1/arduino_giga_r1_stm32h747xx_m7.dts
@@ -169,11 +169,35 @@
 
                slot0_partition: partition@40000 {
                        label = "image-0";
-                       reg = <0x40000 0x000c0000>;
+                       reg = <0x40000 0x001c0000>;
                };
        };
 };
 
+sdhc: &sdmmc1 {
+       compatible = "st,stm32-sdhc";
+       interrupts = <49 0>;
+       interrupt-names = "event";
+       hw-flow-control;
+       bus-width = <4>;
+       clk-div = <1>;
+       pinctrl-0 = <&sdmmc1_d0_pc8 &sdmmc1_d1_pc9
+                       &sdmmc1_d2_pc10 &sdmmc1_d3_pc11
+                       &sdmmc1_ck_pc12 &sdmmc1_cmd_pd2>;
+       pinctrl-names = "default";
+       status= "okay";
+       sdhi-on-gpios    = <&gpiob 10 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>;
+
+       airoc-wifi {
+               compatible = "infineon,airoc-wifi";
+               status = "okay";
+
+               /* Wi-Fi control gpios */
+               /* wifi-reg-on-gpios    = <&gpiob 10 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; */
+               wifi-host-wake-gpios = <&gpioi 8 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>;
+       };
+};

(partition size increase is needed to accommodate the 4343W firmware)

To echo @danieldegrasse , the current call to HAL_SDIO_Init makes the driver only capable of performing CMD52/CMD53 , since it overrides all the card identification stack already in place in zephyr. If I bypass that section, the driver breaks very soon with an SDIO timeout.

For reference, the previous version(s) of the driver that we wrote for mbed was heavily patched and didn't use many of the APIs provided by ST since they were not atomic enough. Cypress is currently providing a cleaned-up version here that shows exactly which modifications are needed.

Last but not least, let's remember to invalidate the caches after direct_read/write , I've been burned by that before 🙃

We have updated both the HAL and driver code. As a result, the card identification sequence is now skipped during HAL_SDIO_Init.

[ExaltZephyr]

@ExaltZephyr I think it's better if you try to reproduce independently, the log is not very informative and probably most of the readings are failing anyway even before the timeout since I get 0 from the chip identification routine.

If you need it, here's the patch I'm using to bypass the missing CMD8.

diff --git a/subsys/sd/sd.c b/subsys/sd/sd.c
index d8c28914380..2bc70fa6624 100644
--- a/subsys/sd/sd.c
+++ b/subsys/sd/sd.c
@@ -58,6 +58,9 @@ static int sd_send_interface_condition(struct sd_card *card)
        cmd.retries = CONFIG_SD_CMD_RETRIES;
        cmd.timeout_ms = CONFIG_SD_CMD_TIMEOUT;
        ret = sdhc_request(card->sdhc, &cmd, NULL);
+       if (ret == -ENOTSUP) {
+               return ret;
+       }
        if (ret) {
                LOG_DBG("SD CMD8 failed with error %d", ret);
                /* Retry */
@@ -200,7 +202,8 @@ static int sd_command_init(struct sd_card *card)
         * specification and expect something like this too.
         */
        ret = sd_common_init(card);
-       if (ret) {
+       if (ret && (ret != -ENOTSUP)) {
                return ret;
        }
 
diff --git a/subsys/sd/sdio.c b/subsys/sd/sdio.c
index a81403c5f82..8ec475e7f78 100644
--- a/subsys/sd/sdio.c
+++ b/subsys/sd/sdio.c
@@ -38,6 +38,9 @@ static int sdio_send_ocr(struct sd_card *card, uint32_t ocr)
        /* Send OCR5 to initialize card */
        for (retries = 0; retries < CONFIG_SD_OCR_RETRY_COUNT; retries++) {
                ret = sdhc_request(card->sdhc, &cmd, NULL);
+               if (ret == -ENOTSUP) {
+                       return 0;
+               }
                if (ret) {
                        if (ocr == 0) {
                                /* Just probing card, likely not SDIO */
@@ -553,6 +556,10 @@ int sdio_card_init(struct sd_card *card)
        /* Probe card with SDIO OCR CM5 */
        ret = sdio_send_ocr(card, ocr_arg);
        if (ret) {
+               LOG_ERR("Card did not respond to CMD5");
+               if (ret == -ENOTSUP) {
+                       return 0;
+               }
                return ret;
        }
        /* Card responded to CMD5, type is SDIO */

Yes, we were able to reproduce the issue. We have identified and could address the problem related to the chip ID (we will discuss this later). Additionally, we resolved an issue with the block size.

These fixes correct the failure of some calls; however, we are still encountering failures in subsequent calls.

[pillo79]

@ExaltZephyr any more updates? In any case, please rebase to fix the conflict, otherwise CI cannot run. Thanks!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud