Re: [PATCHv2 2/4] memory: fsl_ifc: Use devm_platform_ioremap_resource and fix error paths

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/07/2026 22:24, Rosen Penev wrote:
> Replace of_iomap() with devm_platform_ioremap_resource() for automatic
> resource management, eliminating manual iounmap in remove and error
> paths. devm_ioremap_resource() reserves the region and checks for
> overlaps, which is safe here since the IFC controller register space
> and child chip-select ranges are always at distinct addresses in all
> DTS files.
> 
> As one of the errors is -EPROBE_DEFER, run it before anything else to
> avoid doing actual work in such a case.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/memory/fsl_ifc.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> index dce94d8b05b9..7e6800fc8480 100644
> --- a/drivers/memory/fsl_ifc.c
> +++ b/drivers/memory/fsl_ifc.c
> @@ -92,10 +92,6 @@ static void fsl_ifc_ctrl_remove(struct platform_device *dev)
>  	if (ctrl->nand_irq > 0)
>  		free_irq(ctrl->nand_irq, ctrl);
>  	free_irq(ctrl->irq, ctrl);
> -
> -	iounmap(ctrl->gregs);
> -
> -	dev_set_drvdata(&dev->dev, NULL);
>  }
>  
>  /*
> @@ -201,6 +197,7 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  {
>  	int ret = 0;
>  	int version, banks;
> +	void __iomem *gregs;

Why another local variable?

>  	void __iomem *addr;
>  	int nand_irq;
>  	int irq;
> @@ -217,6 +214,11 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  	if (nand_irq == -EPROBE_DEFER)
>  		return nand_irq;
>  
> +	/* IOMAP the entire IFC region */
> +	gregs = devm_platform_ioremap_resource(dev, 0);
> +	if (IS_ERR(gregs))
> +		return PTR_ERR(gregs);
> +
>  	fsl_ifc_ctrl_dev = devm_kzalloc(&dev->dev, sizeof(*fsl_ifc_ctrl_dev),
>  					GFP_KERNEL);
>  	if (!fsl_ifc_ctrl_dev)
> @@ -224,12 +226,7 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  
>  	dev_set_drvdata(&dev->dev, fsl_ifc_ctrl_dev);
>  
> -	/* IOMAP the entire IFC region */
> -	fsl_ifc_ctrl_dev->gregs = of_iomap(dev->dev.of_node, 0);

This is not equivalent and commit msg does not address that. Old code
allowed multiple maps of the same address space but new code bails with
EBUSY.

Again, I asked last time - how did you test it?

Please mark the patches as RFT.


Best regards,
Krzysztof