drivers/spi/spi-atmel.c | 94 ++++++++++++++----------------------------------- 1 file changed, 27 insertions(+), 67 deletions(-)
The original code set use_dma to false when dma_alloc_coherent() for
bounce buffers failed, but DMA channels acquired earlier via
atmel_spi_configure_dma() were never freed.
When devm_request_irq() or clk_prepare_enable() failed later in probe,
the driver also did not release DMA channels or bounce buffers already
allocated.
And in the following error path, the driver released DMA channels but
did not free the bounce buffers.
Fix by moving bounce buffer allocation into atmel_spi_configure_dma()
and switching both DMA channels and bounce buffers to their devm-
managed variants. Any allocation failure in the DMA configuration path
now correctly rolls back through devres cleanup, and thenow-unnecessary
atmel_spi_release_dma() function is removed.
Fixes: a9889ed62d06 ("spi: atmel: Implements transfers with bounce buffer")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Changes in v2:
- Switch to devm-managed variants to fix Claudiu Beznea's comment.
- Link to v1: https://patch.msgid.link/20260516-atmel-v1-0-674fb4707af6@gmail.com
To: Ryan Wanner <ryan.wanner@microchip.com>
To: Mark Brown <broonie@kernel.org>
To: Nicolas Ferre <nicolas.ferre@microchip.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Claudiu Beznea <claudiu.beznea@tuxon.dev>
To: Radu Pirea <radu.pirea@microchip.com>
Cc: linux-spi@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/spi/spi-atmel.c | 94 ++++++++++++++-----------------------------------
1 file changed, 27 insertions(+), 67 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 25aa294631c8..23022665c3a4 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -565,14 +565,15 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
struct device *dev = &as->pdev->dev;
int err;
- host->dma_tx = dma_request_chan(dev, "tx");
+ host->dma_tx = devm_dma_request_chan(dev, "tx");
if (IS_ERR(host->dma_tx)) {
err = PTR_ERR(host->dma_tx);
dev_dbg(dev, "No TX DMA channel, DMA is disabled\n");
- goto error_clear;
+ host->dma_tx = NULL;
+ return err;
}
- host->dma_rx = dma_request_chan(dev, "rx");
+ host->dma_rx = devm_dma_request_chan(dev, "rx");
if (IS_ERR(host->dma_rx)) {
err = PTR_ERR(host->dma_rx);
/*
@@ -580,12 +581,27 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
* requested tx channel.
*/
dev_dbg(dev, "No RX DMA channel, DMA is disabled\n");
- goto error;
+ host->dma_rx = NULL;
+ return err;
}
err = atmel_spi_dma_slave_config(as, 8);
if (err)
- goto error;
+ return err;
+
+ if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+ as->addr_tx_bbuf = dmam_alloc_coherent(dev, SPI_MAX_DMA_XFER,
+ &as->dma_addr_tx_bbuf,
+ GFP_KERNEL | GFP_DMA);
+ if (!as->addr_tx_bbuf)
+ return -ENOMEM;
+
+ as->addr_rx_bbuf = dmam_alloc_coherent(dev, SPI_MAX_DMA_XFER,
+ &as->dma_addr_rx_bbuf,
+ GFP_KERNEL | GFP_DMA);
+ if (!as->addr_rx_bbuf)
+ return -ENOMEM;
+ }
dev_info(&as->pdev->dev,
"Using %s (tx) and %s (rx) for DMA transfers\n",
@@ -593,14 +609,7 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
dma_chan_name(host->dma_rx));
return 0;
-error:
- if (!IS_ERR(host->dma_rx))
- dma_release_channel(host->dma_rx);
- if (!IS_ERR(host->dma_tx))
- dma_release_channel(host->dma_tx);
-error_clear:
- host->dma_tx = host->dma_rx = NULL;
- return err;
+
}
static void atmel_spi_stop_dma(struct spi_controller *host)
@@ -611,18 +620,6 @@ static void atmel_spi_stop_dma(struct spi_controller *host)
dmaengine_terminate_all(host->dma_tx);
}
-static void atmel_spi_release_dma(struct spi_controller *host)
-{
- if (host->dma_rx) {
- dma_release_channel(host->dma_rx);
- host->dma_rx = NULL;
- }
- if (host->dma_tx) {
- dma_release_channel(host->dma_tx);
- host->dma_tx = NULL;
- }
-}
-
/* This function is called by the DMA driver from tasklet context */
static void dma_callback(void *data)
{
@@ -1581,30 +1578,6 @@ static int atmel_spi_probe(struct platform_device *pdev)
as->use_pdc = true;
}
- if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
- as->addr_rx_bbuf = dma_alloc_coherent(&pdev->dev,
- SPI_MAX_DMA_XFER,
- &as->dma_addr_rx_bbuf,
- GFP_KERNEL | GFP_DMA);
- if (!as->addr_rx_bbuf) {
- as->use_dma = false;
- } else {
- as->addr_tx_bbuf = dma_alloc_coherent(&pdev->dev,
- SPI_MAX_DMA_XFER,
- &as->dma_addr_tx_bbuf,
- GFP_KERNEL | GFP_DMA);
- if (!as->addr_tx_bbuf) {
- as->use_dma = false;
- dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
- as->addr_rx_bbuf,
- as->dma_addr_rx_bbuf);
- }
- }
- if (!as->use_dma)
- dev_info(host->dev.parent,
- " can not allocate dma coherent memory\n");
- }
-
if (as->caps.has_dma_support && !as->use_dma)
dev_info(&pdev->dev, "Atmel SPI Controller using PIO only\n");
@@ -1652,7 +1625,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
ret = spi_register_controller(host);
if (ret)
- goto out_free_dma;
+ goto out_disable_rpm;
/* go! */
dev_info(&pdev->dev, "Atmel SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
@@ -1661,16 +1634,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
return 0;
-out_free_dma:
+out_disable_rpm:
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);
-
- if (as->use_dma)
- atmel_spi_release_dma(host);
-
spi_writel(as, CR, SPI_BIT(SWRST));
spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
- clk_disable_unprepare(as->gclk);
+ if (as->gclk)
+ clk_disable_unprepare(as->gclk);
out_disable_clk:
clk_disable_unprepare(clk);
@@ -1687,18 +1657,8 @@ static void atmel_spi_remove(struct platform_device *pdev)
spi_unregister_controller(host);
/* reset the hardware and block queue progress */
- if (as->use_dma) {
+ if (as->use_dma)
atmel_spi_stop_dma(host);
- atmel_spi_release_dma(host);
- if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
- dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
- as->addr_tx_bbuf,
- as->dma_addr_tx_bbuf);
- dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
- as->addr_rx_bbuf,
- as->dma_addr_rx_bbuf);
- }
- }
spin_lock_irq(&as->lock);
spi_writel(as, CR, SPI_BIT(SWRST));
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260516-atmel-6d6b0150eb7e
Best regards,
--
Felix Gu <ustc.gu@gmail.com>
On Sun, May 17, 2026 at 06:39:54PM +0800, Felix Gu wrote:
> Changes in v2:
> - Switch to devm-managed variants to fix Claudiu Beznea's comment.
The switch to devm is possibly a bit enthusiastic.
> - host->dma_tx = dma_request_chan(dev, "tx");
> + host->dma_tx = devm_dma_request_chan(dev, "tx");
> if (IS_ERR(host->dma_tx)) {
> - host->dma_rx = dma_request_chan(dev, "rx");
> + host->dma_rx = devm_dma_request_chan(dev, "rx");
> if (IS_ERR(host->dma_rx)) {
> err = PTR_ERR(host->dma_rx);
> /*
> @@ -580,12 +581,27 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
> * requested tx channel.
> */
> dev_dbg(dev, "No RX DMA channel, DMA is disabled\n");
> - goto error;
> + host->dma_rx = NULL;
> + return err;
> }
If the rx allocation fails then instead of jumping to cleanup we'll
return, and since the driver supports PIO operation it'll still be able
to probe...
> -error:
> - if (!IS_ERR(host->dma_rx))
> - dma_release_channel(host->dma_rx);
> - if (!IS_ERR(host->dma_tx))
> - dma_release_channel(host->dma_tx);
...with the tx DMA channel still allocated. This is very much an edge
case though, how much it matters is very questionalble.
© 2016 - 2026 Red Hat, Inc.