diff options
author | Jes B. Klinke <jbk@chromium.org> | 2023-03-14 15:55:56 -0700 |
---|---|---|
committer | Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2023-03-16 17:14:12 +0000 |
commit | 13785916328b5fbdd11fb14130ab86ee6de2b894 (patch) | |
tree | 5dc0e4652e18a2b6548b9774fd198c33fb7eada3 /chip | |
parent | 5812ebf3b5ce7a5d2314a693d0713e9ac455585a (diff) | |
download | chrome-ec-13785916328b5fbdd11fb14130ab86ee6de2b894.tar.gz |
chip/stm32: Enable asynchronous write-only SPI transactions
During my investigation into performance of SPI flashing with servo-like
devices, I stumbled upon an odd behavior. It seems that asynchronous
execution of write-only SPI transactions does not quite work on STM
chips.
For transactions that consist of writing a few bytes followed by reading
a block of data, the current spi_transaction_async() works well, in that
it first performs the write part, then waits for the DMA to finish,
before setting up the larger read part, and returning without waiting
for it to finish. Later, spi_transaction_flush() will call
spi_dma_wait() to know when the transfer has completed.
For write-only transactions, however, spi_transaction_async() blocks in
its "intermediate" spi_dma_wait(), before realizing that there is
nothing to read, meaning spi_transaction_async() in effect blocks until
the transaction is complete, and the subsequent spi_dma_wait() in
spi_transaction_flush() is superfluous.
I have found that with this very small change, spi_transaction_async()
will return soon after a write-only transaction has been initiated, and
spi_transaction_flush() will wait for its completion.
BUG=b:273601311
TEST=Flash using c2d2 and HyperDebug
Change-Id: I015f2870b1f0e561050b75618545dee6bcf04690
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4336269
Tested-by: Jes Klinke <jbk@chromium.org>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Commit-Queue: Jes Klinke <jbk@chromium.org>
Diffstat (limited to 'chip')
-rw-r--r-- | chip/stm32/spi_controller.c | 34 |
1 files changed, 29 insertions, 5 deletions
diff --git a/chip/stm32/spi_controller.c b/chip/stm32/spi_controller.c index 70e0eb3cd7..2802c35e90 100644 --- a/chip/stm32/spi_controller.c +++ b/chip/stm32/spi_controller.c @@ -371,6 +371,7 @@ int spi_transaction_async(const struct spi_device_t *spi_device, spi_clear_rx_fifo(spi); + /* Initiate write part of the transaction, non-blocking. */ if (txlen) { rv = spi_dma_start(port, txdata, buf, txlen); if (rv != EC_SUCCESS) @@ -383,13 +384,20 @@ int spi_transaction_async(const struct spi_device_t *spi_device, if (full_readback) return EC_SUCCESS; - rv = spi_dma_wait(port); - if (rv != EC_SUCCESS) - goto err_free; + if (rxlen) { + /* + * If "write then read" was requested, then we have to wait for + * the write to complete, before we can initiate read. + */ + if (txlen) { + rv = spi_dma_wait(port); + if (rv != EC_SUCCESS) + goto err_free; - spi_clear_tx_fifo(spi); + spi_clear_tx_fifo(spi); + } - if (rxlen) { + /* Initiate read part of the transaction, non-blocking. */ rv = spi_dma_start(port, buf, rxdata, rxlen); if (rv != EC_SUCCESS) goto err_free; @@ -398,6 +406,13 @@ int spi_transaction_async(const struct spi_device_t *spi_device, #endif } + /* + * At this point, there is EITHER a pending non-blocking write OR a + * pending non-blocking read. In either case, spi_transaction_flush() + * will wait for completion of the DMA transfer, and also make sure + * that any last bits in the transmit shift register is flushed. + */ + err_free: #ifndef CONFIG_SPI_HALFDUPLEX if (!full_readback) @@ -409,6 +424,15 @@ err_free: int spi_transaction_flush(const struct spi_device_t *spi_device) { int rv = spi_dma_wait(spi_device->port); + int port = spi_device->port; + stm32_spi_regs_t *spi = SPI_REGS[port]; + + /* + * For the case that the DMA transfer just finished was for SPI + * transfer, ensure that the last bits are fully transmitted before + * releasing CS. + */ + spi_clear_tx_fifo(spi); if (!IS_ENABLED(CONFIG_USB_SPI) || !spi_chip_select_already_asserted[spi_device->port]) { |