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

blackpill-f4: Enable internal weak pull-up on nRST, simplify set_val #2068

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

ALTracer
Copy link
Contributor

@ALTracer ALTracer commented Feb 2, 2025

Detailed description

  • This is a bug fix for an half-implemented feature.
  • The existing problem is floating nRST pin of blackpill-f4, which is, at least, inconsistent with other in-tree platforms.
  • The PR solves it by enabling internal weak pull-up resistor on it.

The original implementation was contributed like this in #1280 v1.8.0-702-g 02986d6 and such code snippet is correct for F1 GPIO, where resistors are part of input stage, but superflous for F4 GPIO, which retains PU/PD config across all digital modes (Input, Output, AF). I don't even see a reason to change modes or output-settings away from Output + Open-Drain + Pull-up + Lowest OSPEED. gpio_clear activates the low FET, gpio_set deactivates it, so GPIO_ACTIVE_LOW in Linux device-tree gpio bindings speak, or inverse polarity (!assert). See stlinkv3, except I omit the busy-delay, more on that later.
For platform_init, I guess neither TRST nor NRST were set up into output modes. For F4, gpio_set() during Digital Input mode does not enable internal weak pull-up, contrary to F1. The gpio_clear() during Output open-drain worked, but only when the wire was connected, and otherwise the level read back was whatever/floating. This interferes with BMD ADIv5 and Cortex-M layers logic. I can't tell for sure where exactly. Another adjacent problem is inconsistent nrst control implementation in platforms, I believe any delays should be hoisted up from platforms (volatile busy-delays) to callsites, but this is almost out of scope here.

Tested on blackpill-f411ce with no shields/carriers in default pinout. Stabilizes the scans where previously bogus romtable entries were logged. Removes "SWD scan failed!" occasions when wiring and power are otherwise correct (and target's SWJ-DP is mapped and functional etc.)

Your checklist for this pull request

Closing issues

Fixes #2062.

@dragonmux dragonmux added this to the v2.0 release milestone Feb 3, 2025
@dragonmux dragonmux added Bug Confirmed bug HwIssue Mitigation Solving or mitigating a Hardware issue in Software Foreign Host Board Non Native hardware to runing Black Magic firmware on labels Feb 3, 2025
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

This LGTM - please rebase on main and we'll get this merged. Thank you for the contribution and tidy bugfix!

@ALTracer ALTracer force-pushed the fix/blackpill-f4-nrst branch from 9394c34 to 31d30ad Compare February 4, 2025 02:57
@dragonmux dragonmux merged commit 31d30ad into blackmagic-debug:main Feb 4, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug Foreign Host Board Non Native hardware to runing Black Magic firmware on HwIssue Mitigation Solving or mitigating a Hardware issue in Software
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants