Re: [PATCHv2 1/3] EDAC/fsl_ddr: use devm_platform_ioremap_resource()

[Frank Li]

On Thu, Jul 30, 2026 at 06:21:28PM -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.
>
> Remove devres stuff. Not needed. devm is already used.
>
> Fix a mistaken return code of -ENOMEM when PTR_ERR should be used.
>
> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---

Need change log here.

You missed my review-by and tested-by tags. If you delete it by some reason
like big changes, you should said here explicit.

Frank

>  drivers/edac/fsl_ddr_edac.c | 39 +++++++------------------------------
>  1 file changed, 7 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index 2a545e5812e7..3745eec67335 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,13 +494,14 @@ 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;
>
> -	if (!devres_open_group(&op->dev, fsl_mc_err_probe, GFP_KERNEL))
> -		return -ENOMEM;
> +	mc_vbase = devm_platform_ioremap_resource(op, 0);
> +	if (IS_ERR(mc_vbase))
> +		return PTR_ERR(mc_vbase);
>
>  	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>  	layers[0].size = 4;
> @@ -511,10 +511,8 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	layers[1].is_virt_csrow = false;
>  	mci = edac_mc_alloc(edac_mc_idx, ARRAY_SIZE(layers), layers,
>  			    sizeof(*pdata));
> -	if (!mci) {
> -		devres_release_group(&op->dev, fsl_mc_err_probe);
> +	if (!mci)
>  		return -ENOMEM;
> -	}
>
>  	pdata = mci->pvt_info;
>  	pdata->name = "fsl_mc_err";
> @@ -531,33 +529,12 @@ 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");
>  		if (IS_ERR(pdata->inject_vbase)) {
> -			res = -ENOMEM;
> +			res = PTR_ERR(pdata->inject_vbase);
>  			goto err;
>  		}
>  	}
> @@ -637,7 +614,6 @@ int fsl_mc_err_probe(struct platform_device *op)
>  		       pdata->irq);
>  	}
>
> -	devres_remove_group(&op->dev, fsl_mc_err_probe);
>  	edac_dbg(3, "success\n");
>  	pr_info(EDAC_MOD_STR " MC err registered\n");
>
> @@ -646,7 +622,6 @@ int fsl_mc_err_probe(struct platform_device *op)
>  err2:
>  	edac_mc_del_mc(&op->dev);
>  err:
> -	devres_release_group(&op->dev, fsl_mc_err_probe);
>  	edac_mc_free(mci);
>  	return res;
>  }
> --
> 2.55.0
>
>