Skip to content

Add optional SDIO pin pull-ups - #11754

Merged
sensei-hacker merged 2 commits into
iNavFlight:maintenance-10.xfrom
newbeedrone:fix/sdio-pullup
Aug 3, 2026
Merged

Add optional SDIO pin pull-ups#11754
sensei-hacker merged 2 commits into
iNavFlight:maintenance-10.xfrom
newbeedrone:fix/sdio-pullup

Conversation

@LYNHQQ

@LYNHQQ LYNHQQ commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Provide pull-up options for SDIO SD cards to improve compatibility robustness

@github-actions

Copy link
Copy Markdown

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

  • maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

  • maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.


This is an automated suggestion to help route contributions to the appropriate branch.

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add optional SDIO pin pull-ups for improved SD card compatibility

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Add compile-time option to enable SDIO GPIO pull-ups for SDMMC pins
• Keep default behavior as no-pull to preserve existing electrical characteristics
Diagram

graph TD
  A{"USE_SDIO_PULLUP?"} --> B["IOCFG_SDMMC"] --> C["SDIO GPIO init"] --> D["SDMMC peripheral"] --> E[("SD card")]
  F[["sdmmc_sdio_hal.c"]] --> B
  subgraph Legend
    direction LR
    _cfg{"Decision"} ~~~ _proc["Process"] ~~~ _dev[("Device")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Enable pull-ups by default
  • ➕ Improves compatibility without requiring a build-time option
  • ➖ May increase leakage/current draw or alter signal integrity on boards with external pull-ups already fitted
  • ➖ Behavior change could surprise existing targets that relied on NOPULL
2. Make pull-ups target/board-specific (defaults per target)
  • ➕ Matches hardware reality: only enable where needed
  • ➕ Avoids a global build flag across all boards
  • ➖ Requires plumbing the option through target configuration and maintaining it per board
3. Runtime-configurable pull-up setting
  • ➕ Allows end users to toggle without rebuilding firmware
  • ➖ GPIO pull configuration is typically fixed early; adding runtime re-init complexity and testing surface area

Recommendation: The current compile-time flag is a good minimal-change approach: it preserves existing defaults (NOPULL) while allowing compatibility-focused builds to opt in. If multiple targets consistently need this, consider promoting it to a per-target default to avoid bespoke build flags.

Files changed (1) +4 / -0

Enhancement (1) +4 / -0
sdmmc_sdio_hal.cAdd optional GPIO pull-up configuration for SDMMC pins +4/-0

Add optional GPIO pull-up configuration for SDMMC pins

• Introduces a USE_SDIO_PULLUP compile-time switch to configure SDMMC pins with GPIO_PULLUP instead of GPIO_NOPULL. Keeps the existing no-pull behavior as the default when the flag is not defined.

src/main/drivers/sdcard/sdmmc_sdio_hal.c

@qodo-code-review

qodo-code-review Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Clock pin pull-up enabled ✓ Resolved 🐞 Bug ☼ Reliability
Description
With USE_SDIO_PULLUP enabled, IOCFG_SDMMC becomes GPIO_PULLUP and is applied to the SDMMC CK pin as
well as CMD/DAT. This adds an unnecessary pull-up load on the push-pull clock line (extra sink
current when CLK is driven low) and may reduce clock signal margin on some boards.
Code

src/main/drivers/sdcard/sdmmc_sdio_hal.c[R77-81]

+#if defined(USE_SDIO_PULLUP)
+#define IOCFG_SDMMC       IO_CONFIG(GPIO_MODE_AF_PP, GPIO_SPEED_FREQ_VERY_HIGH, GPIO_PULLUP)
+#else
#define IOCFG_SDMMC       IO_CONFIG(GPIO_MODE_AF_PP, GPIO_SPEED_FREQ_VERY_HIGH, GPIO_NOPULL)
+#endif
Evidence
The PR changes IOCFG_SDMMC to use GPIO_PULLUP under USE_SDIO_PULLUP. That same IOCFG_SDMMC
value is then applied to the CK pin via IOConfigGPIOAF(clk, IOCFG_SDMMC, ...). The IO layer
decodes the pull bits from cfg into GPIO_InitTypeDef.Pull, so the pull-up will actually be
enabled on the CK pin when the option is on.

src/main/drivers/sdcard/sdmmc_sdio_hal.c[75-81]
src/main/drivers/sdcard/sdmmc_sdio_hal.c[259-273]
src/main/drivers/io.c[296-330]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `USE_SDIO_PULLUP` is defined, the shared `IOCFG_SDMMC` config enables `GPIO_PULLUP` and is used for **all** SDMMC pins, including the SDMMC clock (CK). The clock is driven push-pull by the peripheral, so an internal pull-up is unnecessary and adds load/current.

## Issue Context
`IOCFG_SDMMC` is used for `clk`, `cmd`, and `d0..d3` in `HAL_SD_MspInit()`.

## Fix Focus Areas
- src/main/drivers/sdcard/sdmmc_sdio_hal.c[77-81]
- src/main/drivers/sdcard/sdmmc_sdio_hal.c[259-273]

## Suggested fix
1. Keep pull-ups optional for `CMD` and `DAT0..DAT3` only.
2. Always configure `CK` with `GPIO_NOPULL`.

Example approach:
- Define `IOCFG_SDMMC_CLK` as `IO_CONFIG(GPIO_MODE_AF_PP, GPIO_SPEED_FREQ_VERY_HIGH, GPIO_NOPULL)`.
- Define `IOCFG_SDMMC_DATA` as either PULLUP or NOPULL based on `USE_SDIO_PULLUP`.
- Use `IOCFG_SDMMC_CLK` for `IOConfigGPIOAF(clk, ...)` and `IOCFG_SDMMC_DATA` for `cmd/d0..d3`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/main/drivers/sdcard/sdmmc_sdio_hal.c
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

Test firmware build ready — commit 61d0f23

Download firmware for PR #11754

243 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

@sensei-hacker sensei-hacker added this to the 10.0 milestone Aug 2, 2026
@sensei-hacker
sensei-hacker changed the base branch from master to maintenance-10.x August 2, 2026 18:05
@sensei-hacker

Copy link
Copy Markdown
Member

This looks good. Is there a target without external pull ups that it has been tested on?

@LYNHQQ

LYNHQQ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

This looks good. Is there a target without external pull ups that it has been tested on?

Yes, the HUMMINGBIRD_FC305_H7 is an SDIO target without external pull-up resistors.
During our testing on Betaflight, we found that without enabling the internal pull-ups, the SD card is frequently not recognized and fails to initialize for Blackbox logging. We noticed that both PX4 and ArduPilot utilize internal pull-ups for SDIO, which is why we wanted to add this feature.
I'll also do some further testing on it a bit later!

@sensei-hacker
sensei-hacker merged commit ff3ba30 into iNavFlight:maintenance-10.x Aug 3, 2026
23 checks passed
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