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

Feature: change platform GPIO slew rate when high SWCLK frequency is requested #2065

Merged
merged 4 commits into from
Feb 10, 2025

Conversation

ALTracer
Copy link
Contributor

Detailed description

  • This is a new feature.
  • The existing problem is poor signal integrity (ringing/reflections, ground bounce) of stlinkv3 and blackpill-f4 platforms resulting in failure of initial scan on setups with poor ground connections.
  • This PR solves it by runtime-matching STM32 GPIO OSPEED to the toggle rates indicated in datasheets.

Prior changes:
stlinkv3: #1945, v1.10.0-1289-g 27cbec7
blackpill-f4: #1717, v1.10.0-500-g b9969de; and #2044, v2.0.0-rc1-3-g a3bb02e

Because STLINK/V3E onboard Nucleo-H743ZI2 outright didn't work at highest reachable frequency (above 12 MHz) at 2 MHz slew rate, I gave 25 MHz a try and it worked. That platform is able to checksum target data at about 400 KiB/s.
MiniSTM32F4x1Cx is slower, but still can hit 8.72 MHz (96/11) without significant firmware changes, and this is uncomfortably outside DS10314 ranges (C_L = 10 pF, Vdd > 2.7, F_max(IO)out = 8 MHz).
But to reduce noise at lower frequencies, I suggest a platform callback which reverts to OSPEED_2MHz when crossing some frequency boundary. There are no series resistors, no buffers, and we can't expect users to always use two good ground wires, especially when other adapters work fine with just one. Shields and carriers can help this.
I don't think STM32F1-based platforms are affected, because a) fastest SWCLK primitives (no_delay) only reach 3.6-4.0 MHz, b) datasheet indicates 2 and 10 MHz f_max(IO)out for C_L = 50 pF.

Your checklist for this pull request

Closing issues

@ALTracer ALTracer force-pushed the feature/swclk-ospeed branch from 3773ef9 to d2b9bde Compare January 29, 2025 18:29
@ALTracer ALTracer force-pushed the feature/swclk-ospeed branch from d2b9bde to 88ac09b Compare February 4, 2025 16:26
@dragonmux dragonmux added this to the v2.0 release milestone Feb 5, 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 5, 2025
@birkenfeld
Copy link
Contributor

Works for me, however I have to set SWD frequency to 2 MHz to get the low speed because the default is 3 MHz.

Could you either change the default to 2 MHz or change the cutoff to 3 MHz? People who want really high speed will have to set the frequency anyway, while people like me who expect things to "just work" will be initially just as surprised with the cutoff in this patch.

Thanks!

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.

We've got only the one comment, but otherwise this LGTM so as this has been shown to work on a couple of people's adaptors, we'll get this merged. Thank you for the contribution!

@@ -141,6 +141,11 @@ uint32_t platform_time_ms(void)
return time_ms;
}

__attribute__((weak)) void platform_ospeed_update(const uint32_t frequency)
Copy link
Member

Choose a reason for hiding this comment

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

Ideally this wouldn't be here.. but we recognise that this is the cleanest way to have the function just implemented for all platforms while still being overridable, so we'll allow it.

@dragonmux
Copy link
Member

We'll add that: despite your self-classification as a feature, because of what this changes and that it fixes a reliability issue, we are treating this as a fix for a bug.

@dragonmux dragonmux merged commit 88ac09b into blackmagic-debug:main Feb 10, 2025
36 checks passed
birkenfeld added a commit to birkenfeld/blackmagic that referenced this pull request Feb 11, 2025
which is the default bus frequency (see line 146), so that the probe
works on more targets by default (see blackmagic-debug#2065).
birkenfeld added a commit to birkenfeld/blackmagic that referenced this pull request Feb 11, 2025
so that the probe works on more targets out of the box (see blackmagic-debug#2065).
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.

3 participants