| Message ID | 20160812054934.GJ17275@pek-khao-d1 (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Fri, Aug 12, 2016 at 12:49 AM, Kevin Hao <haokexin@gmail.com> wrote: > On Fri, Aug 12, 2016 at 02:39:32PM +1000, Michael Ellerman wrote: >> Kevin Hao <haokexin@gmail.com> writes: [...] >> Every one of those initcall changes could be introducing a bug, by >> changing the order vs other init calls. >> >> Can we just go back to the old behaviour on ppc? > > Sure. How about this one? > > From 4362b4cdd8a6198df4cc46c628473f0d44e03fa8 Mon Sep 17 00:00:00 2001 > From: Kevin Hao <haokexin@gmail.com> > Date: Fri, 12 Aug 2016 13:30:03 +0800 > Subject: [PATCH v2] of/platform: disable the > of_platform_default_populate_init() for all the ppc boards > > 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. But a lot of ppc boards use machine_device_initcall() to > populate the default bus. This means that the default populate function > has higher priority and would override the arch specific population of > the bus. The side effect is that some arch specific bus are not probed, > then cause various malfunction due to the miss of some devices. Since > it is very possible to introduce bugs if we simply change the initcall > level for all these boards(about 30+). This just disable this default > function for all the ppc boards. > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > --- > drivers/of/platform.c | 2 ++ > 1 file changed, 2 insertions(+) I've applied this one. Rob
On Fri, 2016-08-12 at 14:30 -0500, Rob Herring 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. But a lot of ppc boards use machine_device_initcall() to > > populate the default bus. This means that the default populate > > function > > has higher priority and would override the arch specific population > > of > > the bus. The side effect is that some arch specific bus are not > > probed, > > then cause various malfunction due to the miss of some devices. > > Since > > it is very possible to introduce bugs if we simply change the > > initcall > > level for all these boards(about 30+). This just disable this > > default > > function for all the ppc boards. > > > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > > --- > > drivers/of/platform.c | 2 ++ > > 1 file changed, 2 insertions(+) > > I've applied this one. Not fan of the hard #ifdef at all... it will make it hard to convert platforms one by one. Why not an arch_want_default_of_probe() or something like this which we can then plumb into ppc_md. ? Cheers, Ben.
On Sun, Aug 14, 2016 at 4:21 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Fri, 2016-08-12 at 14:30 -0500, Rob Herring 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. But a lot of ppc boards use machine_device_initcall() to >> > populate the default bus. This means that the default populate >> > function >> > has higher priority and would override the arch specific population >> > of >> > the bus. The side effect is that some arch specific bus are not >> > probed, >> > then cause various malfunction due to the miss of some devices. >> > Since >> > it is very possible to introduce bugs if we simply change the >> > initcall >> > level for all these boards(about 30+). This just disable this >> > default >> > function for all the ppc boards. >> > >> > Signed-off-by: Kevin Hao <haokexin@gmail.com> >> > --- >> > drivers/of/platform.c | 2 ++ >> > 1 file changed, 2 insertions(+) >> >> I've applied this one. > > Not fan of the hard #ifdef at all... it will make it hard to convert > platforms one by one. Why not an arch_want_default_of_probe() or > something like this which we can then plumb into ppc_md. ? Yeah, I'm not either really. I didn't propose using IS_ENABLED here either as it does add a needless initcall. Do you have any idea which platforms are not broken and could be switched over now. If not, I think we should just leave as is for 4.8 and make it per platform selectable for 4.9. Rob
On Sun, 2016-08-14 at 17:42 -0500, Rob Herring wrote: > > Not fan of the hard #ifdef at all... it will make it hard to > convert > > platforms one by one. Why not an arch_want_default_of_probe() or > > something like this which we can then plumb into ppc_md. ? > > Yeah, I'm not either really. I didn't propose using IS_ENABLED here > either as it does add a needless initcall. > > Do you have any idea which platforms are not broken and could be > switched over now. If not, I think we should just leave as is for 4.8 > and make it per platform selectable for 4.9. Not really... the non-embedded platforms (mac, pseries, etc...) don't really use the OF bus probing iirc, so it's going to be a matter of scrubbing the embedded ones. I agree we can do that in 4.9 Cheers, Ben.
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 8aa197691074..f39ccd5aa701 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -497,6 +497,7 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); +#ifndef CONFIG_PPC static int __init of_platform_default_populate_init(void) { struct device_node *node; @@ -521,6 +522,7 @@ static int __init of_platform_default_populate_init(void) return 0; } arch_initcall_sync(of_platform_default_populate_init); +#endif static int of_platform_device_destroy(struct device *dev, void *data) {