Skip to content

Conversation

@FoniksFox
Copy link
Contributor

Implement Sd module using the new compile-infrastructure. Full documentation in the wiki.

It is a ST-LIB_LOW module even though it directly uses the HAL because it isn't actually a direct abstraction over a peripheral, but an implementation on top of it (the SDMMC peripheral is quite agnostic about what you do with it). Also, it would be great to implement an SDMMC module in HALAL, and refactor the Sd module to use that at some point in the future.

The module only allows for IDMA double-buffering mode, since it is the only mode we have a need for.

Not tested yet on hardware.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements the SD card module (SdDomain) using the new compile-time infrastructure pattern. The module provides an abstraction for SDMMC peripherals on STM32H7 microcontrollers, supporting IDMA double-buffering mode for efficient data transfer. The implementation is placed in ST-LIB_LOW despite using HAL directly, as it's an implementation on top of the SDMMC peripheral rather than a direct peripheral abstraction.

Key Changes

  • Added complete SdDomain implementation with compile-time configuration for SDMMC1/SDMMC2 peripherals
  • Integrated SdDomain into the compile-time build infrastructure (ST-LIB.hpp)
  • Implemented custom DMA multi-buffer read/write functions to support specific double-buffering requirements

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 22 comments.

File Description
Inc/ST-LIB_LOW/Sd/Sd.hpp New SD card module with SdDomain struct, compile-time configuration, runtime wrappers, interrupt handlers, and HAL callbacks
Inc/ST-LIB_LOW/ST-LIB_LOW.hpp Includes the new Sd.hpp header file
Inc/ST-LIB.hpp Integrates SdDomain into DomainsCtx, adds SD configuration building and initialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 84 to 85
buffer0 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1);
buffer1 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1);
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Potential DMA alignment issue. The buffer configuration uses default alignment (4 bytes for uint32_t array) but SDMMC DMA operations typically require cache line alignment (32 bytes) to avoid cache coherency issues. The MPUDomain::Buffer constructor should likely pass force_cache_alignment=true as the third parameter to ensure proper alignment for DMA operations.

Suggested change
buffer0 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1);
buffer1 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1);
buffer0 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1, true);
buffer1 = MPUDomain::Buffer<std::array<uint32_t, 512 * buffer_blocks / 4>>(MPUDomain::MemoryType::NonCached, MPUDomain::MemoryDomain::D1, true);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 18 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FoniksFox and others added 3 commits December 25, 2025 20:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@FoniksFox FoniksFox marked this pull request as ready for review December 25, 2025 20:01
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