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

[sashiko-bot@kernel.org]

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