[PATCH 0/2] spi: atmel: two fixes

[PATCH 0/2] spi: atmel: two fixes

[Felix Gu]

Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Felix Gu (2):
      spi: atmel: fix resource leak on DMA buffer allocation failure
      spi: atmel: fix DMA resource leak on probe error paths

 drivers/spi/spi-atmel.c | 129 ++++++++++++++++++++++++------------------------
 1 file changed, 65 insertions(+), 64 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260516-atmel-6d6b0150eb7e

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>

[PATCH 1/2] spi: atmel: fix resource leak on DMA buffer allocation failure

[Felix Gu]

The original code set use_dma to false when dma_alloc_coherent() fails,
so DMA channels allocated earlier were never freed, causing a resource
leak.

Fix by moving the bounce buffer allocation into
atmel_spi_configure_dma() and extending atmel_spi_release_dma() to
also free the bounce buffers. Any allocation failure in the DMA
configuration path now rolls back both channels and buffers through
the same release function.

Fixes: a9889ed62d06 ("spi: atmel: Implements transfers with bounce buffer")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/spi/spi-atmel.c | 113 ++++++++++++++++++++++++------------------------
 1 file changed, 57 insertions(+), 56 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 25aa294631c8..e519a86a2b45 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -559,6 +559,34 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as, u8 bits_per_word)
 	return err;
 }
 
+static void atmel_spi_release_dma(struct spi_controller *host,
+				  struct atmel_spi *as)
+{
+	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;
+	}
+
+	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		if (as->addr_tx_bbuf) {
+			dma_free_coherent(&as->pdev->dev, SPI_MAX_DMA_XFER,
+					  as->addr_tx_bbuf,
+					  as->dma_addr_tx_bbuf);
+			as->addr_tx_bbuf = NULL;
+		}
+		if (as->addr_rx_bbuf) {
+			dma_free_coherent(&as->pdev->dev, SPI_MAX_DMA_XFER,
+					  as->addr_rx_bbuf,
+					  as->dma_addr_rx_bbuf);
+			as->addr_rx_bbuf = NULL;
+		}
+	}
+}
+
 static int atmel_spi_configure_dma(struct spi_controller *host,
 				   struct atmel_spi *as)
 {
@@ -569,7 +597,8 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
 	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");
@@ -580,12 +609,31 @@ 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;
+		goto err_release_dma;
 	}
 
 	err = atmel_spi_dma_slave_config(as, 8);
 	if (err)
-		goto error;
+		goto err_release_dma;
+
+	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		as->addr_tx_bbuf = dma_alloc_coherent(dev, SPI_MAX_DMA_XFER,
+						      &as->dma_addr_tx_bbuf,
+						      GFP_KERNEL | GFP_DMA);
+		if (!as->addr_tx_bbuf) {
+			err = -ENOMEM;
+			goto err_release_dma;
+		}
+
+		as->addr_rx_bbuf = dma_alloc_coherent(dev, SPI_MAX_DMA_XFER,
+						      &as->dma_addr_rx_bbuf,
+						      GFP_KERNEL | GFP_DMA);
+		if (!as->addr_rx_bbuf) {
+			err = -ENOMEM;
+			goto err_release_dma;
+		}
+	}
 
 	dev_info(&as->pdev->dev,
 			"Using %s (tx) and %s (rx) for DMA transfers\n",
@@ -593,13 +641,10 @@ 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;
+
+err_release_dma:
+	atmel_spi_release_dma(host, as);
+
 	return err;
 }
 
@@ -611,18 +656,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 +1614,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");
 
@@ -1666,7 +1675,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 
 	if (as->use_dma)
-		atmel_spi_release_dma(host);
+		atmel_spi_release_dma(host, as);
 
 	spi_writel(as, CR, SPI_BIT(SWRST));
 	spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
@@ -1689,15 +1698,7 @@ static void atmel_spi_remove(struct platform_device *pdev)
 	/* reset the hardware and block queue progress */
 	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);
-		}
+		atmel_spi_release_dma(host, as);
 	}
 
 	spin_lock_irq(&as->lock);

-- 
2.43.0

[PATCH 2/2] spi: atmel: fix DMA resource leak on probe error paths

[Felix Gu]

The DMA resources allocated by atmel_spi_configure_dma() were not
released when devm_request_irq() or clk_prepare_enable() failed.
Route these two error paths through out_free_dma so that
atmel_spi_release_dma() releases the channels.

The same issue existed in the gclk_prepare_enable() failure path,
which jumped to out_disable_clk and returned without DMA cleanup.
The new fall-through from out_disable_clk into out_free_dma now
handles it as well.

Fixes: 1ccc404a7fc4 ("spi/spi-atmel: add dmaengine support")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/spi/spi-atmel.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index e519a86a2b45..893f7c7ce358 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1625,12 +1625,12 @@ static int atmel_spi_probe(struct platform_device *pdev)
 					0, dev_name(&pdev->dev), host);
 	}
 	if (ret)
-		return ret;
+		goto out_free_dma;
 
 	/* Initialize the hardware */
 	ret = clk_prepare_enable(clk);
 	if (ret)
-		return ret;
+		goto out_free_dma;
 
 	/*
 	 * In cases where the peripheral clock is higher,the FLEX_SPI_CSRx.SCBR
@@ -1661,7 +1661,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",
@@ -1670,18 +1670,18 @@ 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, as);
-
 	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);
+out_free_dma:
+	if (as->use_dma)
+		atmel_spi_release_dma(host, as);
 
 	return ret;
 }

-- 
2.43.0

Re: [PATCH 0/2] spi: atmel: two fixes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Sat, May 16, 2026 at 01:20:33AM +0800, Felix Gu wrote:
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> Felix Gu (2):
>       spi: atmel: fix resource leak on DMA buffer allocation failure
>       spi: atmel: fix DMA resource leak on probe error paths

This needs a rebase for current mainline, I didn't check but I suspect
it's conflicting witj Johan's fixes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/2] spi: atmel: two fixes

[Felix Gu]

On Sat, May 16, 2026 at 10:34 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Sat, May 16, 2026 at 01:20:33AM +0800, Felix Gu wrote:
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> > ---
> > Felix Gu (2):
> >       spi: atmel: fix resource leak on DMA buffer allocation failure
> >       spi: atmel: fix DMA resource leak on probe error paths
>
> This needs a rebase for current mainline, I didn't check but I suspect
> it's conflicting witj Johan's fixes.
Hi Mark,
The two commits are already based on Johan’s fixes:
commit ebf99aebc458391893c598f1caddd42bfcc97fc5
Author: Johan Hovold johan@kernel.org
Date: Wed Apr 29 11:13:16 2026 +0200
spi: atmel: switch to managed controller allocation

There should not be any conflicts.

Best regards,
Felix

Re: [PATCH 0/2] spi: atmel: two fixes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 460 bytes --]

On Sat, May 16, 2026 at 12:41:56PM +0800, Felix Gu wrote:

> The two commits are already based on Johan’s fixes:
> commit ebf99aebc458391893c598f1caddd42bfcc97fc5
> Author: Johan Hovold johan@kernel.org
> Date: Wed Apr 29 11:13:16 2026 +0200
> spi: atmel: switch to managed controller allocation
> 
> There should not be any conflicts.

Those are on my for-7.2 branch, we're currently in v7.1-rcN...  I guess
it's probably not urgent either way.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]