summaryrefslogtreecommitdiff
path: root/chip
diff options
context:
space:
mode:
authorJes B. Klinke <jbk@chromium.org>2023-03-14 15:55:56 -0700
committerChromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com>2023-03-16 17:14:12 +0000
commit13785916328b5fbdd11fb14130ab86ee6de2b894 (patch)
tree5dc0e4652e18a2b6548b9774fd198c33fb7eada3 /chip
parent5812ebf3b5ce7a5d2314a693d0713e9ac455585a (diff)
downloadchrome-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.c34
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]) {