drivers: spi: spi_pico_pio: Implement DMA support for 4-wire operation #85807
Conversation
zephyrbot
added
area: SPI
zephyrbot
requested review from
ajf58,
soburi,
tbursztyka,
teburd,
ThreeEights and
yonsch
TerryGeng
force-pushed
the
pio_spi_dma
branch
2 times, most recently
from
df154a1 to
fa86c95
Compare
drivers/spi/spi_rpi_pico_pio.c
Outdated
| struct spi_pico_pio_config { | ||
| const struct device *piodev; | ||
| const struct pinctrl_dev_config *pin_cfg; | ||
| const bool dma_enabled; |
There was a problem hiding this comment.
Do you really need this? is dma_config.dev is non NULL it would be the same no?
(this bool is misaligning your struct thus why there is perhaps a way to not introduce it)
There was a problem hiding this comment.
Yes, I agree with you. I was just mimicking what was done in the pl022 driver, and I think your comment will apply to the pl022 driver equally well.
TerryGeng
force-pushed
the
pio_spi_dma
branch
2 times, most recently
from
6481024 to
ecc07bb
Compare
drivers/spi/spi_rpi_pico_pio.c
Outdated
| size_t chunk_len; | ||
| int err = 0; |
There was a problem hiding this comment.
| size_t chunk_len; | |
| int err = 0; | |
| int err = 0; |
drivers/spi/spi_rpi_pico_pio.c
Outdated
|
|
||
| key = k_spin_lock(&data->lock); | ||
|
|
||
| chunk_len = spi_context_max_continuous_chunk(&data->spi_ctx); |
There was a problem hiding this comment.
| chunk_len = spi_context_max_continuous_chunk(&data->spi_ctx); | |
| size_t chunk_len = spi_context_max_continuous_chunk(&data->spi_ctx); |
nit (?): C99 is 25 years old. Let's declare variables where they're required.
drivers/spi/spi_rpi_pico_pio.c
Outdated
| data->rx_count = 0; | ||
|
|
||
| pio_sm_clear_fifos(data->pio, data->pio_sm); | ||
|
|
||
| while (data->rx_count < chunk_len || data->tx_count < chunk_len) { |
There was a problem hiding this comment.
| while (data->rx_count < chunk_len || data->tx_count < chunk_len) { | |
| while ((data->rx_count < chunk_len) || (data->tx_count < chunk_len)) { |
This commit largely mirrors the approach of implementing DMA in the pl022 driver. Signed-off-by: Terry Geng <[email protected]>
…DMA. With the addition of a new overlay file that specifies DMA channels. Signed-off-by: Terry Geng <[email protected]>
There was a problem hiding this comment.
Personally, I'd do something like
#include "rpi_pico_pio.overlay"
but perhaps that's a misuse of the device tree compiler. Either way, I like this new style: I think it makes it clear that this is most the same as the former, with a bit more added.
No action required, just sharing my train of thought.
|
@tbursztyka @soburi It has been quite a while since the last activity. Would you guys help with reviewing this PR? |
| - EXTRA_CONF_FILE="overlay-rpi-pico-pio.conf" | ||
| extra_configs: | ||
| - CONFIG_SPI_RPI_PICO_PIO_DMA=y | ||
| - CONFIG_DMA=y |
There was a problem hiding this comment.
Superfluous as the option one line before selects it.
|
@TerryGeng it looks like this has merge conflicts due to changes on |
This commit largely mirrors the approach of implementing DMA in the pl022 driver.
Though #85112 is still pending, I figured out that maybe it is enough for users to only specify the DMA channels and figure out the DREQ by myself in the code. I have been working on a personal project that uses DMA with PIO so I think it is to the benefit of the community if I submit it here.
Also included here is an updated version of the spi loopback test that includes a case for PIO with DMA.