Messages in this thread | | | Subject | Re: [PATCH v1] of: platform: Batch fwnode parsing in the init_machine() path | From | Grygorii Strashko <> | Date | Fri, 2 Oct 2020 18:03:48 +0300 |
| |
On 02/10/2020 14:40, Grygorii Strashko wrote: > > > On 02/10/2020 02:19, Laurent Pinchart wrote: >> Hi Saravana, >> >> Thank you for the patch. >> >> On Thu, Oct 01, 2020 at 03:59:51PM -0700, Saravana Kannan wrote: >>> When commit 93d2e4322aa7 ("of: platform: Batch fwnode parsing when >>> adding all top level devices") optimized the fwnode parsing when all top >>> level devices are added, it missed out optimizing this for platform >>> where the top level devices are added through the init_machine() path. >>> >>> This commit does the optimization for all paths by simply moving the >>> fw_devlink_pause/resume() inside of_platform_default_populate(). >> >> Based on v5.9-rc5, before the patch: >> >> [ 0.652887] cpuidle: using governor menu >> [ 12.349476] No ATAGs? >> >> After the patch: >> >> [ 0.650460] cpuidle: using governor menu >> [ 12.262101] No ATAGs? >> >> :-( > > This is kinda expected :( because omap2 arch doesn't call of_platform_default_populate() > > Call path: > board-generic.c > DT_MACHINE_START() > .init_machine = omap_generic_init, > > omap_generic_init() > pdata_quirks_init(omap_dt_match_table); > of_platform_populate(NULL, omap_dt_match_table, > omap_auxdata_lookup, NULL); > > Other affected platforms > arm: mach-ux500 > some mips > some powerpc > > there are also case when a lot of devices placed under bus node, in such case > of_platform_populate() calls from bus drivers will also suffer from this issue. > > I think one option could be to add some parameter to _populate() or introduce new api. > > By the way, is there option to disable this feature at all? > Is there Kconfig option? > Is there any reasons why such complex and time consuming code added to the kernel and not implemented on DTC level? > > > Also, I've came with another diff, pls check. > > [ 0.000000] Booting Linux on physical CPU 0x0 > [ 0.000000] Linux version 5.9.0-rc6-01791-g9acba6b38757-dirty (grygorii@grygorii-XPS-13-9370) (arm-linux-gnueabihf-gcc (GNU Toolcha0 > [ 0.000000] CPU: ARMv7 Processor [412fc0f2] revision 2 (ARMv7), cr=10c5387d > [ 0.000000] CPU: div instructions available: patching division code > [ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache > [ 0.000000] OF: fdt: Machine model: TI AM5718 IDK > ... > [ 0.053443] cpuidle: using governor ladder > [ 0.053470] cpuidle: using governor menu > [ 0.089304] No ATAGs? > ... > [ 3.092291] devtmpfs: mounted > [ 3.095804] Freeing unused kernel memory: 1024K > [ 3.100483] Run /sbin/init as init process > > > > ------ >< --- > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 071f04da32c8..4521b26e7745 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -514,6 +514,12 @@ static const struct of_device_id reserved_mem_matches[] = { > {} > }; > > +static int __init of_platform_fw_devlink_pause(void) > +{ > + fw_devlink_pause(); > +} > +core_initcall(of_platform_fw_devlink_pause); > + > static int __init of_platform_default_populate_init(void) > { > struct device_node *node; > @@ -538,9 +544,7 @@ static int __init of_platform_default_populate_init(void) > } > > /* Populate everything else. */ > - fw_devlink_pause(); > of_platform_default_populate(NULL, NULL, NULL); > - fw_devlink_resume(); > > return 0; > } > @@ -548,6 +552,7 @@ arch_initcall_sync(of_platform_default_populate_init); > > static int __init of_platform_sync_state_init(void) > { > + fw_devlink_resume();
^ it seems has to be done earlier, like +static int __init of_platform_fw_devlink_resume(void) +{ + fw_devlink_resume(); + return 0; +} +device_initcall_sync(of_platform_fw_devlink_resume);
> device_links_supplier_sync_state_resume(); > return 0; > } > > >
-- Best regards, grygorii
| |