-
Notifications
You must be signed in to change notification settings - Fork 2.1k
boards/esp32s3-usb-otg: enable SDMMC peripheral #21471
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
boards/esp32s3-usb-otg: enable SDMMC peripheral #21471
Conversation
boards/esp32s3-usb-otg/doc.txt
Outdated
@note | ||
SPI_DEV(1) is only configured if the `sdcard_spi` module is explicitly used to | ||
access the SD card in SPI mode. Otherwise, the SDMMC host is configured and | ||
used by the `periph_sdmmc` module to access the SD card (default). That is, | ||
add module `sdcard_spi` to the make variable `USEMODULE` to use the SD Card | ||
in SPI mode. |
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.
@note | |
SPI_DEV(1) is only configured if the `sdcard_spi` module is explicitly used to | |
access the SD card in SPI mode. Otherwise, the SDMMC host is configured and | |
used by the `periph_sdmmc` module to access the SD card (default). That is, | |
add module `sdcard_spi` to the make variable `USEMODULE` to use the SD Card | |
in SPI mode. | |
@note | |
SPI_DEV(1) is only configured if the `sdcard_spi` module is explicitly added to the | |
`USEMODULE` in the application's Makefile to access the SD card in SPI mode. | |
By default, the SDMMC host is configured and used by the `periph_sdmmc` | |
module to access the SD card. |
The last sentence is a bit redundant IMO.
ifeq (,$(filter sdcard_spi,$(USEMODULE))) | ||
# use mtd_sdmmc_default if sdcard_spi isn't explicitly enabled | ||
USEMODULE += mtd_sdmmc_default | ||
endif |
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.
Wouldn't this need an "else" case to include mtd_sdcard_default
?
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.
I had the else
branch in the initial commit d1774e6, but I had to remove it with commit 9960a62 because the compilation of tests/pkg/fatfs
failed because both mtd_sdmmc_default
and mtd_sdcard_default
were enabled for this application.
BOARD=esp32s3-usb-otg make -j4 -C tests/pkg/fatfs info-modules
...
mtd
mtd_sdcard
mtd_sdcard_default
mtd_sdmmc
mtd_sdmmc_default
The reason seems to be that the Makefile of this application enables mtd_sdcard
unconditionally. We should probably change this Makefile in near future. But for this PR, it works in that way and it is defined in same way for the esp32-wrover-kit
board.
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.
The same problem is given for tests/pkg/fatfs_vfs
.
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.
Okay, I see. I'd feel comfortable merging this intermediate solution if you opened a second PR or issue, so that we don't lose track of it and possibly introduce unforeseen behavior.
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.
d1481be
to
e770299
Compare
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.
Again, I have no hardware to test this, but the changes look reasonable as well.
I'm sorry that this took such a tangent with the other PR, which was more work than anticipated 😅
But nevertheless, thank you for your persistence.
You can squash the fixups and hit merge once everything is ready.
I have the hardware and have tested it extensively 😉 |
e770299
to
1842b2f
Compare
Contribution description
This PR enables the
periph_sdmmc
feature for theesp32s3-usb-otg
board to use the SDMMC Host to access the SD Card instead of using the SD Card in SPI mode.Testing procedure
should still work.
Issues/PRs references
Depends on PR #21478