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

Add SD card support #46

Merged
merged 3 commits into from
Jun 21, 2024
Merged

Add SD card support #46

merged 3 commits into from
Jun 21, 2024

Conversation

mariobalanica
Copy link
Member

This is an sdport-based miniport driver for the two MSHC instances on
RK3588: one for SD card slot, the other for SDIO Wi-Fi.

Features:

  • Speed modes: Default, HS, SDR50, SDR104
  • Internal 32-bit scatter/gather DMA support
  • Card detection
  • Crashdump support

To-do:

  • Test eMMC support
  • SDIO (e.g. CMD53)
  • Support SD version 1 cards (they don't respond to CMD8 by design,
    which makes sdport assume it's not an SD card at all)

Known issues:

  • Plugging/unplugging the card quickly can lead to a bugcheck. This
    happens due to race conditions in sdport w.r.t. card removal and
    request completion. It ends up dereferencing a NULL pointer, which was
    supposed to be the outstanding request.

drivers/include/TraceDbg.h Outdated Show resolved Hide resolved
drivers/include/TraceDbg.h Outdated Show resolved Hide resolved
drivers/include/TraceDbg.h Outdated Show resolved Hide resolved
drivers/include/TraceDbg.h Outdated Show resolved Hide resolved
drivers/include/TraceDbg.h Outdated Show resolved Hide resolved

#pragma pack(1)

typedef struct _MSHC_IDMAC_DESCRIPTOR {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW, current "official" Microsoft opinion of typedef struct _STRUCT {...} STRUCT, *PSTRUCT; is "strongly discourage".

  • For code that needs to work on C, the recommendation is typedef struct STRUCT {...} STRUCT;.
  • For code that only needs to be C++, the recommendation is struct STRUCT {...};.
  • _STRUCT was just because the older MSVC compilers acted funny when both identifiers were the same, but this no longer seems to have any real benefit, and it conflicts with the C/C++ rule "only the compiler-provided headers should define stuff starting with double-underscore or underscore-capital".
  • Most of the time, PSTRUCT has no advantage over STRUCT*, and it has several disadvantages (you have to look for two different types, and it discourages use of const).

Copy link
Member Author

Choose a reason for hiding this comment

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

If you feel strongly about this, I can change it. I'm not a fan of PSTRUCT either, but nearly all existing MSFT driver code is written this way, and so I've been doing it just for consistency sake.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't feel strongly. You do what seems best. Consistency is a good thing too.

@mariobalanica mariobalanica force-pushed the dwcmshc-wip branch 2 times, most recently from 0ad8df5 to 25b0b46 Compare June 20, 2024 21:30
This library allows the SD/eMMC drivers to control necessary
clocks/voltages in both normal and crashdump environments.

Corresponding TF-A patch:
worproject/arm-trusted-firmware@b6c0df4

Signed-off-by: Mario Bălănică <[email protected]>
This is an sdport-based miniport driver for the two MSHC instances on
RK3588: one for SD card slot, the other for SDIO Wi-Fi.

Features:
- Speed modes: Default, HS, SDR50, SDR104
- Internal 32-bit scatter/gather DMA support
- Card detection
- Crashdump support

To-do:
- Test eMMC support
- SDIO (e.g. CMD53)
- Support SD version 1 cards (they don't respond to CMD8 by design,
which makes sdport assume it's not an SD card at all)

Known issues:
- Plugging/unplugging the card quickly can lead to a bugcheck. This
happens due to race conditions in sdport w.r.t. card removal and
request completion. It ends up dereferencing a NULL pointer, which was
supposed to be the outstanding request.

Signed-off-by: Mario Bălănică <[email protected]>
dwcmshc depends on GPIO for card detection. When booting from SD, the
GPIO driver must be loaded first.

Signed-off-by: Mario Bălănică <[email protected]>
@mariobalanica mariobalanica merged commit 7ef5269 into master Jun 21, 2024
2 checks passed
@mariobalanica mariobalanica deleted the dwcmshc-wip branch June 21, 2024 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants