RE: [PATCH 1/2] ARM: imx: drop uneccessary of_platform_default_populate

[Peng Fan]

Hi Aisheng,

> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019年3月15日 18:17
> To: Peng Fan <peng.fan@xxxxxxx>; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> dl-linux-imx <linux-imx@xxxxxxx>; Anson Huang <anson.huang@xxxxxxx>;
> arnd@xxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Cc: van.freenix@xxxxxxxxx
> Subject: RE: [PATCH 1/2] ARM: imx: drop uneccessary
> of_platform_default_populate
> 
> > From: Peng Fan
> >
> > "arch_initcall_sync(of_platform_default_populate_init);" could be used
> > to populate the device tree, there is no need to call
> > of_platform_default_populate in machine code.
> >
> > Tested on i.MX6Q-SDB i.MX6SL-EVK i.MX6UL-EVK board.
> >
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >  arch/arm/mach-imx/mach-imx6q.c  | 2 --
> > arch/arm/mach-imx/mach-imx6sl.c | 2 --
> > arch/arm/mach-imx/mach-imx6sx.c | 2 --
> > arch/arm/mach-imx/mach-imx6ul.c | 1 -
> >  4 files changed, 7 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c
> > b/arch/arm/mach-imx/mach-imx6q.c index 7d80a0ae723c..655398c20256
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -278,8 +278,6 @@ static void __init imx6q_init_machine(void)
> >
> >  	imx6q_enet_phy_init();
> >
> > -	of_platform_default_populate(NULL, NULL, parent);
> > -
> 
> This slightly change the device register sequence as well as possible driver
> probe sequence as a consequence.

This patch is inspired from patch:

commit 850bea2335e42780a0752a75860d3fbcc3d12d6e
Author: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
Date:   Wed Jun 1 14:52:56 2016 +0800

    arm: Remove unnecessary of_platform_populate with default match table

    After patch "of/platform: Add common method to populate default bus",
    it is possible for arch code to remove unnecessary callers of
    of_platform_populate with default match table.

> 
> Originally devices are registered in arch_initcall. Now it will be a bit later in
> arch_initcall_sync and this may cause a bit risk if the code under the
> default_populate want to access the device service provided by early probe.
> 
> Probably it's more safe to leave as it is unless you can double confirm there're
> no such code depends on accessing early probed devices as follows before we
> can make the change.

This has been boot tested on 6Q-SDB/6UL-EVK/6SL-EVK board.
For i.MX, I only see pinctrl driver use arch_initcall from the link,
https://elixir.bootlin.com/linux/latest/ident/arch_initcall

From my boot test, the pinctrl driver probe will be delayed a bit, but I do
not see issues.

The common code is introduced by this patch, one thing need to consider
is the of_iommu_init and swiotlb_late_init on aarch64. My patch
is only for i.MX6/7, so I think there is no such issue.
commit 44a7185c2ae6383c88ff5b1ef2e2969f35d7b8b7
Author: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
Date:   Wed Jun 1 14:52:54 2016 +0800

    of/platform: Add common method to populate default bus

    The arch code calls of_platform_populate() with default match table
    when it wants to populate default bus.

    This patch introduce a new of_platform_default_populate_init() and make it
    arch_initcall_sync(it should be later than some iommu configration, eg,
    of_iommu_init() and swiotlb_late_init in arm64), then we can finish above
    job in common method.

    In order to avoid the default bus being populated twice, simply checking
    the flag of bus node whether has be set OF_POPULATED_BUS or not.

    After that, we can safely remove the caller in arch code.

    Btw, add debug print in of_platform_populate(), and use __func__ to
    print function's name of of_platform_bus_create().


Thanks,
Peng.

> 
> >  	imx_anatop_init();
> >  	cpu_is_imx6q() ?  imx6q_pm_init() : imx6dl_pm_init();
> >  	imx6q_1588_init();
> 
> Regards
> Dong Aisheng
> 
> > diff --git a/arch/arm/mach-imx/mach-imx6sl.c
> > b/arch/arm/mach-imx/mach-imx6sl.c index 99be4225297a..9743bdbb68fa
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6sl.c
> > +++ b/arch/arm/mach-imx/mach-imx6sl.c
> > @@ -56,8 +56,6 @@ static void __init imx6sl_init_machine(void)
> >  	if (parent == NULL)
> >  		pr_warn("failed to initialize soc device\n");
> >
> > -	of_platform_default_populate(NULL, NULL, parent);
> > -
> >  	if (cpu_is_imx6sl())
> >  		imx6sl_fec_init();
> >  	imx_anatop_init();
> > diff --git a/arch/arm/mach-imx/mach-imx6sx.c
> > b/arch/arm/mach-imx/mach-imx6sx.c index 7f52d9b1e8a4..19b9f2dd309e
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6sx.c
> > +++ b/arch/arm/mach-imx/mach-imx6sx.c
> > @@ -72,8 +72,6 @@ static void __init imx6sx_init_machine(void)
> >  	if (parent == NULL)
> >  		pr_warn("failed to initialize soc device\n");
> >
> > -	of_platform_default_populate(NULL, NULL, parent);
> > -
> >  	imx6sx_enet_init();
> >  	imx_anatop_init();
> >  	imx6sx_pm_init();
> > diff --git a/arch/arm/mach-imx/mach-imx6ul.c
> > b/arch/arm/mach-imx/mach-imx6ul.c index 6cb8a22b617d..c57b9df791b1
> > 100644
> > --- a/arch/arm/mach-imx/mach-imx6ul.c
> > +++ b/arch/arm/mach-imx/mach-imx6ul.c
> > @@ -65,7 +65,6 @@ static void __init imx6ul_init_machine(void)
> >  	if (parent == NULL)
> >  		pr_warn("failed to initialize soc device\n");
> >
> > -	of_platform_default_populate(NULL, NULL, parent);
> >  	imx6ul_enet_init();
> >  	imx_anatop_init();
> >  	imx6ul_pm_init();
> > --
> > 2.16.4