RE: [PATCH] Revert "pinctrl: freescale: imx: Use 'devm_of_iomap()' to avoid a resource leak in case of error in 'imx_pinctrl_probe()'"

[BOUGH CHEN <haibo.chen@xxxxxxx>]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> Sent: 2020年6月8日 22:45
> To: Aisheng Dong <aisheng.dong@xxxxxxx>
> Cc: BOUGH CHEN <haibo.chen@xxxxxxx>; festevam@xxxxxxxxx;
> shawnguo@xxxxxxxxxx; stefan@xxxxxxxx; kernel@xxxxxxxxxxxxxx;
> linus.walleij@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>;
> linux-gpio@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] Revert "pinctrl: freescale: imx: Use 'devm_of_iomap()' to
> avoid a resource leak in case of error in 'imx_pinctrl_probe()'"
> 
> On Mon, Jun 08, 2020 at 02:06:35PM +0000, Aisheng Dong wrote:
> > > From: haibo.chen@xxxxxxx <haibo.chen@xxxxxxx>
> > > Sent: Monday, June 8, 2020 6:00 PM
> > >
> > > This patch block system booting, find on imx7d-sdb board.
> > > From the dts we can see, iomux and iomux_lpsr share the memory
> > > region [0x30330000-0x3033ffff], so will trigger the following issue:
> > >
> > > [    0.179561] imx7d-pinctrl 302c0000.iomuxc-lpsr: initialized IMX pinctrl
> > > driver
> > > [    0.191742] imx7d-pinctrl 30330000.pinctrl: can't request region for
> > > resource [mem 0x30330000-0x3033ffff]
> > > [    0.191842] imx7d-pinctrl: probe of 30330000.pinctrl failed with error
> -16
> > >
> > > This reverts commit ba403242615c2c99e27af7984b1650771a2cc2c9.
> >
> > Better add your sign-off.
> > Otherwise:
> > Reviewed-by: Dong Aisheng <aisheng.dong@xxxxxxx>
> >
> > Maybe you or Christophe could resubmit another proper fix for the original
> issue.
> 
> I'm really sorry about that.  This was largely my fault.
> 
> I still don't understand how commit ba403242615c caused a problem.
> 
> It sounds like in the original code ipctl->input_sel_base was released somehow?
> I do a `git grep input_sel_base` and it doesn't show anything.
> What am I missing?
> 

Hi Dan,

I can give you a detail explain why this patch trigger issues.
Take our imx7d-sdb board as example, in dts file we can see to nodes:
440                         iomuxc_lpsr: iomuxc-lpsr@302c0000 {
441                                 compatible = "fsl,imx7d-iomuxc-lpsr";
442                                 reg = <0x302c0000 0x10000>;
443                                 fsl,input-sel = <&iomuxc>;
444                         };
And 
493                         iomuxc: pinctrl@30330000 {
494                                 compatible = "fsl,imx7d-iomuxc";
495                                 reg = <0x30330000 0x10000>;
496                         };

First time when probe the iomuxc_lpsr, due to it has "fsl,input-sel", so in the pinctrl driver, it will use API devm_of_iomap() to handle the iomuxc for the first time. Devm_of_iomap() use the API devm_request_mem_region() for the region <0x30330000-0x3033ffff>. 
Then, when probe the iomuxc, the pinctrl driver call the API devm_platform_ioremap_resource(), this API also call the devm_request_mem_region() for the same region <0x30330000-0x3033ffff>. 
So we see the log as following:
[    0.179561] imx7d-pinctrl 302c0000.iomuxc-lpsr: initialized IMX pinctrl driver
[    0.191742] imx7d-pinctrl 30330000.pinctrl: can't request region for resource [mem 0x30330000-0x3033ffff]
[    0.191842] imx7d-pinctrl: probe of 30330000.pinctrl failed with error -16

I will send a V2 patch to add my sign-off-by and fix tag.

Best Regards
Haibo Chen
> regards,
> dan carpenter