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

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 cause=
s incorrect UDMA enablement.
--

--- Patch [1]: [PATCH] ata: pata_ep93xx: use devm_dma_request_chan for DMA =
channels ---
commit 320ef19a1b3a76deb7df0daa1ed0cdbb72e88bfa
Author: Rosen Penev <[email protected]>

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 remov=
es
the now-unused ep93xx_pata_release_dma() helper and simplifies the error pa=
th
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_da=
ta *drv_data)
>  	ret =3D 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;
>  	}
> =20
>  	/* 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 expli=
citly
freed the channel and set drv_data->dma_rx_channel to NULL. Without nullify=
ing
it here, the driver might incorrectly assume the channel is fully configure=
d.

> @@ -694,21 +682,10 @@ static int ep93xx_pata_dma_init(struct ep93xx_pata_=
data *drv_data)
>  	ret =3D 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;
>  	}
> =20
>  	return 0;

[Severity: High]
Similarly, does returning early here without clearing drv_data->dma_tx_chan=
nel
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 =3D soc_device_match(ep93xx_soc_table);
		if (match)
			ap->udma_mask =3D (unsigned long) match->data;
		else
			ap->udma_mask =3D ATA_UDMA2;
	}

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

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260723214128.3569=
[email protected]?part=3D1