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.

Sign up for GitHub

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

Jump to bottom

Support SDMMC driver for Renesas RA8 devices #79862

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Support SDMMC driver for Renesas RA8 devices #79862

wants to merge 2 commits into from
+915 −2

Conversation

khoa-nguyen-18
Copy link

Add support for SDMMC driver running on Renesas RA8 SDMMC

  • boards: renesas: Add support for SDMMC.
  • drivers: disk: Add support for SDMMC
  • dts: arm: Add support for SDMMC.
  • dts: bindings: Add support for mmc driver.
  • tests: drivers: disk: disk_performance: Add support for RA8.
  • tests: drivers: disk: disk_access: Add support for RA8.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Oct 15, 2024 •
edited
Loading

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_renesas zephyrproject-rtos/hal_renesas@3dafd03 zephyrproject-rtos/hal_renesas#41 zephyrproject-rtos/hal_renesas#41/files

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

@zephyrbot zephyrbot added manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Oct 15, 2024
@thaoluonguw thaoluonguw marked this pull request as ready for review October 15, 2024 12:19
@thaoluonguw thaoluonguw added the platform: Renesas RA Renesas Electronics Corporation, RA label Oct 15, 2024
danieldegrasse
danieldegrasse requested changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@danieldegrasse danieldegrasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution, but this driver is implementing the disk API directly for an SD host controller. New SD host controllers need to be implemented using the SDHC API: https://docs.zephyrproject.org/latest/hardware/peripherals/sdhc.html.

Take a look at the drivers within drivers/sdhc for some examples of how this would be implemented.

@khoa-nguyen-18
drivers: sdmmc: Add sdmmc driver support for Renesas RA8 devices
f5a24da 
Add support for SDMMC driver running on Renesas RA SDMMC
- boards: renesas: Add support for SDMMC.
- drivers: disk: Add support for SDMMC
- dts: arm: Add support for SDMMC.
- dts: bindings: Add support for mmc driver.
- tests: drivers: disk: disk_performance: Add support for RA8.
- tests: drivers: disk: disk_access: Add support for RA8.

Signed-off-by: Phi Tran <phi.tran.jg@bp.renesas.com>
Signed-off-by: Khoa Nguyen <khoa.nguyen.xh@renesas.com>
@khoa-nguyen-18
manifest: Update renesas hal commit ID for SDMMC support
d3fed74 
Update hal_renesas commit ID for SDMMC support

Signed-off-by: Khoa Nguyen <khoa.nguyen.xh@renesas.com>
@khoa-nguyen-18
Copy link
Author

Thank you for the contribution, but this driver is implementing the disk API directly for an SD host controller. New SD host controllers need to be implemented using the SDHC API: https://docs.zephyrproject.org/latest/hardware/peripherals/sdhc.html.

Take a look at the drivers within drivers/sdhc for some examples of how this would be implemented.

We noticed a similar implementation for the disk driver on the SDMMC IP in drivers\disk\sdmmc_stm32.c. If we want to support SDMMC, do we have to implement it in drivers/sdhc, instead of drivers/disk?

@danieldegrasse
Copy link
Collaborator

We noticed a similar implementation for the disk driver on the SDMMC IP in drivers\disk\sdmmc_stm32.c. If we want to support SDMMC, do we have to implement it in drivers/sdhc, instead of drivers/disk?

Yes, you do- I'm sorry that the STM32 implementation made things unclear. This implementation is present from before the SDHC layer was introduced, and needs to be transitioned to use the SDHC class instead of the disk driver layer. New implementations should not be based off it

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

@danieldegrasse danieldegrasse danieldegrasse requested changes

@decsny decsny Awaiting requested review from decsny

@duynguyenxa duynguyenxa Awaiting requested review from duynguyenxa

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@KhiemNguyenT KhiemNguyenT Awaiting requested review from KhiemNguyenT

@soburi soburi Awaiting requested review from soburi

@thaoluonguw thaoluonguw Awaiting requested review from thaoluonguw

Requested changes must be addressed to merge this pull request.

Assignees

@danieldegrasse danieldegrasse

Labels
area: Disk Access DNM This PR should not be merged (Do Not Merge) manifest manifest-hal_renesas platform: Renesas RA Renesas Electronics Corporation, RA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@khoa-nguyen-18 @zephyrbot @danieldegrasse @thaoluonguw