This series contains fixes & cleanup mainly regarding fifo
and the way end of transfer triggered, when used with or
without DMA.
An additional patch cleans up the pm_runtime calls and another
one enables the autosuspend.
v2: - split pm_runtime fix patch into two
- correct revert commit subject line
Alain Volmat (6):
spi: stm32: fixes pm_runtime calls in probe/remove
spi: stm32: enable pm_runtime autosuspend
spi: stm32h7: fix full duplex irq handler handling
spi: stm32: Revert "properly handle 0 byte transfer"
spi: stm32h7: don't wait for EOT and flush fifo on disable
spi: stm32: finalize message either on dma callback or EOT
Amelie Delaunay (1):
spi: stm32h7: rework rx fifo read function
drivers/spi/spi-stm32.c | 146 +++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 85 deletions(-)
--
2.25.1
In nominal cases, disable is called as part of the unprepare_message,
after receiving a EOT and after receiving all data so it doesn't
make sense to check for EOT and empty the FIFO.
Moreover, at the end of the disable, the SPI is disable (SPE) leading
to clear of all internal FIFO, leaving the IP in a known status.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/spi/spi-stm32.c | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index c2144e3c57eb..535f4bebc010 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -677,13 +677,12 @@ static void stm32f4_spi_disable(struct stm32_spi *spi)
* stm32h7_spi_disable - Disable SPI controller
* @spi: pointer to the spi controller data structure
*
- * RX-Fifo is flushed when SPI controller is disabled. To prevent any data
- * loss, use stm32_spi_read_rxfifo to read the remaining bytes in RX-Fifo.
+ * RX-Fifo is flushed when SPI controller is disabled.
*/
static void stm32h7_spi_disable(struct stm32_spi *spi)
{
unsigned long flags;
- u32 cr1, sr;
+ u32 cr1;
dev_dbg(spi->dev, "disable controller\n");
@@ -696,25 +695,6 @@ static void stm32h7_spi_disable(struct stm32_spi *spi)
return;
}
- /* Wait on EOT or suspend the flow */
- if (readl_relaxed_poll_timeout_atomic(spi->base + STM32H7_SPI_SR,
- sr, !(sr & STM32H7_SPI_SR_EOT),
- 10, 100000) < 0) {
- if (cr1 & STM32H7_SPI_CR1_CSTART) {
- writel_relaxed(cr1 | STM32H7_SPI_CR1_CSUSP,
- spi->base + STM32H7_SPI_CR1);
- if (readl_relaxed_poll_timeout_atomic(
- spi->base + STM32H7_SPI_SR,
- sr, !(sr & STM32H7_SPI_SR_SUSP),
- 10, 100000) < 0)
- dev_warn(spi->dev,
- "Suspend request timeout\n");
- }
- }
-
- if (!spi->cur_usedma && spi->rx_buf && (spi->rx_len > 0))
- stm32h7_spi_read_rxfifo(spi);
-
if (spi->cur_usedma && spi->dma_tx)
dmaengine_terminate_all(spi->dma_tx);
if (spi->cur_usedma && spi->dma_rx)
--
2.25.1
In case of Full-Duplex mode, DXP flag is set when RXP and TXP flags are
set. But to avoid 2 different handlings, just add TXP and RXP flag in
the mask instead of DXP, and then keep the initial handling of TXP and
RXP events.
Also rephrase comment about EOTIE which is one of the interrupt enable
bits. It is not triggered by any event.
Signed-off-by: Amelie Delaunay <[email protected]>
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/spi/spi-stm32.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index fe35c5cfb820..4dbd5cbe0c11 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -886,15 +886,18 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
ier = readl_relaxed(spi->base + STM32H7_SPI_IER);
mask = ier;
- /* EOTIE is triggered on EOT, SUSP and TXC events. */
+ /*
+ * EOTIE enables irq from EOT, SUSP and TXC events. We need to set
+ * SUSP to acknowledge it later. TXC is automatically cleared
+ */
+
mask |= STM32H7_SPI_SR_SUSP;
/*
- * When TXTF is set, DXPIE and TXPIE are cleared. So in case of
- * Full-Duplex, need to poll RXP event to know if there are remaining
- * data, before disabling SPI.
+ * DXPIE is set in Full-Duplex, one IT will be raised if TXP and RXP
+ * are set. So in case of Full-Duplex, need to poll TXP and RXP event.
*/
- if (spi->rx_buf && !spi->cur_usedma)
- mask |= STM32H7_SPI_SR_RXP;
+ if ((spi->cur_comm == SPI_FULL_DUPLEX) && !spi->cur_usedma)
+ mask |= STM32H7_SPI_SR_TXP | STM32H7_SPI_SR_RXP;
if (!(sr & mask)) {
dev_warn(spi->dev, "spurious IT (sr=0x%08x, ier=0x%08x)\n",
--
2.25.1
Depending on the usage, it is necessary to perform the finalize
message operation either upon receiving the EOT interruption,
eiher upon receiving the DMA callback. Indeed, when relying
on DMA, even if the SPI EOT IT has been received, it is
necessary to wait for the end of the DMA RX transaction before
accessing to the data.
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/spi/spi-stm32.c | 57 +++++++++++++++--------------------------
1 file changed, 20 insertions(+), 37 deletions(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 535f4bebc010..14ca7ea04e47 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -911,7 +911,10 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
if (sr & STM32H7_SPI_SR_EOT) {
if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
stm32h7_spi_read_rxfifo(spi);
- end = true;
+ if (!spi->cur_usedma ||
+ (spi->cur_usedma && (spi->cur_comm == SPI_SIMPLEX_TX ||
+ spi->cur_comm == SPI_3WIRE_TX)))
+ end = true;
}
if (sr & STM32H7_SPI_SR_TXP)
@@ -1019,42 +1022,17 @@ static void stm32f4_spi_dma_tx_cb(void *data)
}
/**
- * stm32f4_spi_dma_rx_cb - dma callback
+ * stm32_spi_dma_rx_cb - dma callback
* @data: pointer to the spi controller data structure
*
* DMA callback is called when the transfer is complete for DMA RX channel.
*/
-static void stm32f4_spi_dma_rx_cb(void *data)
+static void stm32_spi_dma_rx_cb(void *data)
{
struct stm32_spi *spi = data;
spi_finalize_current_transfer(spi->master);
- stm32f4_spi_disable(spi);
-}
-
-/**
- * stm32h7_spi_dma_cb - dma callback
- * @data: pointer to the spi controller data structure
- *
- * DMA callback is called when the transfer is complete or when an error
- * occurs. If the transfer is complete, EOT flag is raised.
- */
-static void stm32h7_spi_dma_cb(void *data)
-{
- struct stm32_spi *spi = data;
- unsigned long flags;
- u32 sr;
-
- spin_lock_irqsave(&spi->lock, flags);
-
- sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
-
- spin_unlock_irqrestore(&spi->lock, flags);
-
- if (!(sr & STM32H7_SPI_SR_EOT))
- dev_warn(spi->dev, "DMA error (sr=0x%08x)\n", sr);
-
- /* Now wait for EOT, or SUSP or OVR in case of error */
+ spi->cfg->disable(spi);
}
/**
@@ -1220,11 +1198,13 @@ static void stm32f4_spi_transfer_one_dma_start(struct stm32_spi *spi)
*/
static void stm32h7_spi_transfer_one_dma_start(struct stm32_spi *spi)
{
- /* Enable the interrupts relative to the end of transfer */
- stm32_spi_set_bits(spi, STM32H7_SPI_IER, STM32H7_SPI_IER_EOTIE |
- STM32H7_SPI_IER_TXTFIE |
- STM32H7_SPI_IER_OVRIE |
- STM32H7_SPI_IER_MODFIE);
+ uint32_t ier = STM32H7_SPI_IER_OVRIE | STM32H7_SPI_IER_MODFIE;
+
+ /* Enable the interrupts */
+ if (spi->cur_comm == SPI_SIMPLEX_TX || spi->cur_comm == SPI_3WIRE_TX)
+ ier |= STM32H7_SPI_IER_EOTIE | STM32H7_SPI_IER_TXTFIE;
+
+ stm32_spi_set_bits(spi, STM32H7_SPI_IER, ier);
stm32_spi_enable(spi);
@@ -1736,7 +1716,7 @@ static const struct stm32_spi_cfg stm32f4_spi_cfg = {
.set_mode = stm32f4_spi_set_mode,
.transfer_one_dma_start = stm32f4_spi_transfer_one_dma_start,
.dma_tx_cb = stm32f4_spi_dma_tx_cb,
- .dma_rx_cb = stm32f4_spi_dma_rx_cb,
+ .dma_rx_cb = stm32_spi_dma_rx_cb,
.transfer_one_irq = stm32f4_spi_transfer_one_irq,
.irq_handler_event = stm32f4_spi_irq_event,
.irq_handler_thread = stm32f4_spi_irq_thread,
@@ -1756,8 +1736,11 @@ static const struct stm32_spi_cfg stm32h7_spi_cfg = {
.set_data_idleness = stm32h7_spi_data_idleness,
.set_number_of_data = stm32h7_spi_number_of_data,
.transfer_one_dma_start = stm32h7_spi_transfer_one_dma_start,
- .dma_rx_cb = stm32h7_spi_dma_cb,
- .dma_tx_cb = stm32h7_spi_dma_cb,
+ .dma_rx_cb = stm32_spi_dma_rx_cb,
+ /*
+ * dma_tx_cb is not necessary since in case of TX, dma is followed by
+ * SPI access hence handling is performed within the SPI interrupt
+ */
.transfer_one_irq = stm32h7_spi_transfer_one_irq,
.irq_handler_thread = stm32h7_spi_irq_thread,
.baud_rate_div_min = STM32H7_SPI_MBR_DIV_MIN,
--
2.25.1
Add pm_runtime calls in probe/probe error path and remove
in order to be consistent in all places in ordering and
ensure that pm_runtime is disabled prior to resources used
by the SPI controller.
This patch also fixes the 2 following warnings on driver remove:
WARNING: CPU: 0 PID: 743 at drivers/clk/clk.c:594 clk_core_disable_lock+0x18/0x24
WARNING: CPU: 0 PID: 743 at drivers/clk/clk.c:476 clk_unprepare+0x24/0x2c
Fixes: 038ac869c9d2 ("spi: stm32: add runtime PM support")
Signed-off-by: Amelie Delaunay <[email protected]>
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/spi/spi-stm32.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 8ffcffbb8157..a92a28933edb 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1925,6 +1925,7 @@ static int stm32_spi_probe(struct platform_device *pdev)
master->can_dma = stm32_spi_can_dma;
pm_runtime_set_active(&pdev->dev);
+ pm_runtime_get_noresume(&pdev->dev);
pm_runtime_enable(&pdev->dev);
ret = spi_register_master(master);
@@ -1940,6 +1941,8 @@ static int stm32_spi_probe(struct platform_device *pdev)
err_pm_disable:
pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
err_dma_release:
if (spi->dma_tx)
dma_release_channel(spi->dma_tx);
@@ -1956,9 +1959,14 @@ static int stm32_spi_remove(struct platform_device *pdev)
struct spi_master *master = platform_get_drvdata(pdev);
struct stm32_spi *spi = spi_master_get_devdata(master);
+ pm_runtime_get_sync(&pdev->dev);
+
spi_unregister_master(master);
spi->cfg->disable(spi);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
if (master->dma_tx)
dma_release_channel(master->dma_tx);
if (master->dma_rx)
@@ -1966,7 +1974,6 @@ static int stm32_spi_remove(struct platform_device *pdev)
clk_disable_unprepare(spi->clk);
- pm_runtime_disable(&pdev->dev);
pinctrl_pm_select_sleep_state(&pdev->dev);
--
2.25.1
From: Amelie Delaunay <[email protected]>
Remove flush parameter and check RXWNE or RXPLVL when end of transfer
flag is set.
Signed-off-by: Amelie Delaunay <[email protected]>
Signed-off-by: Alain Volmat <[email protected]>
---
drivers/spi/spi-stm32.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index d37bfead4d8c..c2144e3c57eb 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -570,29 +570,30 @@ static void stm32f4_spi_read_rx(struct stm32_spi *spi)
/**
* stm32h7_spi_read_rxfifo - Read bytes in Receive Data Register
* @spi: pointer to the spi controller data structure
- * @flush: boolean indicating that FIFO should be flushed
*
* Write in rx_buf depends on remaining bytes to avoid to write beyond
* rx_buf end.
*/
-static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
+static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi)
{
u32 sr = readl_relaxed(spi->base + STM32H7_SPI_SR);
u32 rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
while ((spi->rx_len > 0) &&
((sr & STM32H7_SPI_SR_RXP) ||
- (flush && ((sr & STM32H7_SPI_SR_RXWNE) || (rxplvl > 0))))) {
+ ((sr & STM32H7_SPI_SR_EOT) &&
+ ((sr & STM32H7_SPI_SR_RXWNE) || (rxplvl > 0))))) {
u32 offs = spi->cur_xferlen - spi->rx_len;
if ((spi->rx_len >= sizeof(u32)) ||
- (flush && (sr & STM32H7_SPI_SR_RXWNE))) {
+ (sr & STM32H7_SPI_SR_RXWNE)) {
u32 *rx_buf32 = (u32 *)(spi->rx_buf + offs);
*rx_buf32 = readl_relaxed(spi->base + STM32H7_SPI_RXDR);
spi->rx_len -= sizeof(u32);
} else if ((spi->rx_len >= sizeof(u16)) ||
- (flush && (rxplvl >= 2 || spi->cur_bpw > 8))) {
+ (!(sr & STM32H7_SPI_SR_RXWNE) &&
+ (rxplvl >= 2 || spi->cur_bpw > 8))) {
u16 *rx_buf16 = (u16 *)(spi->rx_buf + offs);
*rx_buf16 = readw_relaxed(spi->base + STM32H7_SPI_RXDR);
@@ -608,8 +609,8 @@ static void stm32h7_spi_read_rxfifo(struct stm32_spi *spi, bool flush)
rxplvl = FIELD_GET(STM32H7_SPI_SR_RXPLVL, sr);
}
- dev_dbg(spi->dev, "%s%s: %d bytes left\n", __func__,
- flush ? "(flush)" : "", spi->rx_len);
+ dev_dbg(spi->dev, "%s: %d bytes left (sr=%08x)\n",
+ __func__, spi->rx_len, sr);
}
/**
@@ -677,12 +678,7 @@ static void stm32f4_spi_disable(struct stm32_spi *spi)
* @spi: pointer to the spi controller data structure
*
* RX-Fifo is flushed when SPI controller is disabled. To prevent any data
- * loss, use stm32h7_spi_read_rxfifo(flush) to read the remaining bytes in
- * RX-Fifo.
- * Normally, if TSIZE has been configured, we should relax the hardware at the
- * reception of the EOT interrupt. But in case of error, EOT will not be
- * raised. So the subsystem unprepare_message call allows us to properly
- * complete the transfer from an hardware point of view.
+ * loss, use stm32_spi_read_rxfifo to read the remaining bytes in RX-Fifo.
*/
static void stm32h7_spi_disable(struct stm32_spi *spi)
{
@@ -717,7 +713,7 @@ static void stm32h7_spi_disable(struct stm32_spi *spi)
}
if (!spi->cur_usedma && spi->rx_buf && (spi->rx_len > 0))
- stm32h7_spi_read_rxfifo(spi, true);
+ stm32h7_spi_read_rxfifo(spi);
if (spi->cur_usedma && spi->dma_tx)
dmaengine_terminate_all(spi->dma_tx);
@@ -913,7 +909,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
if (__ratelimit(&rs))
dev_dbg_ratelimited(spi->dev, "Communication suspended\n");
if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
- stm32h7_spi_read_rxfifo(spi, false);
+ stm32h7_spi_read_rxfifo(spi);
/*
* If communication is suspended while using DMA, it means
* that something went wrong, so stop the current transfer
@@ -934,7 +930,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
if (sr & STM32H7_SPI_SR_EOT) {
if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
- stm32h7_spi_read_rxfifo(spi, true);
+ stm32h7_spi_read_rxfifo(spi);
end = true;
}
@@ -944,7 +940,7 @@ static irqreturn_t stm32h7_spi_irq_thread(int irq, void *dev_id)
if (sr & STM32H7_SPI_SR_RXP)
if (!spi->cur_usedma && (spi->rx_buf && (spi->rx_len > 0)))
- stm32h7_spi_read_rxfifo(spi, false);
+ stm32h7_spi_read_rxfifo(spi);
writel_relaxed(sr & mask, spi->base + STM32H7_SPI_IFCR);
--
2.25.1
On Wed, 7 Jul 2021 10:26:59 +0200, Alain Volmat wrote:
> This series contains fixes & cleanup mainly regarding fifo
> and the way end of transfer triggered, when used with or
> without DMA.
> An additional patch cleans up the pm_runtime calls and another
> one enables the autosuspend.
>
> v2: - split pm_runtime fix patch into two
> - correct revert commit subject line
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[2/7] spi: stm32: enable pm_runtime autosuspend
commit: 9d5354145104cf392568a948c5ce2cb97f373fd7
[3/7] spi: stm32h7: fix full duplex irq handler handling
(no commit info)
[4/7] spi: stm32: Revert "properly handle 0 byte transfer"
commit: 70526e0b7601792bf546044fff92c368112f1d3f
[5/7] spi: stm32h7: rework rx fifo read function
commit: d87a5d64b5037cfedd7eb47d785b5c159ace8d9b
[6/7] spi: stm32h7: don't wait for EOT and flush fifo on disable
commit: dc6620c31326bc50fa22fd8900a9f995d0a04bc1
[7/7] spi: stm32: finalize message either on dma callback or EOT
commit: 7ceb0b8a3ceddc36ae4ef1cba6c25a0e28ed65fc
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark