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

mimxrt: Add SDCard support. #7608

Merged
merged 1 commit into from Sep 10, 2021
Merged

Conversation

alphaFred
Copy link
Contributor

Added SDCard support with driver using USDHC peripheral of MIMXRT chip. SDCard interface is configured 4Bit wide with 50MHz operating frequency.

This change was possible with the untiring efforts of @robert-hh in testing, bugfixing and design decisions. Many thanks.

@alphaFred alphaFred force-pushed the mimxrt/sdcard branch 2 times, most recently from fa30318 to 6a3535a Compare August 2, 2021 20:17
@alphaFred
Copy link
Contributor Author

Looking forward to your review @dpgeorge 😃

ports/mimxrt/boards/make-pins.py Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/modules/_boot.py Outdated Show resolved Hide resolved
ports/mimxrt/modules/_boot.py Outdated Show resolved Hide resolved
ports/mimxrt/modules/_boot.py Show resolved Hide resolved
ports/mimxrt/sdcard.c Show resolved Hide resolved
@alphaFred
Copy link
Contributor Author

@dpgeorge Please let me know once you are satisfied with the change. I would like to squash and finalise the commit myself before you merge.

@robert-hh
Copy link
Contributor

@dpgeorge Is there anything we can do to support you in processing this PR. It's holding up other PRs for the family, like Ethernet and SDRAM support. Since they all change similar files, we want to submit them one-by-one, such that they can be merged easily.

robert-hh referenced this pull request Aug 20, 2021
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@alphaFred
Copy link
Contributor Author

@dpgeorge - fyi I just rebased to current master branch. No changes. Still ready for review 😄

robert-hh referenced this pull request in dpgeorge/micropython Sep 4, 2021
Signed-off-by: Damien George <damien@micropython.org>
ports/mimxrt/Makefile Outdated Show resolved Hide resolved
ports/mimxrt/Makefile Outdated Show resolved Hide resolved
ports/mimxrt/Makefile Outdated Show resolved Hide resolved
ports/mimxrt/Makefile Outdated Show resolved Hide resolved
ports/mimxrt/Makefile Outdated Show resolved Hide resolved
ports/mimxrt/sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/sdcard.c Show resolved Hide resolved
ports/mimxrt/sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/sdcard.h Outdated Show resolved Hide resolved
@dpgeorge
Copy link
Member

dpgeorge commented Sep 4, 2021

in addition to the comments above, please also rebase on the latest master

@alphaFred
Copy link
Contributor Author

@dpgeorge if you are happy with the change, let me know. I would like to squash myself.

@alphaFred
Copy link
Contributor Author

@dpgeorge we (myself and @robert-hh) ran our tests again, yesterday. The latest changes are also working as intended and all test cases are green.

ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
ports/mimxrt/machine_sdcard.c Outdated Show resolved Hide resolved
@dpgeorge
Copy link
Member

dpgeorge commented Sep 6, 2021

Apart from the last few minor comments, this would be ready to merge. So feel free to squash.

@alphaFred
Copy link
Contributor Author

@dpgeorge - fixed and squashed

@dpgeorge
Copy link
Member

dpgeorge commented Sep 7, 2021

@alphaFred one final thing: if you'd like this PR to be merged as-is with no rebase or change then please modify the commit message so it's not in the past tense.

In this case the message would be something like: mimxrt/sdcard: Implement SDCard driver. (note "Implement" not "Implemented"). This makes it consistent with other commit messages.

- Configures `PLL2->PFD0` with **198MHz** as base clock of
	`USDHCx` peripheral.
- Adds guards for SDCard related files via `MICROPY_PY_MACHINE_SDCARD`
- Adds creation of pin defines for SDCard to make-pins.py
- Adds new configuration option for SDCard peripheral pinout
        to mpconfigport.h
- Adds interrupt handling support instead of polling
- Adds support for `ADMA2` powered data transfer
- Configures SDCard to run in HS (high-speed mode) with **50MHz** only!

SDCard support is optional and requires `USDHC` peripheral.
Thus this driver is not available on `MIMXRT1010_EVK`.
SDCard support is enabled by setting `MICROPY_PY_MACHINE_SDCARD = 1`
in mpconfigboard.mk.

Signed-off-by: Philipp Ebensberger
@alphaFred alphaFred changed the title mimxrt: Added SDCard support. mimxrt: Add SDCard support. Sep 8, 2021
@robert-hh
Copy link
Contributor

Hi @dpgeorge. It looks as if you are very busy, just finding time late at night for a few short actions. All changes including the "final thing" are made, so this PR could be merged, if you find the time.

@dpgeorge dpgeorge merged commit 87f97e4 into micropython:master Sep 10, 2021
@dpgeorge
Copy link
Member

It looks as if you are very busy, just finding time late at night for a few short actions.

Yes, that's right.

All changes including the "final thing" are made,

Ah, I didn't see it was updated. Usually github sends an email notification for (force-)pushes but apparently not this time. So thanks for the note.

Now merged! Thanks for the hard work on this.

@robert-hh
Copy link
Contributor

Thank you very much. @alphaFred just submitted the next PR. I made another comment there.

@alphaFred
Copy link
Contributor Author

Also from my side many thanks. Our PR train just got rolling 😂 we were quite busy over the summer.

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

Successfully merging this pull request may close these issues.

None yet

3 participants