Re: [PATCH] powerpc: populate the default bus with machine_arch_initcall

[Kevin Hao <haokexin@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4073 bytes --]

On Thu, Aug 11, 2016 at 08:17:52AM -0500, Rob Herring wrote:
> On Thu, Aug 11, 2016 at 6:09 AM, Kevin Hao <haokexin@gmail.com> wrote:
> > With the commit 44a7185c2ae6 ("of/platform: Add common method to
> > populate default bus"), a default function is introduced to populate
> > the default bus and this function is invoked at the arch_initcall_sync
> > level. This will override the arch specific population of default bus
> > which run at a lower level than arch_initcall_sync. Since not all
> > powerpc specific buses are added to the of_default_bus_match_table[],
> > this causes some powerpc specific bus are not probed. Fix this by
> > using a more preceding initcall.
> >
> > Signed-off-by: Kevin Hao <haokexin@gmail.com>
> > ---
> > Of course we can adjust the powerpc arch codes to use the
> > of_platform_default_populate_init(), but it has high risk to break
> > other boards given the complicated powerpc specific buses. So I would
> > like just to fix the broken boards in the current release, and cook
> > a patch to change to of_platform_default_populate_init() for linux-next.
> 
> The patch that broke things was sitting in -next for some time and no
> one reported anything. Are all these boards broken?

At least in theory. :-)
The effect may be different due to what devices are missed. For me, the
Gianfar Ethernet on my mpc8315erdb board is malfunction due to the MIDIO bus
is not probed.

> 
> I'm fine to just disable the default call for PPC instead if there's
> some chance this does not fix some boards.

I have tried to cover all the invocation of of_platform_bus_probe() via
machine_device_initcall(). Yes, I maybe missed some boards. But won't
we want to take this as a step to use the default populate function since
it does remove some reduplication codes?

> There could be some other
> initcall ordering dependencies.
> 
> >
> > Only boot test on a mpc8315erdb board.
> 
> Curious, what would it take to remove the of_platform_bus_probe and
> use the default here? We can add additional bus compatibles to match.

I thought about this. But the bus compatibles list seems a bit longer and
it may cause some side effects on some boards due to all these additional
buses. So that changes seem a bit aggressive to me. It does seem a feature
for linux-next. The following is the compatible buses list which are needed
to be added to the default match table if we want fix all the current broken
boards:
	{ .compatible = "fsl,ep8248e-bcsr", },
	{ .compatible = "fsl,pq2pro-localbus", },
	{ .compatible = "fsl,qe", },
	{ .compatible = "fsl,srio", },
	{ .compatible = "gianfar", },
	{ .compatible = "gpio-leds", },
	{ .compatible = "hawk-bridge", },
	{ .compatible = "ibm,ebc", },
	{ .compatible = "ibm,opb", },
	{ .compatible = "ibm,plb3", },
	{ .compatible = "ibm,plb4", },
	{ .compatible = "ibm,plb6", },
	{ .compatible = "nintendo,flipper", },
	{ .compatible = "nintendo,hollywood", },
	{ .compatible = "pasemi,localbus", },
	{ .compatible = "pasemi,sdc", },
	{ .compatible = "soc", },
	{ .compatible = "xlnx,compound", },
	{ .compatible = "xlnx,dcr-v29-1.00.a", },
	{ .compatible = "xlnx,opb-v20-1.10.c", },
	{ .compatible = "xlnx,plb-v34-1.01.a", },
	{ .compatible = "xlnx,plb-v34-1.02.a", },
	{ .compatible = "xlnx,plb-v46-1.00.a", },
	{ .compatible = "xlnx,plb-v46-1.02.a", },
	{ .name = "cpm", },
	{ .name = "localbus", },
	{ .name = "soc", },
	{ .type = "axon", },
	{ .type = "ebc", },
	{ .type = "opb", },
	{ .type = "plb4", },
	{ .type = "plb5", },
	{ .type = "qe", },
	{ .type = "soc", },
	{ .type = "spider", },

Of course I can choose to use the default function if all you guys think it is
better. :-)

> The difference between of_platform_bus_probe and
> of_platform_bus_populate is the former will match root nodes with no
> compatible string. Most platforms should not need that behavior and it
> would be nice to know which ones.

I don't think this difference would cause any real side effect for these boards.

Thanks,
Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]