Date:   Thu, 23 Jun 2022 11:17:03 -0700
From:   Saravana Kannan <saravanak@...gle.com>
To:     Ahmad Fatoum <a.fatoum@...gutronix.de>
Cc:     sascha hauer <sha@...gutronix.de>, andrew lunn <andrew@...n.ch>,
        peng fan <peng.fan@....com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        linus walleij <linus.walleij@...aro.org>,
        ulf hansson <ulf.hansson@...aro.org>,
        eric dumazet <edumazet@...gle.com>,
        pavel machek <pavel@....cz>, will deacon <will@...nel.org>,
        kevin hilman <khilman@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        joerg roedel <joro@...tes.org>,
        russell king <linux@...linux.org.uk>,
        linux-acpi@...r.kernel.org, jakub kicinski <kuba@...nel.org>,
        paolo abeni <pabeni@...hat.com>, kernel-team@...roid.com,
        Len Brown <lenb@...nel.org>, len brown <len.brown@...el.com>,
        kernel@...gutronix.de, linux-pm@...r.kernel.org,
        linux-gpio@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        hideaki yoshifuji <yoshfuji@...ux-ipv6.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        david ahern <dsahern@...nel.org>, linux-kernel@...r.kernel.org,
        Daniel Scally <djrscally@...il.com>,
        iommu@...ts.linux-foundation.org,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        netdev@...r.kernel.org, "david s. miller" <davem@...emloft.net>,
        devicetree@...r.kernel.org, heiner kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH v2 2/2] of: base: Avoid console probe delay when fw_devlink.strict=1

On Thu, Jun 23, 2022 at 10:36 AM Ahmad Fatoum <a.fatoum@...gutronix.de> wrote:
>
> Hello Saravana,
>
> On 23.06.22 19:26, Saravana Kannan wrote:
> > On Thu, Jun 23, 2022 at 3:05 AM sascha hauer <sha@...gutronix.de> wrote:
> >>
> >> On Thu, Jun 23, 2022 at 01:03:43AM -0700, Saravana Kannan wrote:
> >>> Commit 71066545b48e ("driver core: Set fw_devlink.strict=1 by default")
> >>> enabled iommus and dmas dependency enforcement by default. On some
> >>> systems, this caused the console device's probe to get delayed until the
> >>> deferred_probe_timeout expires.
> >>>
> >>> We need consoles to work as soon as possible, so mark the console device
> >>> node with FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay
> >>> the probe of the console device for suppliers without drivers. The
> >>> driver can then make the decision on where it can probe without those
> >>> suppliers or defer its probe.
> >>>
> >>> Fixes: 71066545b48e ("driver core: Set fw_devlink.strict=1 by default")
> >>> Reported-by: Sascha Hauer <sha@...gutronix.de>
> >>> Reported-by: Peng Fan <peng.fan@....com>
> >>> Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> >>> Tested-by: Peng Fan <peng.fan@....com>
> >>> ---
> >>>  drivers/of/base.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/of/base.c b/drivers/of/base.c
> >>> index d4f98c8469ed..a19cd0c73644 100644
> >>> --- a/drivers/of/base.c
> >>> +++ b/drivers/of/base.c
> >>> @@ -1919,6 +1919,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
> >>>                       of_property_read_string(of_aliases, "stdout", &name);
> >>>               if (name)
> >>>                       of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
> >>> +             if (of_stdout)
> >>> +                     of_stdout->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT;
> >>
> >> The device given in the stdout-path property doesn't necessarily have to
> >> be consistent with the console= parameter. The former is usually
> >> statically set in the device trees contained in the kernel while the
> >> latter is dynamically set by the bootloader. So if you change the
> >> console uart in the bootloader then you'll still run into this trap.
> >>
> >> It's problematic to consult only the device tree for dependencies. I
> >> found several examples of drivers in the tree for which dma support
> >> is optional. They use it if they can, but continue without it when
> >> not available. "hwlock" is another property which consider several
> >> drivers as optional. Also consider SoCs in early upstreaming phases
> >> when the device tree is merged with "dmas" or "hwlock" properties,
> >> but the corresponding drivers are not yet upstreamed. It's not nice
> >> to defer probing of all these devices for a long time.
> >>
> >> I wonder if it wouldn't be a better approach to just probe all devices
> >> and record the device(node) they are waiting on. Then you know that you
> >> don't need to probe them again until the device they are waiting for
> >> is available.
> >
> > That actually breaks things in a worse sense. There are cases where
> > the consumer driver is built in and the optional supplier driver is
> > loaded at boot. Without fw_devlink and the deferred probe timeout, we
> > end up probing the consumer with limited functionality. With the
> > current setup, sure we delay some probes a bit but at least everything
> > works with the right functionality. And you can reduce or remove the
> > delay if you want to optimize it.
>
> I have a system that doesn't use stdout-path and has the bootloader
> set console= either to ttynull when secure booting or to an UART
> when booting normally. How would I optimize the kernel to avoid
> my UART being loaded after DMA controller probe without touching
> the bootloader?
>

Thanks for the report Ahmad. I think someone else reported a similar
thing in another thread. I plan to take a look at it. It should be
possible to find the device and set the flag for those devices too.

-Saravana

Powered by blists - more mailing lists