Re: [PATCH 08/17] clk: imx: convert to devm_platform_ioremap_resource

[Leonard Crestez <leonard.crestez@nxp.com>]

On 09.12.2019 21:58, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
>   drivers/clk/imx/clk-imx8qxp-lpcg.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> index c0aff7ca6374..10ae712447c6 100644
> --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
> +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
> @@ -164,7 +164,6 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>   	struct clk_hw_onecell_data *clk_data;
>   	const struct imx8qxp_ss_lpcg *ss_lpcg;
>   	const struct imx8qxp_lpcg_data *lpcg;
> -	struct resource *res;
>   	struct clk_hw **clks;
>   	void __iomem *base;
>   	int i;
> @@ -173,10 +172,7 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
>   	if (!ss_lpcg)
>   		return -ENODEV;
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res)
> -		return -EINVAL;
> -	base = devm_ioremap(dev, res->start, resource_size(res));
> +	base = devm_platform_ioremap_resource(pdev, 0);
>   	if (!base)
>   		return -ENOMEM;

This breaks imx8qxp-mek boot by causing most peripherals (like uart) to 
fail to probe.

The old and new paths are not equivalent: devm_platform_ioremap_resource
calls devm_ioremap_resource which differs from devm_ioremap by also 
calling devm_request_mem_region.

This prevents other mappings in the area and imx8qxp-lpcg nodes map 
whole hardware "subsystems" and overlap most peripherals. For example:

                  adma_lpcg: clock-controller@59000000 {
                          compatible = "fsl,imx8qxp-lpcg-adma";
                          reg = <0x59000000 0x2000000>;
                          #clock-cells = <1>;
		};

                 adma_lpuart0: serial@5a060000 {
			reg = <0x5a060000 0x1000>;
			...
		};

I don't know if this issue affects any other platforms (imx8 lpcg 
bindings are unusual) but if you found this with an automated tool 
perhaps it should be adjusted?

By my count it's the 4th time this incorrect cleanup was posted.

Previously: https://lkml.org/lkml/2019/12/4/487

--
Regards,
Leonard