RP2350 Support - #113
Open
andyp1per wants to merge 20 commits into
Open
RP2350 Support#113andyp1per wants to merge 20 commits into
andyp1per wants to merge 20 commits into
Conversation
Upstream's RP2350 support is written against RT 8.0.0. Taking that tree wholesale would bump the kernel under every ArduPilot board, so the RP HAL and the ARMv8-M-ML-ALT core port are brought in on top of the existing RT 7.0.6 kernel instead. Nothing in either needs RT 8; what they do need is added to files the port owns - PORT_WORKING_AREA in the ALT port's chcore.h, and PORT_CORE0_BSS_SECTION / PORT_CORE1_BSS_SECTION in the ALT SMP header aliased to the existing PORT_MEM_LOCAL_COHERENT_BSSn names. ArduPilot's local patches to the replaced files are retained: chMtxForceReleaseS, PORT_IRQ_ATTRIBUTES, the CH_CFG_STACK_OVERFLOW_HOOK call sites and the 1MHz CH_CFG_ST_FREQUENCY conversions.
RP2350: order QMI timing update before XIP access
mmcSequentialWrite() framed every block as four separate transfers - prologue, data, dummy CRC, response - and then polled mmc_wait_idle() a byte at a time. On a port where an SPI transfer completes by interrupt each of those is a thread suspend and resume, which costs far more than the bytes are worth: a one byte poll is under a microsecond on the wire wrapped in a reschedule. Stage the whole frame instead and clock it in one exchange, with a busy window after the response slot so the common case of the card finishing promptly needs no polling transfer at all. The exchange is in place, so the staging buffer afterwards holds the card's replies and the response token can be read out of it. Opt-in and backwards compatible: wbuffer is NULL out of mmcObjectInit() and NULL keeps the original path, so drivers that do not supply a buffer are unaffected. hal_mmc: always poll for card idle after a block write The single exchange block write inferred completion from the last byte of the busy window: all ones meant the card had finished programming, so the poll was skipped. That reads the same whether the card has finished or has not yet pulled MISO low, and on the second case the next data token goes into a busy card. The card discards that block and reports nothing, so the write returns success with the data gone. Above this, f_write() reports success and AP_Logger advances its file offset, which is how whole 4 KB writes went missing from the middle of a log while the file kept its full length. Keep the busy window - the card usually does finish inside it, so the poll returns on its first byte and the saving is retained - but confirm it rather than assume it.
10 tasks
spi_lld_start() guarded both dmaChannelAllocI() calls with osalDbgAssert only. With debug assertions disabled, which is how flight builds are compiled, a failed allocation returns NULL and execution falls straight through to dmaChannelSetSourceX() on a null channel, leaving the peripheral in reset while the caller has no way to know it did not start. Release any partial allocation and return instead, so the driver is left with null channels for the caller to test.
RP2350 pads come out of a power-on reset isolated and stay that way until something writes the pad register. The PAL clears ISO as a side effect of writing a whole mode word, but an ADC pad never goes through palSetLineMode, and adcRPGpioInit() read-modify-writes the pad so it preserved the bit. Clear it alongside the pulls and the input buffer, matching the Pico SDK - adc_gpio_init() goes through gpio_set_function(), which ends by clearing ISO.
mmc_wait_idle() read one byte per SPI transfer. Either size is one DMA setup and one thread suspend, so a byte at a time paid that overhead sixteen times over for the same sixteen byte-times on the wire - and it did so on the path the staged block write exists to keep clear. Read MMC_BUFFER_SIZE and scan instead. Overshooting past the moment the card goes idle costs nothing: the extra bytes clock against an idle bus, and a gap before the next token is allowed. Measured over 5932 block writes on RPI_UAVFC at 22.5 MHz: the card finishes inside the in-frame busy window 91.4% of the time. Of the waits that outlast it, 87% complete within 256 us and so are resolved by the two reads that land before the first sleep. The remainder run from 512 us to 10 ms - internal erase, not overhead - so they are slept on rather than polled. The same measurement rules out folding a 4 KB write into one exchange: at an 8.6% per-block overrun rate, half of all eight-block frames would contain a block that ran long, and every one of those would have to be rewritten.
Two silent failure modes on the RP SPIv1 port, both of which corrupt a transfer without reporting anything. spi_lld_abort() was an empty stub gated on SPI_SUPPORTS_CIRCULAR, which this port declares FALSE, so a transfer abandoned on timeout had nothing to tear it down. Its DMA stayed armed and whatever the device had already clocked in stayed in the receive FIFO, where the next transfer's DMA takes those bytes ahead of its own and shifts everything it reads. Implement it for real - stop both channels, drain the receive FIFO, cycle SSE to clear the transmit side - and drop the gate, since aborting a timed out transfer is not specific to circular mode. Upstream ChibiOS master and stable_21.11.x both still carry the stub. Nothing ever read SSPRIS or wrote SSPICR either, so an SSP receive overrun was invisible: the transfer completes, the DMA moves its full count, and the caller gets a buffer shifted down by one from the discarded byte. Check and clear it in the rx service routine and count it per driver. Counters are per SPIDriver so a non-zero value is attributable to a bus.
A core's DMA channels all share one interrupt, so the priority argument to dmaChannelAllocI() cannot be per channel. It was applied only to the first allocation for a core and silently discarded for every later one, which made the result depend on driver init order. On an RP2350 with SPI0 pinned to core1 that put the IMU's transfers at whatever priority the first unrelated driver to take a channel happened to want - 6 rather than the 2 the SPI driver asked for, because an overlay claimed a channel earlier in boot. Core 0 had the same problem, with the ADC's 3 sitting where SPI wanted 2. Track the most urgent request per core and lower the vector to match, so the outcome is the same whoever allocates first.
Counters splitting a block into wire time, card busy time and the rest, plus the distribution of blocks per disk_write call. Off by default; MMC_USE_WRITE_STATS TRUE turns them on. They are what found the real cost of the write path on RP2350: the logger was syncing every 4 KB, so four fifths of everything reaching the card was single sector filesystem metadata, and the card's apparent program time was mostly the consequence of being asked for eight times too many program cycles rather than the card being slow. Two traps worth knowing before reading them. A poll is a 16 byte exchange, so mmc_wait_idle can never take zero microseconds at 1 us resolution - an earlier "no wait" counter measured nothing. And the outcome counters catch every caller of mmc_wait_idle, not only the per block one, so their proportions are usable and their absolute counts are not per block.
andyp1per
force-pushed
the
rp2350-clean-v7
branch
from
September 12, 2026 17:30
e709d68 to
63cd89e
Compare
chSysWaitSystemState() polls ch_system.state, which another core writes, but the field is not volatile. The compiler loads it once before the loop and then compares the cached value forever, so a core that arrives before chSysInit() has finished on the other core never leaves. On RP2350 core1 calls it from c1_main(). It only worked because core1's CRT0 filled a 16 KB process stack first; with a 1 KB stack core1 won the race and hung with the kernel state already running.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RP2350 support for ArduPilot's ChibiOS, including SMP across both cores. This is what ArduPilot/ardupilot#32995 (the RP2350 flight controller port) sits on, and that PR cannot merge until something like this does.
Shape of the diff
One large commit plus nineteen small ones.
RP: restore upstream RP2350 support on ArduPilot stableis 92 files and the bulk of the +22k. It brings the RP port from upstream ChibiOS onto the older stable base ArduPilot's fork tracks. It is a port-forward, not new work, and is best reviewed as "does this match upstream" rather than line by line.The other nineteen are each a single fix found while bringing a real flight controller up on the chip, and are worth reading individually:
spi_lld_abortimplemented, with receive overrun detectionWhat this changes for existing boards
Most of the diff is under
os/hal/ports/RP/**or the ARMv8-M-ML-ALT port and cannot affect an STM32 target. Three things are shared and worth attention:os/hal/src/hal_mmc_spi.chas two changes. The first writes a block as one full duplex exchange instead of four transfers, and is reached only when the driver is given a staging buffer - a board that does not supply one keeps the existing path, so this is opt-in at runtime. The second is not opt-in:mmc_wait_idlenow readsMMC_BUFFER_SIZEbytes per poll rather than one. Overshooting past the card going idle is harmless because the extra bytes clock against an idle bus, but it is a behaviour change for every board using SD over SPI, and it has only been tested on RP2350.os/hal/include/hal_usb.haddsUSB_USE_EP0_THREAD, defaulting FALSE. The RP USB LLD tests the switch, so it needs a defined default; nothing else reads it.os/rt/src/chinstances.cis guarded bydefined(RP2350) && CH_CFG_SMP_MODE == TRUE.os/hal/ports/common/ARMCMx/mpu_v8m.his a new file, so it cannot regress anything.Write-path statistics in the MMC-SPI driver are behind
MMC_USE_WRITE_STATS, default FALSE.Testing
Exercised on RP2350 hardware, on two boards: a Raspberry Pi UAV flight controller that has flown with this, and a Laurel RP2350B board on the bench. SMP, PIO peripherals, SPI, I2C, ADC, USB CDC, PWM and SD over SPI are all in use on that hardware.
Not tested: any STM32 target. The
mmc_wait_idlechange above is the one place an STM32 board could notice this PR, and someone with an SD-over-SPI STM32 board should confirm it before this merges. RP2040 files move in the port-forward commit but have not been run.