Re: [PATCH] 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:
- [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