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

Support configuring GPIO numbers for SDCARD (with SDIO/MMC and 4 data lines) on the ESP32 platform #1447

Comments

@296370493
Copy link

296370493 commented Feb 29, 2024

Description

https://github.com/nanoframework/Samples/blob/main/samples/System.IO.FileSystem/MountExample/Program.cs
SDCard mycard = new SDCard(new SDCard.SDCardMmcParameters { dataWidth = SDCard.SDDataWidth._4_bit, enableCardDetectPin = true, cardDetectPin = 21 });

I saw an example of mounting an SD card, which is applicable to ESP32, but this example is not applicable to ESP32-S3 because it is not possible to configure the GPIO numbers for data0-data3 and clock.

How to solve the problem

ESP32-S3 platform supports setting data0-data3 and clock to any GPIO number.
I hope to support configuring GPIO numbers for SDCARD (with SDIO/MMC and 4 data lines) data0-data3 and clock on the ESP32-S3 platform.

{1CDAEC80-26B5-421a-A7C9-69E63EA812F8}

Describe alternatives you've considered

No response

Aditional context

No response

@CoryCharlton
Copy link
Member

I took a look at https://github.com/espressif/esp-idf/blob/master/examples/storage/sd_card/sdmmc/README.md and it appears that we are currently hardcoding the MMC pins for the original ESP32 pins which cannot be configured.

As mentioned other ESP32 variants use the GPIO matrix for the SDMMC peripheral so we can't get away with simply using pre-processor directives here to set default values based on the ESP32 variant.

Here's two spots where we're hardcoding the pins:

https://github.com/nanoframework/nf-interpreter/blob/453bf3dee0be63e5538323f1116ca88d3e37e191/targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp#L28
https://github.com/nanoframework/nf-interpreter/blob/453bf3dee0be63e5538323f1116ca88d3e37e191/targets/ESP32/_common/Target_Windows_Storage.c#L107

Additionally it looks like we should be setting the pins and width on the slot_config as shown in the example: https://github.com/espressif/esp-idf/blob/d4cd437ede613fffacc06ac6d6c93a083829022f/examples/storage/sd_card/sdmmc/main/sd_card_example_main.c#L149

As @josesimoes mentioned in #1475 we don't want to expose platform specific concerns in "general" classes so we'll need to think this through. That being said I'm only seeing the SDCard functionality currently implemented on the ESP32 platform so I don't have any reference as to how other platforms.

My initial thought is that a new MMC value should be added to nanoFramework.Hardware.Esp32.DeviceTypes with new functions (MMC1_CLOCK, MMC1_COMMAND, MMC1_D0, MMC1_D1, MMC1_D2, MMC1_D3) added to nanoFramework.Hardware.Esp32.DeviceFunction. The hardcoded values would be replaced with the appropriate NativeGetPinFunction() calls.

@josesimoes
Copy link
Member

This requires some thinking about the feature design. So far we've been trying (and succedded) in NOT having platform specific stuff in the "general" class libraries. The platform specifics are all in the various nanoFramework.NNNN.
Worth give it some though before we decide how to implement this.

@296370493
Copy link
Author

My initial thought is that a new MMC value should be added to nanoFramework.Hardware.Esp32.DeviceTypes with new functions (MMC1_CLOCK, MMC1_COMMAND, MMC1_D0, MMC1_D1, MMC1_D2, MMC1_D3) added to nanoFramework.Hardware.Esp32.DeviceFunction. The hardcoded values would be replaced with the appropriate NativeGetPinFunction() calls.

I think this plan is great

@AdrianSoundy
Copy link
Member

Didn't even realise S3 could use alternate pins. Think thats the way to go as it aligns with what we are doing for other pins.

@josesimoes
Copy link
Member

@AdrianSoundy can you please follow up with this one?

@AdrianSoundy AdrianSoundy self-assigned this Jul 31, 2024
@AdrianSoundy
Copy link
Member

@josesimoes No problem

@dkovyazin
Copy link

@AdrianSoundy We are really looking forward to the GPIO pin configuration feature. Thanks

@AdrianSoundy
Copy link
Member

@AdrianSoundy We are really looking forward to the GPIO pin configuration feature. Thanks

Currently testing so should be available in next few days. @dkovyazin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment