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 21:11:21 +0300 |
| |
On 02/10/2020 20:48, Saravana Kannan wrote: > On Fri, Oct 2, 2020 at 8:03 AM 'Grygorii Strashko' via kernel-team > <kernel-team@android.com> wrote: >> >> >> >> 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); > > This will mean no device will probe until device_initcall_sync(). > Unfortunately, I don't think we can make such a sweeping assumption.
Could you answer below questions, pls? >>> By the way, is there option to disable this feature at all? >>> Is there Kconfig option?
-- Best regards, grygorii
| |