Repository navigation
Backport of blheli pass-thru lockup and corruption (4.7) - #34598
peterbarker merged 3 commits into
Conversation
chEvtGetAndClearEvents() and chVTReset() take the kernel lock themselves, so calling them inside chSysLock() unlocked the critical section early. Use the I-class variants
palSetLineMode() does an unlocked read-modify-write of the GPIO mode registers, and palToggleLine() writes the whole output register. When two threads change pins on the same port these can corrupt each other. This was seen with BLHeli passthrough on Pixhawk6C: SPI1 restoring its SCK pin (PA5) raced with soft-serial switching PA8 to input, leaving PA8 in alternate function 0 (MCO1, a 4MHz clock) with its EXTI interrupt enabled. The resulting interrupt storm locked up the board. Add stm32_set_line_mode(), which locks the mode change, and stm32_toggle_line(), which toggles with a single BSRR write. Use them for all runtime pin changes in the main firmware (IOMCU firmware is not changed). Also lock palReadLineMode() and palLineSetPushPull()
Previous review (2026-10-03)Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting. Reviewed at head Thanks, the three AP_HAL_ChibiOS commits match upstream exactly. The retained logs show builds passing with no warnings on H7, F4, F7, F1 iomcu and L4 periph. The locking, toggle and interrupt-storm changes check out statically and in host models. One low-severity suggestion: the series also has c22cd56 'AP_BoardConfig: use atomic pin mode change for debug pins'. Without it, libraries/AP_BoardConfig/board_drivers.cpp:60 and :63 still do an unlocked palSetLineMode() on SWCLK/SWDIO from init_safety(), after the SPI threads can be changing the SCK mode on the same port A (https://github.com/ArduPilot/ardupilot/pull/34598/files#diff-b70babf7d0694ef34cd28ef37f7df80d70d126d7bf434b695182b39681a8131fR398). That is the same race this PR fixes elsewhere. It is pre-existing and a rare boot-time event, so it does not block the PR, but cherry-picking that commit as a separate AP_BoardConfig commit would complete the backport. Hardware/ESC behaviour and CI (pending) were not verified. |
tridge
changed the title
fc76bc6 to
b3e25fb
Compare
Previous review (2026-10-09)Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Reviewed at b3e25fb. The head moved from fc76bc6, so this review covers only the new head. The interrupt-storm commit was dropped to match the merged upstream series. The two remaining AP_HAL_ChibiOS commits match upstream 52dc853 and c6a5201 exactly. The lock nesting in the soft-serial cleanup, the locked mode helper and the BSRR toggle check out (https://github.com/ArduPilot/ardupilot/pull/34598/files#diff-b70babf7d0694ef34cd28ef37f7df80d70d126d7bf434b695182b39681a8131fR396). Builds for H7, F4, F1, L4 periph and iomcu pass with no warnings. One low, nonblocking suggestion, still open from last time: please also cherry-pick upstream 2a31fa5 'AP_BoardConfig: use atomic pin mode change for debug pins' as a separate commit. Without it, libraries/AP_BoardConfig/board_drivers.cpp:60 and :63 still do an unlocked palSetLineMode() on SWCLK/SWDIO from init_safety(). The SPI threads may be changing SCK mode on the same port at that point (https://github.com/ArduPilot/ardupilot/pull/34598/files#diff-4f940ccdabe735bb649da384f16c6b1429684fa21dd1d6925a5090d4c0db593cR389). The base already has this, and it applies cleanly. Not verified: hardware/ESC behaviour without the storm guard, and full CI (the one failure seen was a Maven network error). |
board_init_debug() runs after other threads have started, so use stm32_set_line_mode() to avoid racing other pin mode changes on the same port
|
Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting. Reviewed at head Reviewed at db92224. The head moved from b3e25fb, so this covers only the new head. The one change is the new commit 'AP_BoardConfig: use atomic pin mode change for debug pins', which is an exact cherry-pick of upstream 2a31fa5. It resolves the earlier suggestion. board_init_debug() now uses the locked stm32_set_line_mode() for SWCLK/SWDIO (https://github.com/ArduPilot/ardupilot/pull/34598/files#diff-5a7a375a2c1a71b2f57bcb3962f78c9c4077eaf4945936283e6f675b71416ac0R61). The two AP_HAL_ChibiOS commits are unchanged and still match upstream. CubeOrange, MatekF405 and sitl plane builds pass with no warnings. The current CI failures look unrelated: a pip hash mismatch for pymavlink in plane-tests1b, and Parameters/TestLogDownloadLogRestart in sitltest-sub. The PR doesn't change any code built for SITL, so a rerun is advisable. Not verified: ESC/pass-through behaviour on hardware, and F1/L4/periph/iomcu builds at this head. |
fbd0a31
into
ArduPilot:ArduPilot-4.7
|
This is included in 4.7.3-beta1 |
Summary
Backport of just the bug fixes from #34597
Classification & Testing (check all that apply and add your own)
Description
The other fixes from the PR rely on the crash-dump to sdcard change, so are omitted
Tested on Pixhawk6C with AM32 settings and fw flash via blheli pass-thru