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

[PATCH] 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>
---
 drivers/ata/pata_ep93xx.c | 50 +++++++--------------------------------
 1 file changed, 9 insertions(+), 41 deletions(-)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index 339ee5e43e9f..04ab37134fb2 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,7 @@ 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;
+		return 0;
 	}
 
 	/* Configure transmit channel direction and destination address */
@@ -694,21 +682,10 @@ 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;
+		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 +936,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 +974,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 +984,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: [PATCH] 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:
- [High] Failure to clear DMA channel pointers on configuration error causes incorrect UDMA enablement.
--

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

ata: pata_ep93xx: use devm_dma_request_chan for DMA channels

This commit converts the manual DMA channel request/release in
ep93xx_pata_dma_init() to the managed devm_dma_request_chan() API. It removes
the now-unused ep93xx_pata_release_dma() helper and simplifies the error path
cleanup.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index 339ee5e43e9f3..04ab37134fb20 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c

[ ... ]

> @@ -683,7 +671,7 @@ 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;
> +		return 0;
>  	}
>  
>  	/* Configure transmit channel direction and destination address */

[Severity: High]
If dmaengine_slave_config() fails, will this leave drv_data->dma_rx_channel
non-NULL?

The old fail_release_dma path called ep93xx_pata_release_dma(), which explicitly
freed the channel and set drv_data->dma_rx_channel to NULL. Without nullifying
it here, the driver might incorrectly assume the channel is fully configured.

> @@ -694,21 +682,10 @@ 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;
> +		return 0;
>  	}
>  
>  	return 0;

[Severity: High]
Similarly, does returning early here without clearing drv_data->dma_tx_channel
leave the pointer set?

Later in ep93xx_pata_probe(), the code relies on these pointers to check
if UDMA should be enabled:

	if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
		const struct soc_device_attribute *match;

		match = soc_device_match(ep93xx_soc_table);
		if (match)
			ap->udma_mask = (unsigned long) match->data;
		else
			ap->udma_mask = ATA_UDMA2;
	}

If the pointers are left non-NULL after a configuration failure, will the driver
incorrectly advertise UDMA capabilities and attempt to use unconfigured
channels for I/O?

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