[PATCHv2] ata: pata_ep93xx: use devm_dma_request_chan for DMA channels

[PATCHv2] ata: pata_ep93xx: use devm_dma_request_chan for DMA channels

[Rosen Penev]

Convert the manual DMA channel request/release in ep93xx_pata_dma_init()
to the managed devm_dma_request_chan() API. This removes the now-unused
ep93xx_pata_release_dma() helper and the error-path cleanup in
ep93xx_pata_probe()/ep93xx_pata_remove(), simplifying the code and
avoiding potential leaks if probe fails after DMA init.

Assisted-by: opencode:hy3-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: add NULL assignments
 drivers/ata/pata_ep93xx.c | 54 ++++++++++-----------------------------
 1 file changed, 13 insertions(+), 41 deletions(-)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index 339ee5e43e9f..c99b8c49a1ce 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -627,18 +627,6 @@ static int ep93xx_pata_bus_softreset(struct ata_port *ap, unsigned int devmask,
 	return ep93xx_pata_wait_after_reset(&ap->link, devmask, deadline);
 }
 
-static void ep93xx_pata_release_dma(struct ep93xx_pata_data *drv_data)
-{
-	if (drv_data->dma_rx_channel) {
-		dma_release_channel(drv_data->dma_rx_channel);
-		drv_data->dma_rx_channel = NULL;
-	}
-	if (drv_data->dma_tx_channel) {
-		dma_release_channel(drv_data->dma_tx_channel);
-		drv_data->dma_tx_channel = NULL;
-	}
-}
-
 static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
 {
 	struct platform_device *pdev = drv_data->pdev;
@@ -655,7 +643,7 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
 	 * to request only one channel, and reprogram it's direction at
 	 * start of new transfer.
 	 */
-	drv_data->dma_rx_channel = dma_request_chan(dev, "rx");
+	drv_data->dma_rx_channel = devm_dma_request_chan(dev, "rx");
 	if (IS_ERR(drv_data->dma_rx_channel)) {
 		ret = PTR_ERR(drv_data->dma_rx_channel);
 		drv_data->dma_rx_channel = NULL;
@@ -665,14 +653,14 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
 		return 0;
 	}
 
-	drv_data->dma_tx_channel = dma_request_chan(&pdev->dev, "tx");
+	drv_data->dma_tx_channel = devm_dma_request_chan(&pdev->dev, "tx");
 	if (IS_ERR(drv_data->dma_tx_channel)) {
 		ret = PTR_ERR(drv_data->dma_tx_channel);
 		drv_data->dma_tx_channel = NULL;
 		if (ret == -EPROBE_DEFER)
-			goto fail_release_rx;
+			return ret;
 		dev_warn(dev, "tx DMA unavailable, using PIO\n");
-		goto fail_release_rx;
+		return 0;
 	}
 
 	/* Configure receive channel direction and source address */
@@ -683,7 +671,9 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
 	ret = dmaengine_slave_config(drv_data->dma_rx_channel, &conf);
 	if (ret) {
 		dev_warn(dev, "failed to configure rx dma channel, using PIO\n");
-		goto fail_release_dma;
+		drv_data->dma_rx_channel = NULL;
+		drv_data->dma_tx_channel = NULL;
+		return 0;
 	}
 
 	/* Configure transmit channel direction and destination address */
@@ -694,21 +684,12 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
 	ret = dmaengine_slave_config(drv_data->dma_tx_channel, &conf);
 	if (ret) {
 		dev_warn(dev, "failed to configure tx dma channel, using PIO\n");
-		goto fail_release_dma;
+		drv_data->dma_rx_channel = NULL;
+		drv_data->dma_tx_channel = NULL;
+		return 0;
 	}
 
 	return 0;
-
-fail_release_rx:
-	dma_release_channel(drv_data->dma_rx_channel);
-	drv_data->dma_rx_channel = NULL;
-	if (ret == -EPROBE_DEFER)
-		return ret;
-	return 0;
-
-fail_release_dma:
-	ep93xx_pata_release_dma(drv_data);
-	return 0;
 }
 
 static void ep93xx_pata_dma_start(struct ata_queued_cmd *qc)
@@ -959,10 +940,8 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
 
 	/* allocate host */
 	host = ata_host_alloc(&pdev->dev, 1);
-	if (!host) {
-		err = -ENOMEM;
-		goto err_rel_dma;
-	}
+	if (!host)
+		return -ENOMEM;
 
 	ep93xx_pata_clear_regs(ide_base);
 
@@ -999,14 +978,8 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "version " DRV_VERSION "\n");
 
 	/* activate host */
-	err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
+	return ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
 		&ep93xx_pata_sht);
-	if (err == 0)
-		return 0;
-
-err_rel_dma:
-	ep93xx_pata_release_dma(drv_data);
-	return err;
 }
 
 static void ep93xx_pata_remove(struct platform_device *pdev)
@@ -1015,7 +988,6 @@ static void ep93xx_pata_remove(struct platform_device *pdev)
 	struct ep93xx_pata_data *drv_data = host->private_data;
 
 	ata_host_detach(host);
-	ep93xx_pata_release_dma(drv_data);
 	ep93xx_pata_clear_regs(drv_data->ide_base);
 }
 
-- 
2.55.0

Re: [PATCHv2] ata: pata_ep93xx: use devm_dma_request_chan for DMA channels

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] DMA channels are permanently leaked (hoarded) for the lifetime of the device if DMA initialization fails and the driver falls back to PIO mode.
--

--- Patch [1]: [PATCHv2] ata: pata_ep93xx: use devm_dma_request_chan for DMA channels ---
commit 7ea9c0e84416db40b47acb17ec9bf21459755ef3
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: use devm_dma_request_chan for DMA channels

This patch converts the manual DMA channel request and release paths in
ep93xx_pata_dma_init() to use the managed devm_dma_request_chan() API. It
removes the explicit release helpers and cleanup paths to simplify the code.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index 339ee5e43e9f3..c99b8c49a1cec 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
[ ... ]
> @@ -665,14 +653,14 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
>  		return 0;
>  	}
>  
> -	drv_data->dma_tx_channel = dma_request_chan(&pdev->dev, "tx");
> +	drv_data->dma_tx_channel = devm_dma_request_chan(&pdev->dev, "tx");
>  	if (IS_ERR(drv_data->dma_tx_channel)) {
>  		ret = PTR_ERR(drv_data->dma_tx_channel);
>  		drv_data->dma_tx_channel = NULL;
>  		if (ret == -EPROBE_DEFER)
> -			goto fail_release_rx;
> +			return ret;
>  		dev_warn(dev, "tx DMA unavailable, using PIO\n");
> -		goto fail_release_rx;
> +		return 0;

[Severity: Medium]
When falling back to PIO mode due to the TX channel request failing, the
driver returns 0 and probe succeeds. Since this uses devres-managed
allocations, does this mean the successfully requested RX channel will be
permanently hoarded by devres for the lifetime of the device without being
used? 

>  	}
>  
>  	/* Configure receive channel direction and source address */
> @@ -683,7 +671,9 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
>  	ret = dmaengine_slave_config(drv_data->dma_rx_channel, &conf);
>  	if (ret) {
>  		dev_warn(dev, "failed to configure rx dma channel, using PIO\n");
> -		goto fail_release_dma;
> +		drv_data->dma_rx_channel = NULL;
> +		drv_data->dma_tx_channel = NULL;
> +		return 0;

[Severity: Medium]
If RX slave configuration fails, the driver nullifies the pointers but returns
0 to gracefully fall back to PIO mode. Since the device successfully binds in
PIO mode, will both the devm-managed RX and TX DMA channels remain permanently
allocated and unavailable to other devices?

>  	}
>  
>  	/* Configure transmit channel direction and destination address */
> @@ -694,21 +684,12 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
>  	ret = dmaengine_slave_config(drv_data->dma_tx_channel, &conf);
>  	if (ret) {
>  		dev_warn(dev, "failed to configure tx dma channel, using PIO\n");
> -		goto fail_release_dma;
> +		drv_data->dma_rx_channel = NULL;
> +		drv_data->dma_tx_channel = NULL;
> +		return 0;

[Severity: Medium]
As with the RX configuration failure, will returning 0 here leave the
allocated devm DMA channels leaked while operating in PIO mode? Since devres
only frees these resources on device unbind, should these fallback paths
explicitly release the hardware DMA channels first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724225232.41149-1-rosenp@gmail.com?part=1