Messages in this thread Patch in this message | | | Subject | Re: [PATCH v1] of: platform: Batch fwnode parsing in the init_machine() path | From | Grygorii Strashko <> | Date | Fri, 2 Oct 2020 21:35:49 +0300 |
| |
hi Saravana,
On 02/10/2020 21:27, Laurent Pinchart wrote: > Hi Saravana, > > On Fri, Oct 02, 2020 at 10:58:55AM -0700, Saravana Kannan wrote: >> On Fri, Oct 2, 2020 at 10:55 AM Laurent Pinchart wrote: >>> On Fri, Oct 02, 2020 at 10:51:51AM -0700, Saravana Kannan wrote: >>>> On Fri, Oct 2, 2020 at 7:08 AM Rob Herring <robh+dt@kernel.org> wrote: >>>>> On Thu, Oct 1, 2020 at 5:59 PM Saravana Kannan <saravanak@google.com> 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(). >>>>>> >>>>>> Reported-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com> >>>>>> --- >>>>>> drivers/of/platform.c | 19 +++++++++++++++---- >>>>>> 1 file changed, 15 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c >>>>>> index 071f04da32c8..79972e49b539 100644 >>>>>> --- a/drivers/of/platform.c >>>>>> +++ b/drivers/of/platform.c >>>>>> @@ -501,8 +501,21 @@ int of_platform_default_populate(struct device_node *root, >>>>>> const struct of_dev_auxdata *lookup, >>>>>> struct device *parent) >>>>>> { >>>>>> - return of_platform_populate(root, of_default_bus_match_table, lookup, >>>>>> - parent); >>>>>> + int ret; >>>>>> + >>>>>> + /* >>>>>> + * fw_devlink_pause/resume() are only safe to be called around top >>>>>> + * level device addition due to locking constraints. >>>>>> + */ >>>>>> + if (!root) >>>>>> + fw_devlink_pause(); >>>>>> + >>>>>> + ret = of_platform_populate(root, of_default_bus_match_table, lookup, >>>>>> + parent); >>>>> >>>>> of_platform_default_populate() vs. of_platform_populate() is just a >>>>> different match table. I don't think the behavior should otherwise be >>>>> different. >>>>> >>>>> There's also of_platform_probe() which has slightly different matching >>>>> behavior. It should not behave differently either with respect to >>>>> devlinks. >>>> >>>> So I'm trying to do this only when the top level devices are added for >>>> the first time. of_platform_default_populate() seems to be the most >>>> common path. For other cases, I think we just need to call >>>> fw_devlink_pause/resume() wherever the top level devices are added for >>>> the first time. As I said in the other email, we can't add >>>> fw_devlink_pause/resume() by default to of_platform_populate(). >>>> >>>> Do you have other ideas for achieving "call fw_devlink_pause/resume() >>>> only when top level devices are added for the first time"? >>> >>> I'm not an expert in this domain, but before investigating it, would you >>> be able to share a hack patch that implements this (in the most simple >>> way) to check if it actually fixes the delays I experience on my system >>> ? >> >> So I take it the patch I sent out didn't work for you? Can you tell me >> what machine/DT you are using? > > I've replied to the patch: > > 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? > > I'm using an AM57xx EVM, whose DT is not upstream, but it's essentially > a am57xx-beagle-x15-revb1.dts (it includes that DTS) with a few > additional nodes for GPIO keys, LCD panel, backlight and touchscreen. >
hope you are receiving my mails as I've provided you with all required information already [1]
with below diff: [ 4.177231] Freeing unused kernel memory: 1024K [ 4.181892] Run /sbin/init as init process
The best time with [2] is [ 3.100483] Run /sbin/init as init process
Still 1 sec lose.
Pls understand an issue - requirements here are like 500ms boot with can, Ethernet, camera and display on ;(
[1] https://lore.kernel.org/patchwork/patch/1316134/#1511276 [2] https://lore.kernel.org/patchwork/patch/1316134/#1511435
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c index 2a4fe3e68b82..ac1ab8928190 100644 --- a/arch/arm/mach-omap2/pdata-quirks.c +++ b/arch/arm/mach-omap2/pdata-quirks.c @@ -591,7 +591,9 @@ void __init pdata_quirks_init(const struct of_device_id *omap_dt_match_table) if (of_machine_is_compatible("ti,omap3")) omap3_mcbsp_init(); pdata_quirks_check(auxdata_quirks); + fw_devlink_pause(); of_platform_populate(NULL, omap_dt_match_table, omap_auxdata_lookup, NULL); + fw_devlink_resume(); pdata_quirks_check(pdata_quirks); }
-- Best regards, grygorii
| |