-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
esp32s3: Implement sdioio #9641
Conversation
I was just scrolling through the changes and was wondering if I should enclose all the ESP_LOGx statements withing if DEBUG compiler conditionals? |
Oh this looks interesting!!! My new storage shield has 4bit wide eMMC data on non-default IO, and folks have only been able to use it in Arduino and IDF but with this folks should be able to use CP as well! Wicked! |
Looks good but shouldn't it be enabled by default for all S3 boards? |
ESP_LOG does this automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this! I'm excited to try it out. I've added a number of suggestions to look into.
Thanks, sounds right to me, I didn't look too long at the mpconfigport settings. The PR currently sets CIRCUITPY_SDIOIO to 0 (using ?=) for all Espressif chips and then hard disables it for the following chips: esp32c2, esp32c6, esp32h2, esp32p4, esp32s2 It's currently left overridable on esp32, esp32c3, esp32s3. It looks to me like the SDMMC_HOST peripheral is supported on the esp32, esp32s3 and esp32p4, so I'll go ahead and update mpconfigport.mk to default (overridable) enabled for those boards and disabled for the rest. EDIT The esp32 SDMMC_HOST peripheral must have a different API so I'm disabling for that chip until I have a chance to look at it more closely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for adding this!
This closes #8498
@jacobcrigby created this update (Thanks so much!!!) to support sdioio for ESP32S3 and I think they planned on submitting it as a PR, however with the recent updates to the IDF I wanted to get this code integrated before the refactoring got more difficult.
Dan had mentioned that updates from #9418 would probably be needed which I've gone ahead and done. The symptoms from #9417 appear to have been resolved with those updates.
I noticed some messages in circuitpython.pot that were very similar and I thought could be factored out so I did a little message factoring as well. I'm not sure if that will cause any side effects and if so, I can remove those changes.
I discussed some testing of the original code from @jacobcrigby's github repository on both the Feather esp32s3 (1-wire and 4-wire) and the Makerfabs MaTouch 7" boards (1-wire) in #8498 but have only done basic testing using the Makerfabs board with the final code from this PR.