[PATCH 0/3] dma: mv_xor: convert to devm resource management

[PATCH 0/3] dma: mv_xor: convert to devm resource management

[Rosen Penev]

Convert the mv_xor driver to use managed device resources (devm)
to simplify error handling and removal paths.

Patch 1 replaces the open-coded clock acquire/enable/disable/put
with devm_clk_get_optional_enabled, eliminating manual clock
cleanup in the probe error path.

Patch 2 adds the missing platform remove callback so that
channels, DMA devices, and IRQs are properly cleaned up on
driver unbind.

Patch 3 converts DMA pool allocation and IRQ requests to their
devm counterparts, allowing removal of the err_free_irq and
err_free_dma error labels.

Rosen Penev (3):
  dma: mv_xor: use devm_clk_get_optional_enabled
  dma: mv_xor: add missing platform remove function
  dma: mv_xor: use devm for dma pool and irq

 drivers/dma/mv_xor.c | 51 +++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

-- 
2.54.0

[PATCH 1/3] dma: mv_xor: use devm_clk_get_optional_enabled

[Rosen Penev]

Replace clk_get + clk_prepare_enable + clk_put with
devm_clk_get_optional_enabled. This eliminates the need for
manual clock cleanup in the probe error path.

It also fixes missing cleanup in a remove function.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/mv_xor.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 25ed61f1b089..a97fa0038652 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1359,12 +1359,9 @@ static int mv_xor_probe(struct platform_device *pdev)
 			mv_xor_conf_mbus_windows(xordev, dram);
 	}
 
-	/* Not all platforms can gate the clock, so it is not
-	 * an error if the clock does not exists.
-	 */
-	xordev->clk = clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(xordev->clk))
-		clk_prepare_enable(xordev->clk);
+	xordev->clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
+	if (IS_ERR(xordev->clk))
+		return PTR_ERR(xordev->clk);
 
 	/*
 	 * We don't want to have more than one channel per CPU in
@@ -1452,11 +1449,6 @@ static int mv_xor_probe(struct platform_device *pdev)
 				irq_dispose_mapping(xordev->channels[i]->irq);
 		}
 
-	if (!IS_ERR(xordev->clk)) {
-		clk_disable_unprepare(xordev->clk);
-		clk_put(xordev->clk);
-	}
-
 	return ret;
 }
 
-- 
2.54.0

[PATCH 2/3] dma: mv_xor: add missing platform remove function

[Rosen Penev]

The driver was missing a remove callback, so channels, DMA
devices, and IRQs were never cleaned up on driver unbind.
Implement mv_xor_remove to undo probe, patterned after the
existing error path.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/mv_xor.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index a97fa0038652..3fc39cca7cbd 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1452,8 +1452,22 @@ static int mv_xor_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void mv_xor_remove(struct platform_device *pdev)
+{
+	struct mv_xor_device *xordev = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < MV_XOR_MAX_CHANNELS; i++)
+		if (xordev->channels[i]) {
+			mv_xor_channel_remove(xordev->channels[i]);
+			if (pdev->dev.of_node)
+				irq_dispose_mapping(xordev->channels[i]->irq);
+		}
+}
+
 static struct platform_driver mv_xor_driver = {
 	.probe		= mv_xor_probe,
+	.remove		= mv_xor_remove,
 	.suspend        = mv_xor_suspend,
 	.resume         = mv_xor_resume,
 	.driver		= {
-- 
2.54.0

[PATCH 3/3] dma: mv_xor: use devm for dma pool and irq

[Rosen Penev]

Replace dma_alloc_wc with dmam_alloc_attrs and request_irq
with devm_request_irq. This eliminates the need for
manual cleanup of the dma pool and irq in both the channel
remove function and the channel add error labels, removing
the err_free_irq and err_free_dma labels entirely.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/mv_xor.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 3fc39cca7cbd..2ac7f01155fe 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1013,8 +1013,6 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
 
 	dma_async_device_unregister(&mv_chan->dmadev);
 
-	dma_free_wc(dev, MV_XOR_POOL_SIZE,
-			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
 	dma_unmap_single(dev, mv_chan->dummy_src_addr,
 			 MV_XOR_MIN_BYTE_COUNT, DMA_FROM_DEVICE);
 	dma_unmap_single(dev, mv_chan->dummy_dst_addr,
@@ -1025,8 +1023,6 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
 		list_del(&chan->device_node);
 	}
 
-	free_irq(mv_chan->irq, mv_chan);
-
 	return 0;
 }
 
@@ -1077,8 +1073,8 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
 	 * requires that we explicitly flush the writes
 	 */
 	mv_chan->dma_desc_pool_virt =
-	  dma_alloc_wc(&pdev->dev, MV_XOR_POOL_SIZE, &mv_chan->dma_desc_pool,
-		       GFP_KERNEL);
+	  dmam_alloc_attrs(&pdev->dev, MV_XOR_POOL_SIZE, &mv_chan->dma_desc_pool,
+			   GFP_KERNEL, DMA_ATTR_WRITE_COMBINE);
 	if (!mv_chan->dma_desc_pool_virt) {
 		ret = -ENOMEM;
 		goto err_unmap_dst;
@@ -1112,10 +1108,10 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
 	/* clear errors before enabling interrupts */
 	mv_chan_clear_err_status(mv_chan);
 
-	ret = request_irq(mv_chan->irq, mv_xor_interrupt_handler,
+	ret = devm_request_irq(&pdev->dev, mv_chan->irq, mv_xor_interrupt_handler,
 			  0, dev_name(&pdev->dev), mv_chan);
 	if (ret)
-		goto err_free_dma;
+		goto err_unmap_dst;
 
 	mv_chan_unmask_interrupts(mv_chan);
 
@@ -1138,14 +1134,14 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
 		ret = mv_chan_memcpy_self_test(mv_chan);
 		dev_dbg(&pdev->dev, "memcpy self test returned %d\n", ret);
 		if (ret)
-			goto err_free_irq;
+			goto err_unmap_dst;
 	}
 
 	if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)) {
 		ret = mv_chan_xor_self_test(mv_chan);
 		dev_dbg(&pdev->dev, "xor self test returned %d\n", ret);
 		if (ret)
-			goto err_free_irq;
+			goto err_unmap_dst;
 	}
 
 	dev_info(&pdev->dev, "Marvell XOR (%s): ( %s%s%s)\n",
@@ -1156,15 +1152,10 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
 
 	ret = dma_async_device_register(dma_dev);
 	if (ret)
-		goto err_free_irq;
+		goto err_unmap_dst;
 
 	return mv_chan;
 
-err_free_irq:
-	free_irq(mv_chan->irq, mv_chan);
-err_free_dma:
-	dma_free_wc(&pdev->dev, MV_XOR_POOL_SIZE,
-			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
 err_unmap_dst:
 	dma_unmap_single(dma_dev->dev, mv_chan->dummy_dst_addr,
 			 MV_XOR_MIN_BYTE_COUNT, DMA_TO_DEVICE);
-- 
2.54.0

Re: [PATCH 3/3] dma: mv_xor: use devm for dma pool and irq

[Frank Li]

On Tue, Jun 09, 2026 at 11:57:37PM -0700, Rosen Penev wrote:
> Replace dma_alloc_wc with dmam_alloc_attrs and request_irq
> with devm_request_irq. This eliminates the need for
> manual cleanup of the dma pool and irq in both the channel
> remove function and the channel add error labels, removing
> the err_free_irq and err_free_dma labels entirely.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

I already said many times, tag should dmaengine, not dma, all functional
need (), please respect reviewer's time.

Frank

>  drivers/dma/mv_xor.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
> index 3fc39cca7cbd..2ac7f01155fe 100644
> --- a/drivers/dma/mv_xor.c
> +++ b/drivers/dma/mv_xor.c
> @@ -1013,8 +1013,6 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
>
>  	dma_async_device_unregister(&mv_chan->dmadev);
>
> -	dma_free_wc(dev, MV_XOR_POOL_SIZE,
> -			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
>  	dma_unmap_single(dev, mv_chan->dummy_src_addr,
>  			 MV_XOR_MIN_BYTE_COUNT, DMA_FROM_DEVICE);
>  	dma_unmap_single(dev, mv_chan->dummy_dst_addr,
> @@ -1025,8 +1023,6 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
>  		list_del(&chan->device_node);
>  	}
>
> -	free_irq(mv_chan->irq, mv_chan);
> -
>  	return 0;
>  }
>
> @@ -1077,8 +1073,8 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>  	 * requires that we explicitly flush the writes
>  	 */
>  	mv_chan->dma_desc_pool_virt =
> -	  dma_alloc_wc(&pdev->dev, MV_XOR_POOL_SIZE, &mv_chan->dma_desc_pool,
> -		       GFP_KERNEL);
> +	  dmam_alloc_attrs(&pdev->dev, MV_XOR_POOL_SIZE, &mv_chan->dma_desc_pool,
> +			   GFP_KERNEL, DMA_ATTR_WRITE_COMBINE);
>  	if (!mv_chan->dma_desc_pool_virt) {
>  		ret = -ENOMEM;
>  		goto err_unmap_dst;
> @@ -1112,10 +1108,10 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>  	/* clear errors before enabling interrupts */
>  	mv_chan_clear_err_status(mv_chan);
>
> -	ret = request_irq(mv_chan->irq, mv_xor_interrupt_handler,
> +	ret = devm_request_irq(&pdev->dev, mv_chan->irq, mv_xor_interrupt_handler,
>  			  0, dev_name(&pdev->dev), mv_chan);
>  	if (ret)
> -		goto err_free_dma;
> +		goto err_unmap_dst;
>
>  	mv_chan_unmask_interrupts(mv_chan);
>
> @@ -1138,14 +1134,14 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>  		ret = mv_chan_memcpy_self_test(mv_chan);
>  		dev_dbg(&pdev->dev, "memcpy self test returned %d\n", ret);
>  		if (ret)
> -			goto err_free_irq;
> +			goto err_unmap_dst;
>  	}
>
>  	if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)) {
>  		ret = mv_chan_xor_self_test(mv_chan);
>  		dev_dbg(&pdev->dev, "xor self test returned %d\n", ret);
>  		if (ret)
> -			goto err_free_irq;
> +			goto err_unmap_dst;
>  	}
>
>  	dev_info(&pdev->dev, "Marvell XOR (%s): ( %s%s%s)\n",
> @@ -1156,15 +1152,10 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>
>  	ret = dma_async_device_register(dma_dev);
>  	if (ret)
> -		goto err_free_irq;
> +		goto err_unmap_dst;
>
>  	return mv_chan;
>
> -err_free_irq:
> -	free_irq(mv_chan->irq, mv_chan);
> -err_free_dma:
> -	dma_free_wc(&pdev->dev, MV_XOR_POOL_SIZE,
> -			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
>  err_unmap_dst:
>  	dma_unmap_single(dma_dev->dev, mv_chan->dummy_dst_addr,
>  			 MV_XOR_MIN_BYTE_COUNT, DMA_TO_DEVICE);
> --
> 2.54.0
>

Re: [PATCH 3/3] dma: mv_xor: use devm for dma pool and irq

[Vinod Koul]

On 10-06-26, 09:42, Frank Li wrote:
> On Tue, Jun 09, 2026 at 11:57:37PM -0700, Rosen Penev wrote:
> > Replace dma_alloc_wc with dmam_alloc_attrs and request_irq
> > with devm_request_irq. This eliminates the need for
> > manual cleanup of the dma pool and irq in both the channel
> > remove function and the channel add error labels, removing
> > the err_free_irq and err_free_dma labels entirely.
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> 
> I already said many times, tag should dmaengine, not dma, all functional
> need (), please respect reviewer's time.

And the worst is that some patches frpm Rosen have dmaengine, some dma.
That make me wonder how much thought has been given to these changes and
might be just an exercise to push AI generated code into wild and see
what sticks

-- 
~Vinod

Re: [PATCH 3/3] dma: mv_xor: use devm for dma pool and irq

[Rosen Penev]

On Wed, Jun 10, 2026 at 10:16 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 10-06-26, 09:42, Frank Li wrote:
> > On Tue, Jun 09, 2026 at 11:57:37PM -0700, Rosen Penev wrote:
> > > Replace dma_alloc_wc with dmam_alloc_attrs and request_irq
> > > with devm_request_irq. This eliminates the need for
> > > manual cleanup of the dma pool and irq in both the channel
> > > remove function and the channel add error labels, removing
> > > the err_free_irq and err_free_dma labels entirely.
> > >
> > > Assisted-by: opencode:big-pickle
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> >
> > I already said many times, tag should dmaengine, not dma, all functional
> > need (), please respect reviewer's time.
>
> And the worst is that some patches frpm Rosen have dmaengine, some dma.
> That make me wonder how much thought has been given to these changes and
> might be just an exercise to push AI generated code into wild and see
> what sticks
sort of. FWIW I use hardware that uses this driver:

 37:   89345094          0 GIC-0  54 Level     f1060800.xor
 38:  174555038          0 GIC-0  97 Level     f1060900.xor

Maybe all of this would finally fix

https://lore.kernel.org/all/CAKxU2N9yHa7ia_=07Csa7dDsZxcbPMmGSZKr+UxRSWH1VpG1fw@mail.gmail.com/

who knows...

Anyway, dma vs dmaengine is the model being stupid and me forgetting to fix it.

With other patches, I've gotten requests to try and fix pre existing
sashiko bug reports. I don't really want to but it is what it is.
>
> --
> ~Vinod