ec: spi_transaction_async() has questionable implementation details on stm32
Reported by
vpalatin@chromium.org,
Dec 18 2017
|
|
Issue description
While implementation SPI transactions for the new SPI controller in STM32H7, I have noticed a couple of weird details in spi_transaction_async() in chip/stm32/spi_master.c.
Writing them down to check them later:
1. the API being calling 'spi_transaction_async' then 'spi_transaction_flush' (effectively doing spi_dma_wait)
it seems that we might want to call the 'spi_dma_wait' in spi_transaction_async only if we are starting a second transaction.
so moving it inside the 'if (rxlen) {' clause ?
2. when we are releasing the shared memory, the SPI DMA transfer from/to it might still be on-going ?
Both of them even if they are real issues, should be harmless in the current code.
int spi_transaction_async(const struct spi_device_t *spi_device,
const uint8_t *txdata, int txlen,
uint8_t *rxdata, int rxlen)
{
int rv = EC_SUCCESS;
int port = spi_device->port;
int full_readback = 0;
stm32_spi_regs_t *spi = SPI_REGS[port];
char *buf = NULL;
#ifndef CONFIG_SPI_HALFDUPLEX
if (rxlen == SPI_READBACK_ALL) {
buf = rxdata;
full_readback = 1;
} else {
rv = shared_mem_acquire(MAX(txlen, rxlen), &buf);
if (rv != EC_SUCCESS)
return rv;
}
#endif
/* Drive SS low */
gpio_set_level(spi_device->gpio_cs, 0);
/* Clear out the FIFO. */
while (spi->sr & (STM32_SPI_SR_FRLVL | STM32_SPI_SR_RXNE))
(void) (uint8_t) spi->dr;
#ifdef CONFIG_SPI_HALFDUPLEX
/* Enable bidirection mode and select output direction */
spi->cr1 |= STM32_SPI_CR1_BIDIMODE | STM32_SPI_CR1_BIDIOE;
#endif
rv = spi_dma_start(port, txdata, buf, txlen);
if (rv != EC_SUCCESS)
goto err_free;
if (full_readback)
return EC_SUCCESS;
rv = spi_dma_wait(port); <<<<<<<<<<<<< (1)
if (rv != EC_SUCCESS)
goto err_free;
if (rxlen) {
#ifdef CONFIG_SPI_HALFDUPLEX
/* Select input direction */
spi->cr1 &= ~STM32_SPI_CR1_BIDIOE;
#endif
rv = spi_dma_start(port, buf, rxdata, rxlen);
if (rv != EC_SUCCESS)
goto err_free;
}
err_free:
#ifndef CONFIG_SPI_HALFDUPLEX
if (!full_readback)
shared_mem_release(buf); <<<<<<<<<<<<<< (2)
#endif
return rv;
}
|
|
►
Sign in to add a comment |
|