All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rosen Penev <rosenp@gmail.com>
To: linux-ide@vger.kernel.org
Cc: Damien Le Moal <dlemoal@kernel.org>,
	Niklas Cassel <cassel@kernel.org>,
	Grant Likely <grant.likely@secretlab.ca>,
	Tim Yamin <plasm@roo.me.uk>, Julia Lawall <Julia.Lawall@lip6.fr>,
	Jeff Garzik <jgarzik@redhat.com>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCHv3 6/6] ata: pata_mpc52xx: convert to full devm resource management
Date: Fri, 12 Jun 2026 11:32:27 -0700	[thread overview]
Message-ID: <20260612183227.20446-7-rosenp@gmail.com> (raw)
In-Reply-To: <20260612183227.20446-1-rosenp@gmail.com>

Replace the open-coded of_address_to_resource()/devm_request_mem_region()/
devm_ioremap() sequence with devm_platform_get_and_ioremap_resource(),
and switch irq_of_parse_and_map() to platform_get_irq().

Use devm_add_action_or_reset() to manage the BestComm DMA task
(bcom_task) lifetime, and switch the task IRQ to devm_request_irq().
With both resources under devm, the LIFO teardown is free_irq before
bcom_ata_release, which is the correct ordering.

Remove the mpc52xx_ata_remove() wrapper and point .remove directly
at ata_platform_remove_one.  All error-path gotos and manual cleanup
are gone; any failure after devm_add_action_or_reset simply returns
and devm handles the unwind.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/pata_mpc52xx.c | 64 ++++++++++++--------------------------
 1 file changed, 20 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
index 238b39af38db..2472a2e2fbf0 100644
--- a/drivers/ata/pata_mpc52xx.c
+++ b/drivers/ata/pata_mpc52xx.c
@@ -20,7 +20,6 @@
 #include <linux/delay.h>
 #include <linux/libata.h>
 #include <linux/of.h>
-#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/types.h>
 
@@ -671,10 +670,15 @@ static int mpc52xx_ata_init_one(struct device *dev,
 /* OF Platform driver                                                       */
 /* ======================================================================== */
 
+static void devm_bcom_ata_release(void *data)
+{
+	bcom_ata_release(data);
+}
+
 static int mpc52xx_ata_probe(struct platform_device *op)
 {
 	unsigned int ipb_freq;
-	struct resource res_mem;
+	struct resource *res_mem;
 	struct mpc52xx_ata __iomem *ata_regs;
 	struct mpc52xx_ata_priv *priv = NULL;
 	int rv, task_irq;
@@ -690,25 +694,9 @@ static int mpc52xx_ata_probe(struct platform_device *op)
 		return -ENODEV;
 	}
 
-	/* Get device base address from device tree, request the region
-	 * and ioremap it. */
-	rv = of_address_to_resource(op->dev.of_node, 0, &res_mem);
-	if (rv) {
-		dev_err(&op->dev, "could not determine device base address\n");
-		return rv;
-	}
-
-	if (!devm_request_mem_region(&op->dev, res_mem.start,
-				     sizeof(*ata_regs), DRV_NAME)) {
-		dev_err(&op->dev, "error requesting register region\n");
-		return -EBUSY;
-	}
-
-	ata_regs = devm_ioremap(&op->dev, res_mem.start, sizeof(*ata_regs));
-	if (!ata_regs) {
-		dev_err(&op->dev, "error mapping device registers\n");
-		return -ENOMEM;
-	}
+	ata_regs = devm_platform_get_and_ioremap_resource(op, 0, &res_mem);
+	if (IS_ERR(ata_regs))
+		return PTR_ERR(ata_regs);
 
 	/*
 	 * By default, all DMA modes are disabled for the MPC5200.  Some
@@ -738,7 +726,7 @@ static int mpc52xx_ata_probe(struct platform_device *op)
 
 	priv->ipb_period = 1000000000 / (ipb_freq / 1000);
 	priv->ata_regs = ata_regs;
-	priv->ata_regs_pa = res_mem.start;
+	priv->ata_regs_pa = res_mem->start;
 	priv->csel = -1;
 
 	priv->ata_irq = platform_get_irq(op, 0);
@@ -762,47 +750,35 @@ static int mpc52xx_ata_probe(struct platform_device *op)
 
 	priv->dmatsk = dmatsk;
 
+	rv = devm_add_action_or_reset(&op->dev, devm_bcom_ata_release, dmatsk);
+	if (rv)
+		return rv;
+
 	task_irq = bcom_get_task_irq(dmatsk);
 	priv->task_irq = task_irq;
 	rv = devm_request_irq(&op->dev, task_irq, &mpc52xx_ata_task_irq, 0,
-				"ATA task", priv);
+			      "ATA task", priv);
 	if (rv) {
 		dev_err(&op->dev, "error requesting DMA IRQ\n");
-		goto err2;
+		return rv;
 	}
 
 	/* Init the hw */
 	rv = mpc52xx_ata_hw_init(priv);
 	if (rv) {
 		dev_err(&op->dev, "error initializing hardware\n");
-		goto err2;
+		return rv;
 	}
 
 	/* Register ourselves to libata */
-	rv = mpc52xx_ata_init_one(&op->dev, priv, res_mem.start,
+	rv = mpc52xx_ata_init_one(&op->dev, priv, res_mem->start,
 				  mwdma_mask, udma_mask);
 	if (rv) {
 		dev_err(&op->dev, "error registering with ATA layer\n");
-		goto err2;
+		return rv;
 	}
 
 	return 0;
-
- err2:
-	bcom_ata_release(dmatsk);
-	return rv;
-}
-
-static void mpc52xx_ata_remove(struct platform_device *op)
-{
-	struct ata_host *host = platform_get_drvdata(op);
-	struct mpc52xx_ata_priv *priv = host->private_data;
-
-	/* Deregister the ATA interface */
-	ata_platform_remove_one(op);
-
-	/* Clean up DMA */
-	bcom_ata_release(priv->dmatsk);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -843,7 +819,7 @@ static const struct of_device_id mpc52xx_ata_of_match[] = {
 
 static struct platform_driver mpc52xx_ata_of_platform_driver = {
 	.probe		= mpc52xx_ata_probe,
-	.remove		= mpc52xx_ata_remove,
+	.remove		= ata_platform_remove_one,
 #ifdef CONFIG_PM_SLEEP
 	.suspend	= mpc52xx_ata_suspend,
 	.resume		= mpc52xx_ata_resume,
-- 
2.54.0


      parent reply	other threads:[~2026-06-12 18:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-12 18:32 [PATCHv3 0/6] ata: pata_mpc52xx: fix cleanup ordering and modernize Rosen Penev
2026-06-12 18:32 ` [PATCHv3 1/6] ata: pata_mpc52xx: fix NULL pointer dereference in IRQ handler Rosen Penev
2026-06-12 18:46   ` sashiko-bot
2026-06-12 18:32 ` [PATCHv3 2/6] ata: pata_mpc52xx: synchronize with task IRQ before resetting DMA state Rosen Penev
2026-06-12 18:32 ` [PATCHv3 3/6] ata: pata_mpc52xx: reset cached DMA direction on resume Rosen Penev
2026-06-12 18:32 ` [PATCHv3 4/6] ata: pata_mpc52xx: remove redundant irq_dispose_mapping calls Rosen Penev
2026-06-12 18:45   ` sashiko-bot
2026-06-12 18:32 ` [PATCHv3 5/6] ata: pata_mpc52xx: convert to platform_get_irq() Rosen Penev
2026-06-12 18:32 ` Rosen Penev [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260612183227.20446-7-rosenp@gmail.com \
    --to=rosenp@gmail.com \
    --cc=Julia.Lawall@lip6.fr \
    --cc=cassel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=grant.likely@secretlab.ca \
    --cc=jgarzik@redhat.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=plasm@roo.me.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.