drivers: spi: stm32 half duplex support #75983
Conversation
zephyrbot
requested review from
erwango,
FRASTM,
gautierg-st,
GeorgeCGV,
marwaiehm-st,
tbursztyka and
teburd
vai-rikar
force-pushed
the
drivers-spi-stm32-half-duplex-support
branch
from
09a376e to
fb33044
Compare
vai-rikar
force-pushed
the
drivers-spi-stm32-half-duplex-support
branch
from
fb33044 to
8f10035
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
I'm quite late to answer but thanks for this work, we'll review in coming weeks |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
There was a problem hiding this comment.
Sorry for very late review.
Looks sane a priori. I didn't fully completed the review, but some comments.
I have 2 requests:
- Would you mind rebasing (even if there is no conflicts), because I'd like to perform some testing on our test farm but this require to be in recent main due to recent ci changes in twister.
- Any way we can excercise this code in CI ?
vai-rikar
force-pushed
the
drivers-spi-stm32-half-duplex-support
branch
from
8f10035 to
84fdaa0
Compare
Add support for SPI_HALF_DUPLEX flag for STM32 SPI driver Signed-off-by: Riku Karjalainen <[email protected]>
vai-rikar
force-pushed
the
drivers-spi-stm32-half-duplex-support
branch
from
84fdaa0 to
dea9858
Compare
|
Thanks for the review and comments.
Rebase done and fixed the issues mentioned.
Would be great if there were tests for this in CI. I'm not sure how this should be done and unfortunately I don't have time to looking into it in the next couple of months. |
| } else if (transfer_dir == LL_SPI_HALF_DUPLEX_TX) { | ||
| spi_context_update_tx(&data->ctx, frame_size_bytes, dma_len); | ||
| } else { | ||
| spi_context_update_rx(&data->ctx, frame_size_bytes, dma_len); |
There was a problem hiding this comment.
is there any possible value for transfer_dir than LL_SPI_HALF_DUPLEX_RX
maybe with
if (transfer_dir == LL_SPI_HALF_DUPLEX_RX) {
before the else
There was a problem hiding this comment.
Currently transfer direction is set only to LL_SPI_FULL_DUPLEX, LL_SPI_HALF_DUPLEX_TX or LL_SPI_HALF_DUPLEX_RX. So LL_SPI_HALF_DUPLEX_RX is the only possibility in the else branch. I see that there are LL_SPI_SIMPLEX_TX/RX (stm32h7xx_ll_spi.h) definitions also but those are currently not used.
I can change this if you like, but there are multiple places where the if is omitted from the else branch.
|
@FRASTM are you able to actually go as far as giving a +1 to move this forward? |
|
Ping @FRASTM |
Add support for
SPI_HALF_DUPLEXflag for STM32 SPI driver. Tested with stm32u575 with and withoutCONFIG_SPI_STM32_INTERRUPT, and with DMA.Fixes #49446