drivers: spi: Fix PIO program memory leak in RP2040 SPI driver #91343
+35
−5
Conversation
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
The Raspberry Pi Pico SPI PIO driver was leaking PIO program memory when switching between different SPI configurations. Each configuration change would allocate new PIO program space without releasing the previously loaded programs, leading to resource exhaustion after 16 configuration changes. This fix: 1. Adds tracking of loaded PIO programs in the driver data structure 2. Properly removes old PIO programs before loading new ones during configuration changes 3. Cleans up PIO programs when the device is released 4. Initializes program pointers to NULL Fixes the permanent failure that occurred after multiple SPI configuration changes, allowing arbitrary sequences of SPI transfers with different configurations to succeed. Fixes: #xxx
|
nzmichaelh
approved these changes
Jun 21, 2025
| @@ -779,7 +806,10 @@ int spi_pico_pio_init(const struct device *dev) | |||
| static struct spi_pico_pio_data spi_pico_pio_data_##inst = { \ | |||
| SPI_CONTEXT_INIT_LOCK(spi_pico_pio_data_##inst, spi_ctx), \ | |||
| SPI_CONTEXT_INIT_SYNC(spi_pico_pio_data_##inst, spi_ctx), \ | |||
| SPI_CONTEXT_CS_GPIOS_INITIALIZE(DT_DRV_INST(inst), spi_ctx)}; \ | |||
| SPI_CONTEXT_CS_GPIOS_INITIALIZE(DT_DRV_INST(inst), spi_ctx), \ | |||
| .loaded_tx_program = NULL, \ | |||
There was a problem hiding this comment.
This is a static struct, so you can skip initializing to NULL.
| @@ -694,6 +711,16 @@ int spi_pico_pio_release(const struct device *dev, const struct spi_config *spi_ | |||
| { | |||
| struct spi_pico_pio_data *data = dev->data; | |||
|
|
|||
| /* Clean up PIO programs when releasing the device */ | |||
There was a problem hiding this comment.
optional, as it doesn't significantly improve readability: this code is repeated so consider refactoring it out into a 'spi_pico_pio_remove_existing_programs`.
|
You'll want to remove the merge commits from this PR. |
|
Looks like this PR doesn't have any reviewers. You may want to look at the maintainers and previous authors and add them . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hi everyone!
I noticed issue #91323 about the Pico RP2040 SPI PIO driver failing after 16 configuration changes, so I decided to take a look and fix it.
What was happening
The SPI PIO driver was leaking memory every time you switched between different SPI configurations. Each time you called the driver with a different config, it would allocate new PIO program space but never clean up the old programs. After 16 switches, the PIO runs out of memory and the driver permanently fails with error -16.
The fix
I tracked down the issue to the
spi_pico_pio_configure()function. The problem was:My solution adds proper cleanup:
Changes made
loaded_tx_programandloaded_rx_programfields to track active programspio_remove_program()to clean up before loading new programsTesting
With this fix, you should be able to run the patched SPI shell test from the issue indefinitely without hitting the 16-change limit. The driver will properly manage PIO memory and won't enter that permanent error state anymore.
Let me know if you'd like me to test anything specific or if there are any concerns with the approach!
Fixes #91323