kevin/linux
1
Fork 0
Code

spi: stm32: enable controller before asserting CS

Browse Source 
On the STM32F4/7, the MOSI and CLK pins float while the controller is
disabled. CS is a regular GPIO, and therefore always driven. Currently,
the controller is enabled in the transfer_one() callback, which runs
after CS is asserted.  Therefore, there is a period where the SPI pins
are floating while CS is asserted, making it possible for stray signals
to disrupt communications. An analogous problem occurs at the end of the
transfer when the controller is disabled before CS is released.

This problem can be reliably observed by enabling the pull-up (if
CPOL=0) or pull-down (if CPOL=1) on the clock pin. This will cause two
extra unintended clock edges per transfer, when the controller is
enabled and disabled.

Note that this bug is likely not present on the STM32H7, because this
driver sets the AFCNTR bit (not supported on F4/F7), which keeps the SPI
pins driven even while the controller is disabled.

Enabling/disabling the controller as part of runtime PM was suggested as
an alternative approach, but this breaks the driver on the STM32MP1 (see
[1]). The following quote from the manual may explain this:

> To restart the internal state machine properly, SPI is strongly
> suggested to be disabled and re-enabled before next transaction starts
> despite its setting is not changed.

This patch has been tested on an STM32F746 with a MAX14830 UART
expander.

[1] https://lore.kernel.org/lkml/ZXzRi_h2AMqEhMVw@dell-precision-5540/T/

Signed-off-by: Ben Wolsieffer <ben.wolsieffer@hefring.com>
Link: https://lore.kernel.org/r/20240424135237.1329001-2-ben.wolsieffer@hefring.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This commit is contained in:
Ben Wolsieffer 2024-04-24 09:52:38 -04:00 committed by Mark Brown
parent 4756fa529b
commit 52b62e7a5d
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0
1 changed files with 2 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
drivers/spi/spi-stm32.c
View File

@ -1016,10 +1016,8 @@ end_irq:
static irqreturn_t stm32fx_spi_irq_thread(int irq, void *dev_id) static irqreturn_t stm32fx_spi_irq_thread(int irq, void *dev_id)
{ {
struct spi_controller *ctrl = dev_id; struct spi_controller *ctrl = dev_id;
struct stm32_spi *spi = spi_controller_get_devdata(ctrl);
spi_finalize_current_transfer(ctrl); spi_finalize_current_transfer(ctrl);
stm32fx_spi_disable(spi);
return IRQ_HANDLED; return IRQ_HANDLED;
} }
@ -1187,6 +1185,8 @@ static int stm32_spi_prepare_msg(struct spi_controller *ctrl,
~clrb) | setb, ~clrb) | setb,
spi->base + spi->cfg->regs->cpol.reg); spi->base + spi->cfg->regs->cpol.reg);
stm32_spi_enable(spi);
spin_unlock_irqrestore(&spi->lock, flags); spin_unlock_irqrestore(&spi->lock, flags);
return 0; return 0;
@ -1204,7 +1204,6 @@ static void stm32fx_spi_dma_tx_cb(void *data)
if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) { if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX) {
spi_finalize_current_transfer(spi->ctrl); spi_finalize_current_transfer(spi->ctrl);
stm32fx_spi_disable(spi);
} }
} }
@ -1219,7 +1218,6 @@ static void stm32_spi_dma_rx_cb(void *data)
struct stm32_spi *spi = data; struct stm32_spi *spi = data;
spi_finalize_current_transfer(spi->ctrl); spi_finalize_current_transfer(spi->ctrl);
spi->cfg->disable(spi);
} }
/** /**
@ -1307,8 +1305,6 @@ static int stm32fx_spi_transfer_one_irq(struct stm32_spi *spi)
stm32_spi_set_bits(spi, STM32FX_SPI_CR2, cr2); stm32_spi_set_bits(spi, STM32FX_SPI_CR2, cr2);
stm32_spi_enable(spi);
/* starting data transfer when buffer is loaded */ /* starting data transfer when buffer is loaded */
if (spi->tx_buf) if (spi->tx_buf)
spi->cfg->write_tx(spi); spi->cfg->write_tx(spi);
@ -1345,8 +1341,6 @@ static int stm32h7_spi_transfer_one_irq(struct stm32_spi *spi)
spin_lock_irqsave(&spi->lock, flags); spin_lock_irqsave(&spi->lock, flags);
stm32_spi_enable(spi);
/* Be sure to have data in fifo before starting data transfer */ /* Be sure to have data in fifo before starting data transfer */
if (spi->tx_buf) if (spi->tx_buf)
stm32h7_spi_write_txfifo(spi); stm32h7_spi_write_txfifo(spi);
@ -1378,8 +1372,6 @@ static void stm32fx_spi_transfer_one_dma_start(struct stm32_spi *spi)
*/ */
stm32_spi_set_bits(spi, STM32FX_SPI_CR2, STM32FX_SPI_CR2_ERRIE); stm32_spi_set_bits(spi, STM32FX_SPI_CR2, STM32FX_SPI_CR2_ERRIE);
} }
stm32_spi_enable(spi);
} }
/** /**
@ -1413,8 +1405,6 @@ static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
stm32_spi_set_bits(spi, STM32H7_SPI_IER, ier); stm32_spi_set_bits(spi, STM32H7_SPI_IER, ier);
stm32_spi_enable(spi);
if (STM32_SPI_HOST_MODE(spi)) if (STM32_SPI_HOST_MODE(spi))
stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART); stm32_spi_set_bits(spi, STM32H7_SPI_CR1, STM32H7_SPI_CR1_CSTART);
} }