[PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Rosen Penev]

Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
the resource lookup, region reservation and mapping into one step and returns
an ERR_PTR checked with IS_ERR()/PTR_ERR().

Behaviorally equivalent with respect to region reservation: the driver
already reserved the region, so the non-overlapping reg requirement was
already satisfied. Drop the now-unused linux/of_address.h include.

Built for arm64 (defconfig + CONFIG_EDAC_FSL_DDR) with LLVM=1;
drivers/edac/fsl_ddr_edac.o compiles cleanly.

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

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index b27dff96aeb6..ae1d54434e38 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -22,7 +22,6 @@
 #include <linux/gfp.h>
 
 #include <linux/of.h>
-#include <linux/of_address.h>
 #include "edac_module.h"
 #include "fsl_ddr_edac.h"
 
@@ -495,11 +494,15 @@ int fsl_mc_err_probe(struct platform_device *op)
 	struct mem_ctl_info *mci;
 	struct edac_mc_layer layers[2];
 	struct fsl_mc_pdata *pdata;
-	struct resource r;
+	void __iomem *mc_vbase;
 	u32 ecc_en_mask;
 	u32 sdram_ctl;
 	int res;
 
+	mc_vbase = devm_platform_ioremap_resource(op, 0);
+	if (IS_ERR(mc_vbase))
+		return PTR_ERR(mc_vbase);
+
 	if (!devres_open_group(&op->dev, fsl_mc_err_probe, GFP_KERNEL))
 		return -ENOMEM;
 
@@ -531,28 +534,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 	 * Default is big endian.
 	 */
 	pdata->little_endian = of_property_read_bool(op->dev.of_node, "little-endian");
-
-	res = of_address_to_resource(op->dev.of_node, 0, &r);
-	if (res) {
-		pr_err("%s: Unable to get resource for MC err regs\n",
-		       __func__);
-		goto err;
-	}
-
-	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
-				     pdata->name)) {
-		pr_err("%s: Error while requesting mem region\n",
-		       __func__);
-		res = -EBUSY;
-		goto err;
-	}
-
-	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
-	if (!pdata->mc_vbase) {
-		pr_err("%s: Unable to setup MC err regs\n", __func__);
-		res = -ENOMEM;
-		goto err;
-	}
+	pdata->mc_vbase = mc_vbase;
 
 	if (pdata->flag == TYPE_IMX9) {
 		pdata->inject_vbase = devm_platform_ioremap_resource_byname(op, "inject");
-- 
2.55.0

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware registers altered during initialization are not restored on probe error paths.
- [High] Use-after-free of `mci` and `pdata` in the shared interrupt handler during device removal.
--

--- Patch [1]: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource() ---
commit 95d86798c0c6ff810ee7e3129ccd0c259af42fed
Author: Rosen Penev <rosenp@gmail.com>

EDAC: fsl_ddr: use devm_platform_ioremap_resource()
    
Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
the resource lookup, region reservation and mapping into one step and returns
an ERR_PTR checked with IS_ERR()/PTR_ERR().

> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index e4eaec0aa81d5..f19ccef942c02 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c

[ ... ]

> @@ -532,28 +535,7 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	 * Default is big endian.
>  	 */
>  	pdata->little_endian = of_property_read_bool(op->dev.of_node, "little-endian");
> -
> -	res = of_address_to_resource(op->dev.of_node, 0, &r);
> -	if (res) {
> -		pr_err("%s: Unable to get resource for MC err regs\n",
> -		       __func__);
> -		goto err;

[Severity: High]
This is a pre-existing issue, but since we are modifying error paths in
fsl_mc_err_probe(), does the error handling here fail to restore hardware
registers?

Later in the probe function, hardware state is saved and error interrupts
are enabled by clearing FSL_MC_ERR_DISABLE and setting FSL_MC_ERR_INT_EN.

If a subsequent initialization step fails and jumps to the err or err2
labels:

err2:
	edac_mc_del_mc(&op->dev);
err:
	devres_release_group(&op->dev, fsl_mc_err_probe);
	edac_mc_free(mci);
	return res;

These paths clean up driver memory but do not restore the modified hardware
registers. Since the memory controller is left configured to generate
interrupts but no handler is successfully registered, could a subsequent
memory error trigger an unhandled interrupt storm on the shared IRQ line?

> -	}
> -
> -	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
> -				     pdata->name)) {
> -		pr_err("%s: Error while requesting mem region\n",
> -		       __func__);
> -		res = -EBUSY;
> -		goto err;
> -	}
> -
> -	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
> -	if (!pdata->mc_vbase) {
> -		pr_err("%s: Unable to setup MC err regs\n", __func__);
> -		res = -ENOMEM;
> -		goto err;
> -	}
> +	pdata->mc_vbase = mc_vbase;
>  
>  	if (pdata->flag == TYPE_IMX9) {
>  		pdata->inject_vbase = devm_platform_ioremap_resource_byname(op, "inject");

[Severity: High]
This isn't a bug introduced by this patch, but could there be a
use-after-free race condition during device removal?

In fsl_mc_err_probe(), the interrupt is registered as shared using the
managed API devm_request_irq(), passing mci as the context data.

During device removal:

drivers/edac/fsl_ddr_edac.c:fsl_mc_err_remove() {
    ...
    edac_mc_del_mc(&op->dev);
    edac_mc_free(mci);
}

Since devres_release_all() runs after fsl_mc_err_remove() completes, the
shared IRQ remains active while mci is explicitly freed here.

If another device sharing the IRQ line triggers an interrupt during this
window, fsl_mc_isr() will dereference the freed mci and pdata pointers.
Should the IRQ be freed explicitly before freeing mci, or should mci be
managed by devres as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714234609.902367-1-rosenp@gmail.com?part=1

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Frank Li]

On Tue, Jul 14, 2026 at 04:46:09PM -0700, Rosen Penev wrote:
> Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
> and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
> the resource lookup, region reservation and mapping into one step and returns
> an ERR_PTR checked with IS_ERR()/PTR_ERR().
>
> Behaviorally equivalent with respect to region reservation: the driver
> already reserved the region, so the non-overlapping reg requirement was
> already satisfied. Drop the now-unused linux/of_address.h include.
>
> Built for arm64 (defconfig + CONFIG_EDAC_FSL_DDR) with LLVM=1;
> drivers/edac/fsl_ddr_edac.o compiles cleanly.
>
> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/edac/fsl_ddr_edac.c | 30 ++++++------------------------
>  1 file changed, 6 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index b27dff96aeb6..ae1d54434e38 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -22,7 +22,6 @@
>  #include <linux/gfp.h>
>
>  #include <linux/of.h>
> -#include <linux/of_address.h>
>  #include "edac_module.h"
>  #include "fsl_ddr_edac.h"
>
> @@ -495,11 +494,15 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	struct mem_ctl_info *mci;
>  	struct edac_mc_layer layers[2];
>  	struct fsl_mc_pdata *pdata;
> -	struct resource r;
> +	void __iomem *mc_vbase;
>  	u32 ecc_en_mask;
>  	u32 sdram_ctl;
>  	int res;
>
> +	mc_vbase = devm_platform_ioremap_resource(op, 0);
> +	if (IS_ERR(mc_vbase))
> +		return PTR_ERR(mc_vbase);
> +
>  	if (!devres_open_group(&op->dev, fsl_mc_err_probe, GFP_KERNEL))
>  		return -ENOMEM;
>
> @@ -531,28 +534,7 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	 * Default is big endian.
>  	 */
>  	pdata->little_endian = of_property_read_bool(op->dev.of_node, "little-endian");
> -
> -	res = of_address_to_resource(op->dev.of_node, 0, &r);
> -	if (res) {
> -		pr_err("%s: Unable to get resource for MC err regs\n",
> -		       __func__);
> -		goto err;
> -	}
> -
> -	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
> -				     pdata->name)) {
> -		pr_err("%s: Error while requesting mem region\n",
> -		       __func__);
> -		res = -EBUSY;
> -		goto err;
> -	}
> -
> -	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
> -	if (!pdata->mc_vbase) {
> -		pr_err("%s: Unable to setup MC err regs\n", __func__);
> -		res = -ENOMEM;
> -		goto err;
> -	}
> +	pdata->mc_vbase = mc_vbase;
>
>  	if (pdata->flag == TYPE_IMX9) {
>  		pdata->inject_vbase = devm_platform_ioremap_resource_byname(op, "inject");
> --
> 2.55.0
>
>

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Borislav Petkov]

On Wed, Jul 15, 2026 at 11:00:50PM -0500, Frank Li wrote:
> On Tue, Jul 14, 2026 at 04:46:09PM -0700, Rosen Penev wrote:
> > Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
> > and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
> > the resource lookup, region reservation and mapping into one step and returns
> > an ERR_PTR checked with IS_ERR()/PTR_ERR().
> >
> > Behaviorally equivalent with respect to region reservation: the driver
> > already reserved the region, so the non-overlapping reg requirement was
> > already satisfied. Drop the now-unused linux/of_address.h include.
> >
> > Built for arm64 (defconfig + CONFIG_EDAC_FSL_DDR) with LLVM=1;
> > drivers/edac/fsl_ddr_edac.o compiles cleanly.
> >
> > Assisted-by: opencode:hy3-free
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Sashiko caught a bunch of preexisting issues:

https://sashiko.dev/#/patchset/20260714234609.902367-1-rosenp%40gmail.com

Let's fix them first please.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Rosen Penev]

On Sun, Jul 19, 2026 at 8:58 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Jul 15, 2026 at 11:00:50PM -0500, Frank Li wrote:
> > On Tue, Jul 14, 2026 at 04:46:09PM -0700, Rosen Penev wrote:
> > > Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
> > > and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
> > > the resource lookup, region reservation and mapping into one step and returns
> > > an ERR_PTR checked with IS_ERR()/PTR_ERR().
> > >
> > > Behaviorally equivalent with respect to region reservation: the driver
> > > already reserved the region, so the non-overlapping reg requirement was
> > > already satisfied. Drop the now-unused linux/of_address.h include.
> > >
> > > Built for arm64 (defconfig + CONFIG_EDAC_FSL_DDR) with LLVM=1;
> > > drivers/edac/fsl_ddr_edac.o compiles cleanly.
> > >
> > > Assisted-by: opencode:hy3-free
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Sashiko caught a bunch of preexisting issues:
>
> https://sashiko.dev/#/patchset/20260714234609.902367-1-rosenp%40gmail.com
>
> Let's fix them first please.
The unfortunate part of that is sashiko does not understand v2 patches
and whatnot. Meaning it will always complain about things fixed in
other patches.

There's no practical way to move forward unless done piece by piece.

As an example fixing the first issue causes
 [High] Calling `ddr_out32()` in the `err` cleanup path
unconditionally dereferences unmapped I/O memory pointers if probe
fails early, leading to a kernel crash.

because these functions are split into three.

>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Borislav Petkov]

On Tue, Jul 28, 2026 at 03:08:25PM -0700, Rosen Penev wrote:
> The unfortunate part of that is sashiko does not understand v2 patches
> and whatnot. Meaning it will always complain about things fixed in
> other patches.

That's fine as long as we've replied to the patches and said why it is ok. LLM
is just a tool, at least for now and how it is used with Sashiko.

What is more important here is finding real hw to test it on. I'm hoping Frank
on Cc will move...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Rosen Penev]

On Tue, Jul 28, 2026 at 3:41 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Jul 28, 2026 at 03:08:25PM -0700, Rosen Penev wrote:
> > The unfortunate part of that is sashiko does not understand v2 patches
> > and whatnot. Meaning it will always complain about things fixed in
> > other patches.
>
> That's fine as long as we've replied to the patches and said why it is ok. LLM
> is just a tool, at least for now and how it is used with Sashiko.
>
> What is more important here is finding real hw to test it on. I'm hoping Frank
> on Cc will move...
Based on some git log/grep, I don't see many devices that use this
driver unfortunately.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Frank Li]

On Tue, Jul 28, 2026 at 03:40:59PM -0700, Borislav Petkov wrote:
> [You don't often get email from bp@alien8.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, Jul 28, 2026 at 03:08:25PM -0700, Rosen Penev wrote:
> > The unfortunate part of that is sashiko does not understand v2 patches
> > and whatnot. Meaning it will always complain about things fixed in
> > other patches.
>
> That's fine as long as we've replied to the patches and said why it is ok. LLM
> is just a tool, at least for now and how it is used with Sashiko.
>
> What is more important here is finding real hw to test it on. I'm hoping Frank
> on Cc will move...

Tested-by: Frank Li <Frank.Li@nxp.com>

>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Borislav Petkov]

On Thu, Jul 30, 2026 at 01:44:28PM -0500, Frank Li wrote:
> On Tue, Jul 28, 2026 at 03:40:59PM -0700, Borislav Petkov wrote:
> > [You don't often get email from bp@alien8.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Tue, Jul 28, 2026 at 03:08:25PM -0700, Rosen Penev wrote:
> > > The unfortunate part of that is sashiko does not understand v2 patches
> > > and whatnot. Meaning it will always complain about things fixed in
> > > other patches.
> >
> > That's fine as long as we've replied to the patches and said why it is ok. LLM
> > is just a tool, at least for now and how it is used with Sashiko.
> >
> > What is more important here is finding real hw to test it on. I'm hoping Frank
> > on Cc will move...
> 
> Tested-by: Frank Li <Frank.Li@nxp.com>

Thanks. 

We have a bunch of Sashiko reported pre-existing issues:

https://sashiko.dev/#/patchset/20260714234609.902367-1-rosenp%40gmail.com

Who's taking care of those?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Frank Li]

On Thu, Jul 30, 2026 at 05:04:07PM -0700, Borislav Petkov wrote:
> On Thu, Jul 30, 2026 at 01:44:28PM -0500, Frank Li wrote:
> > On Tue, Jul 28, 2026 at 03:40:59PM -0700, Borislav Petkov wrote:
> > > [You don't often get email from bp@alien8.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > >
> > > On Tue, Jul 28, 2026 at 03:08:25PM -0700, Rosen Penev wrote:
> > > > The unfortunate part of that is sashiko does not understand v2 patches
> > > > and whatnot. Meaning it will always complain about things fixed in
> > > > other patches.
> > >
> > > That's fine as long as we've replied to the patches and said why it is ok. LLM
> > > is just a tool, at least for now and how it is used with Sashiko.
> > >
> > > What is more important here is finding real hw to test it on. I'm hoping Frank
> > > on Cc will move...
> >
> > Tested-by: Frank Li <Frank.Li@nxp.com>
>
> Thanks.
>
> We have a bunch of Sashiko reported pre-existing issues:
>
> https://sashiko.dev/#/patchset/20260714234609.902367-1-rosenp%40gmail.com
>
> Who's taking care of those?

There are too many pre-existing issues at recent review, we are trying to
clean up these, but it takes time. I am not sure this driver is in the front
of queue.

But I help will review if I see these patches.

Frank

>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Borislav Petkov]

On Fri, Jul 31, 2026 at 10:01:57AM -0500, Frank Li wrote:
> There are too many pre-existing issues at recent review, we are trying to
> clean up these, but it takes time. I am not sure this driver is in the front
> of queue.
> 
> But I help will review if I see these patches.

Right, I doubt that anyone would jump at fixing those but we'll see. And yes,
I'd appreciate review and testing.

As to fixing those - I'll simply stop taking any new features to the driver
until issues are fixed or addressed or commented on and hopefully, with a team
effort, we'll get them sorted out.

:-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

[Borislav Petkov]

On Tue, Jul 14, 2026 at 04:46:09PM -0700, Rosen Penev wrote:
> Subject: Re: [PATCH] EDAC: fsl_ddr: use devm_platform_ioremap_resource()

Before you send patches in the future, do:

git log -p drivers/edac/fsl_ddr_edac.c

and look at how things are formatted there. For example, the subject prefix
is:

EDAC/fsl_ddr: ...

and so on.

The EDAC tree uses the same rules as the tip tree, you might want to read them
before sending patches:

https://docs.kernel.org/process/maintainer-tip.html

> Replace the open-coded of_address_to_resource() plus devm_request_mem_region()
> and devm_ioremap() sequence with devm_platform_ioremap_resource(), which folds
> the resource lookup, region reservation and mapping into one step and returns
> an ERR_PTR checked with IS_ERR()/PTR_ERR().
> 
> Behaviorally equivalent with respect to region reservation: the driver
> already reserved the region, so the non-overlapping reg requirement was
> already satisfied. Drop the now-unused linux/of_address.h include.
> 
> Built for arm64 (defconfig + CONFIG_EDAC_FSL_DDR) with LLVM=1;
> drivers/edac/fsl_ddr_edac.o compiles cleanly.

Testing blurb belongs ...

> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

... here, under the three "---" so that it doesn't land in the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette