lkml.org 
[lkml]   [2020]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2 16/17] driver core: Refactor fw_devlink feature
    On 2020-12-11 17:51, Saravana Kannan wrote:
    > On Fri, Dec 11, 2020 at 8:34 AM Robin Murphy <robin.murphy@arm.com>
    > wrote:
    >>
    >> On 2020-12-11 14:11, Qian Cai wrote:
    >> > On Fri, 2020-11-20 at 18:02 -0800, Saravana Kannan wrote:
    >> >> The current implementation of fw_devlink is very inefficient because it
    >> >> tries to get away without creating fwnode links in the name of saving
    >> >> memory usage. Past attempts to optimize runtime at the cost of memory
    >> >> usage were blocked with request for data showing that the optimization
    >> >> made significant improvement for real world scenarios.
    >> >>
    >> >> We have those scenarios now. There have been several reports of boot
    >> >> time increase in the order of seconds in this thread [1]. Several OEMs
    >> >> and SoC manufacturers have also privately reported significant
    >> >> (350-400ms) increase in boot time due to all the parsing done by
    >> >> fw_devlink.
    >> >>
    >> >> So this patch uses all the setup done by the previous patches in this
    >> >> series to refactor fw_devlink to be more efficient. Most of the code has
    >> >> been moved out of firmware specific (DT mostly) code into driver core.
    >> >>
    >> >> This brings the following benefits:
    >> >> - Instead of parsing the device tree multiple times during bootup,
    >> >> fw_devlink parses each fwnode node/property only once and creates
    >> >> fwnode links. The rest of the fw_devlink code then just looks at these
    >> >> fwnode links to do rest of the work.
    >> >>
    >> >> - Makes it much easier to debug probe issue due to fw_devlink in the
    >> >> future. fw_devlink=on blocks the probing of devices if they depend on
    >> >> a device that hasn't been added yet. With this refactor, it'll be very
    >> >> easy to tell what that device is because we now have a reference to
    >> >> the fwnode of the device.
    >> >>
    >> >> - Much easier to add fw_devlink support to ACPI and other firmware
    >> >> types. A refactor to move the common bits from DT specific code to
    >> >> driver core was in my TODO list as a prerequisite to adding ACPI
    >> >> support to fw_devlink. This series gets that done.
    >> >>
    >> >> [1] - https://lore.kernel.org/linux-omap/ea02f57e-871d-cd16-4418-c1da4bbc4696@ti.com/
    >> >> Signed-off-by: Saravana Kannan <saravanak@google.com>
    >> >> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    >> >> Tested-by: Grygorii Strashko <grygorii.strashko@ti.com>
    >> >
    >> > Reverting this commit and its dependency:
    >> >
    >> > 2d09e6eb4a6f driver core: Delete pointless parameter in fwnode_operations.add_links
    >> >
    >> > from today's linux-next fixed a boot crash on an arm64 Thunder X2 server.
    >>
    >> Since the call stack implicates the platform-device-wrangling we do in
    >> IORT code I took a quick look; AFAICS my guess would be it's blowing
    >> up
    >> trying to walk a zeroed list head since "driver core: Add
    >> fwnode_init()"
    >> missed acpi_alloc_fwnode_static().
    >
    > Thanks Robin. I'm pretty sure this is the reason. I thought I fixed
    > all ACPI cases, but clearly I missed this one. I'll send out a patch
    > for this today. If you think there are any other places I missed
    > please let me know. I'll try some git grep foo to see if I missed any
    > other instances of fwnode ops being set.

    Yup, that fixed it here (QDF2400).

    Thanks,

    M.

    diff --git a/include/linux/acpi.h b/include/linux/acpi.h
    index 39263c6b52e1..2630c2e953f7 100644
    --- a/include/linux/acpi.h
    +++ b/include/linux/acpi.h
    @@ -55,7 +55,7 @@ static inline struct fwnode_handle
    *acpi_alloc_fwnode_static(void)
    if (!fwnode)
    return NULL;

    - fwnode->ops = &acpi_static_fwnode_ops;
    + fwnode_init(fwnode, &acpi_static_fwnode_ops);

    return fwnode;
    }


    --
    Jazz is not dead. It just smells funny...

    \
     
     \ /
      Last update: 2020-12-11 19:06    [W:4.086 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site