* [PATCH] ata: sata_dwc_460ex: use devm for old DMA resource lifetime management
@ 2026-06-30 0:17 Rosen Penev
2026-06-30 2:06 ` Damien Le Moal
0 siblings, 1 reply; 3+ messages in thread
From: Rosen Penev @ 2026-06-30 0:17 UTC (permalink / raw)
To: linux-ide; +Cc: Damien Le Moal, Niklas Cassel, open list
Convert sata_dwc_dma_exit_old() to a devm action callback and register
it via devm_add_action_or_reset() after dw_dma_probe() succeeds. This
lets the devm framework handle DMA controller teardown automatically on
probe failure and driver removal.
As a result the probe function can use plain return-on-error for all
failure paths — the dma_initialized flag, the goto labels, and the
explicit sata_dwc_dma_exit_old() call in sata_dwc_remove() are all
eliminated.
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/ata/sata_dwc_460ex.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 35aa7f9acdf7..a1e319deb0b4 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -221,10 +221,18 @@ static int sata_dwc_dma_get_channel_old(struct sata_dwc_device_port *hsdevp)
return 0;
}
+static void sata_dwc_dma_exit_old(void *data)
+{
+ struct sata_dwc_device *hsdev = data;
+
+ dw_dma_remove(hsdev->dma);
+}
+
static int sata_dwc_dma_init_old(struct platform_device *pdev,
struct sata_dwc_device *hsdev)
{
struct device *dev = &pdev->dev;
+ int err;
hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
if (!hsdev->dma)
@@ -244,15 +252,11 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
return PTR_ERR(hsdev->dma->regs);
/* Initialize AHB DMAC */
- return dw_dma_probe(hsdev->dma);
-}
-
-static void sata_dwc_dma_exit_old(struct sata_dwc_device *hsdev)
-{
- if (!hsdev->dma)
- return;
+ err = dw_dma_probe(hsdev->dma);
+ if (err)
+ return err;
- dw_dma_remove(hsdev->dma);
+ return devm_add_action_or_reset(dev, sata_dwc_dma_exit_old, hsdev);
}
#endif
@@ -1187,7 +1191,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
err = phy_init(hsdev->phy);
if (err)
- goto error_out;
+ return err;
/*
* Now, register with libATA core, this will also initiate the
@@ -1195,12 +1199,10 @@ static int sata_dwc_probe(struct platform_device *ofdev)
* error_handler() to execute a dummy Softreset EH session
*/
err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
- if (err)
- dev_err(dev, "failed to activate host");
+ if (!err)
+ return err;
- return 0;
-
-error_out:
+ dev_err(dev, "failed to activate host");
phy_exit(hsdev->phy);
return err;
}
@@ -1215,11 +1217,6 @@ static void sata_dwc_remove(struct platform_device *ofdev)
phy_exit(hsdev->phy);
-#ifdef CONFIG_SATA_DWC_OLD_DMA
- /* Free SATA DMA resources */
- sata_dwc_dma_exit_old(hsdev);
-#endif
-
dev_dbg(dev, "done\n");
}
--
2.54.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ata: sata_dwc_460ex: use devm for old DMA resource lifetime management
2026-06-30 0:17 [PATCH] ata: sata_dwc_460ex: use devm for old DMA resource lifetime management Rosen Penev
@ 2026-06-30 2:06 ` Damien Le Moal
2026-06-30 2:23 ` Rosen Penev
0 siblings, 1 reply; 3+ messages in thread
From: Damien Le Moal @ 2026-06-30 2:06 UTC (permalink / raw)
To: Rosen Penev, linux-ide; +Cc: Niklas Cassel, open list
On 6/30/26 09:17, Rosen Penev wrote:
> Convert sata_dwc_dma_exit_old() to a devm action callback and register
> it via devm_add_action_or_reset() after dw_dma_probe() succeeds. This
> lets the devm framework handle DMA controller teardown automatically on
> probe failure and driver removal.
>
> As a result the probe function can use plain return-on-error for all
> failure paths — the dma_initialized flag, the goto labels, and the
> explicit sata_dwc_dma_exit_old() call in sata_dwc_remove() are all
> eliminated.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/ata/sata_dwc_460ex.c | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 35aa7f9acdf7..a1e319deb0b4 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -221,10 +221,18 @@ static int sata_dwc_dma_get_channel_old(struct sata_dwc_device_port *hsdevp)
> return 0;
> }
>
> +static void sata_dwc_dma_exit_old(void *data)
> +{
> + struct sata_dwc_device *hsdev = data;
> +
> + dw_dma_remove(hsdev->dma);
> +}
> +
> static int sata_dwc_dma_init_old(struct platform_device *pdev,
> struct sata_dwc_device *hsdev)
> {
> struct device *dev = &pdev->dev;
> + int err;
>
> hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
> if (!hsdev->dma)
> @@ -244,15 +252,11 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> return PTR_ERR(hsdev->dma->regs);
>
> /* Initialize AHB DMAC */
> - return dw_dma_probe(hsdev->dma);
> -}
> -
> -static void sata_dwc_dma_exit_old(struct sata_dwc_device *hsdev)
> -{
> - if (!hsdev->dma)
> - return;
> + err = dw_dma_probe(hsdev->dma);
> + if (err)
> + return err;
>
> - dw_dma_remove(hsdev->dma);
> + return devm_add_action_or_reset(dev, sata_dwc_dma_exit_old, hsdev);
> }
>
> #endif
> @@ -1187,7 +1191,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
>
> err = phy_init(hsdev->phy);
> if (err)
> - goto error_out;
> + return err;
>
> /*
> * Now, register with libATA core, this will also initiate the
> @@ -1195,12 +1199,10 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> * error_handler() to execute a dummy Softreset EH session
> */
> err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
> - if (err)
> - dev_err(dev, "failed to activate host");
> + if (!err)
> + return err;
What about the generic error check pattern:
err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
if (err) {
dev_err(dev, "failed to activate host");
phy_exit(hsdev->phy);
return err;
}
return 0;
Which I find personally much nicer to read.
>
> - return 0;
> -
> -error_out:
> + dev_err(dev, "failed to activate host");
> phy_exit(hsdev->phy);
> return err;
> }
> @@ -1215,11 +1217,6 @@ static void sata_dwc_remove(struct platform_device *ofdev)
>
> phy_exit(hsdev->phy);
>
> -#ifdef CONFIG_SATA_DWC_OLD_DMA
> - /* Free SATA DMA resources */
> - sata_dwc_dma_exit_old(hsdev);
> -#endif
> -
> dev_dbg(dev, "done\n");
> }
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ata: sata_dwc_460ex: use devm for old DMA resource lifetime management
2026-06-30 2:06 ` Damien Le Moal
@ 2026-06-30 2:23 ` Rosen Penev
0 siblings, 0 replies; 3+ messages in thread
From: Rosen Penev @ 2026-06-30 2:23 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, Niklas Cassel, open list
On Mon, Jun 29, 2026 at 7:06 PM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 6/30/26 09:17, Rosen Penev wrote:
> > Convert sata_dwc_dma_exit_old() to a devm action callback and register
> > it via devm_add_action_or_reset() after dw_dma_probe() succeeds. This
> > lets the devm framework handle DMA controller teardown automatically on
> > probe failure and driver removal.
> >
> > As a result the probe function can use plain return-on-error for all
> > failure paths — the dma_initialized flag, the goto labels, and the
> > explicit sata_dwc_dma_exit_old() call in sata_dwc_remove() are all
> > eliminated.
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> > drivers/ata/sata_dwc_460ex.c | 35 ++++++++++++++++-------------------
> > 1 file changed, 16 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> > index 35aa7f9acdf7..a1e319deb0b4 100644
> > --- a/drivers/ata/sata_dwc_460ex.c
> > +++ b/drivers/ata/sata_dwc_460ex.c
> > @@ -221,10 +221,18 @@ static int sata_dwc_dma_get_channel_old(struct sata_dwc_device_port *hsdevp)
> > return 0;
> > }
> >
> > +static void sata_dwc_dma_exit_old(void *data)
> > +{
> > + struct sata_dwc_device *hsdev = data;
> > +
> > + dw_dma_remove(hsdev->dma);
> > +}
> > +
> > static int sata_dwc_dma_init_old(struct platform_device *pdev,
> > struct sata_dwc_device *hsdev)
> > {
> > struct device *dev = &pdev->dev;
> > + int err;
> >
> > hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
> > if (!hsdev->dma)
> > @@ -244,15 +252,11 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> > return PTR_ERR(hsdev->dma->regs);
> >
> > /* Initialize AHB DMAC */
> > - return dw_dma_probe(hsdev->dma);
> > -}
> > -
> > -static void sata_dwc_dma_exit_old(struct sata_dwc_device *hsdev)
> > -{
> > - if (!hsdev->dma)
> > - return;
> > + err = dw_dma_probe(hsdev->dma);
> > + if (err)
> > + return err;
> >
> > - dw_dma_remove(hsdev->dma);
> > + return devm_add_action_or_reset(dev, sata_dwc_dma_exit_old, hsdev);
> > }
> >
> > #endif
> > @@ -1187,7 +1191,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> >
> > err = phy_init(hsdev->phy);
> > if (err)
> > - goto error_out;
> > + return err;
> >
> > /*
> > * Now, register with libATA core, this will also initiate the
> > @@ -1195,12 +1199,10 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> > * error_handler() to execute a dummy Softreset EH session
> > */
> > err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
> > - if (err)
> > - dev_err(dev, "failed to activate host");
> > + if (!err)
> > + return err;
>
> What about the generic error check pattern:
>
> err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
> if (err) {
> dev_err(dev, "failed to activate host");
> phy_exit(hsdev->phy);
> return err;
> }
>
> return 0;
>
> Which I find personally much nicer to read.
OK. Will do for v2.
I did it this way to reduce indentation.
>
> >
> > - return 0;
> > -
> > -error_out:
> > + dev_err(dev, "failed to activate host");
> > phy_exit(hsdev->phy);
> > return err;
> > }
> > @@ -1215,11 +1217,6 @@ static void sata_dwc_remove(struct platform_device *ofdev)
> >
> > phy_exit(hsdev->phy);
> >
> > -#ifdef CONFIG_SATA_DWC_OLD_DMA
> > - /* Free SATA DMA resources */
> > - sata_dwc_dma_exit_old(hsdev);
> > -#endif
> > -
> > dev_dbg(dev, "done\n");
> > }
> >
>
>
> --
> Damien Le Moal
> Western Digital Research
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-30 2:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-30 0:17 [PATCH] ata: sata_dwc_460ex: use devm for old DMA resource lifetime management Rosen Penev
2026-06-30 2:06 ` Damien Le Moal
2026-06-30 2:23 ` Rosen Penev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®