Linux EDAC development
 help / color / mirror / Atom feed
From: Rosen Penev <rosenp@gmail.com>
To: linux-edac@vger.kernel.org
Cc: Dinh Nguyen <dinguyen@kernel.org>, Borislav Petkov <bp@alien8.de>,
	Tony Luck <tony.luck@intel.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	linux-kernel@vger.kernel.org (open list),
	llvm@lists.linux.dev (open list:CLANG/LLVM BUILD
	SUPPORT:Keyword:\b(?i:clang|llvm)\b)
Subject: [PATCH] EDAC/altera: manage dci lifetime via devres and simplify ioremap
Date: Sun, 19 Jul 2026 14:12:38 -0700	[thread overview]
Message-ID: <20260719211238.589402-1-rosenp@gmail.com> (raw)

Replace the open-coded platform_get_resource() +
devm_request_mem_region() + devm_ioremap() sequence with the single
helper devm_platform_ioremap_resource(), and move the platform_get_irq()
calls earlier, dropping the redundant -ENODEV/-EBUSY/-ENOMEM error
paths. Also drop the now-orphaned devres_close_group() call left in the
success path after the devres group open/release calls were removed.

altr_edac_device_probe() requests its interrupts with devm_request_irq(),
passing dci as the handler context, but altr_edac_device_remove() frees
dci via edac_device_free_ctl_info() before devres releases the IRQs,
leaving a use-after-free window in the shared interrupt handler.
Register an edac_device_free_ctl_info() callback via
devm_add_action_or_reset() right after edac_device_alloc_ctl_info() so
dci is owned by devres and freed only after the IRQs are released. Drop
the explicit edac_device_free_ctl_info() from both the probe error path
and altr_edac_device_remove().

Built for arm (CONFIG_EDAC_ALTERA) with LLVM=1;
drivers/edac/altera_edac.o compiles cleanly and passes checkpatch
--strict.

Assisted-by: opencode:hy3-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/edac/altera_edac.c | 76 ++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 45 deletions(-)

diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index 68846f583eee..1d0d3c99c91b 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -703,37 +703,37 @@ MODULE_DEVICE_TABLE(of, altr_edac_device_of_match);
  *	Module specific initialization is done by passing the
  *	function index in the device tree.
  */
+
+static void altr_edac_device_free(void *data)
+{
+	struct edac_device_ctl_info *dci = data;
+
+	edac_device_free_ctl_info(dci);
+}
+
 static int altr_edac_device_probe(struct platform_device *pdev)
 {
 	struct edac_device_ctl_info *dci;
 	struct altr_edac_device_dev *drvdata;
-	struct resource *r;
+	void __iomem *base;
+	int db_irq;
+	int sb_irq;
 	int res = 0;
 	struct device_node *np = pdev->dev.of_node;
 	char *ecc_name = (char *)np->name;
 	static int dev_instance;
 
-	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Unable to open devm\n");
-		return -ENOMEM;
-	}
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!r) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "Unable to get mem resource\n");
-		res = -ENODEV;
-		goto fail;
-	}
+	sb_irq = platform_get_irq(pdev, 0);
+	if (sb_irq < 0)
+		return sb_irq;
 
-	if (!devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
-				     dev_name(&pdev->dev))) {
-		edac_printk(KERN_ERR, EDAC_DEVICE,
-			    "%s:Error requesting mem region\n", ecc_name);
-		res = -EBUSY;
-		goto fail;
-	}
+	db_irq = platform_get_irq(pdev, 1);
+	if (db_irq < 0)
+		return db_irq;
 
 	dci = edac_device_alloc_ctl_info(sizeof(*drvdata), ecc_name,
 					 1, ecc_name, 1, 0, dev_instance++);
@@ -741,20 +741,18 @@ static int altr_edac_device_probe(struct platform_device *pdev)
 	if (!dci) {
 		edac_printk(KERN_ERR, EDAC_DEVICE,
 			    "%s: Unable to allocate EDAC device\n", ecc_name);
-		res = -ENOMEM;
-		goto fail;
+		return -ENOMEM;
 	}
 
+	if (devm_add_action_or_reset(&pdev->dev, altr_edac_device_free, dci))
+		return -ENOMEM;
+
 	drvdata = dci->pvt_info;
 	dci->dev = &pdev->dev;
 	platform_set_drvdata(pdev, dci);
 	drvdata->edac_dev_name = ecc_name;
 
-	drvdata->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
-	if (!drvdata->base) {
-		res = -ENOMEM;
-		goto fail1;
-	}
+	drvdata->base = base;
 
 	/* Get driver specific data for this EDAC device */
 	drvdata->data = of_match_node(altr_edac_device_of_match, np)->data;
@@ -763,44 +761,33 @@ static int altr_edac_device_probe(struct platform_device *pdev)
 	if (drvdata->data->setup) {
 		res = drvdata->data->setup(drvdata);
 		if (res)
-			goto fail1;
+			return res;
 	}
 
-	drvdata->sb_irq = platform_get_irq(pdev, 0);
+	drvdata->sb_irq = sb_irq;
 	res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
 			       altr_edac_device_handler,
 			       0, dev_name(&pdev->dev), dci);
 	if (res)
-		goto fail1;
+		return res;
 
-	drvdata->db_irq = platform_get_irq(pdev, 1);
+	drvdata->db_irq = db_irq;
 	res = devm_request_irq(&pdev->dev, drvdata->db_irq,
 			       altr_edac_device_handler,
 			       0, dev_name(&pdev->dev), dci);
 	if (res)
-		goto fail1;
+		return res;
 
 	dci->mod_name = "Altera ECC Manager";
 	dci->dev_name = drvdata->edac_dev_name;
 
 	res = edac_device_add_device(dci);
 	if (res)
-		goto fail1;
+		return res;
 
 	altr_create_edacdev_dbgfs(dci, drvdata->data);
 
-	devres_close_group(&pdev->dev, NULL);
-
 	return 0;
-
-fail1:
-	edac_device_free_ctl_info(dci);
-fail:
-	devres_release_group(&pdev->dev, NULL);
-	edac_printk(KERN_ERR, EDAC_DEVICE,
-		    "%s:Error setting up EDAC device: %d\n", ecc_name, res);
-
-	return res;
 }
 
 static void altr_edac_device_remove(struct platform_device *pdev)
@@ -810,7 +797,6 @@ static void altr_edac_device_remove(struct platform_device *pdev)
 
 	debugfs_remove_recursive(drvdata->debugfs_dir);
 	edac_device_del_device(&pdev->dev);
-	edac_device_free_ctl_info(dci);
 }
 
 static struct platform_driver altr_edac_device_driver = {
-- 
2.55.0


                 reply	other threads:[~2026-07-19 21:12 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260719211238.589402-1-rosenp@gmail.com \
    --to=rosenp@gmail.com \
    --cc=bp@alien8.de \
    --cc=dinguyen@kernel.org \
    --cc=justinstitt@google.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=tony.luck@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox