Re: [PATCH v3 1/2] driver core: Rework logic in __driver_deferred_probe_check_state to allow EPROBE_DEFER to be returned for longer

[Rob Herring]

On Tue, Feb 18, 2020 at 4:07 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
>
> Due to commit e01afc3250255 ("PM / Domains: Stop deferring probe
> at the end of initcall"), along with commit 25b4e70dcce9
> ("driver core: allow stopping deferred probe after init") after
> late_initcall, drivers will stop getting EPROBE_DEFER, and
> instead see an error causing the driver to fail to load.
>
> That change causes trouble when trying to use many clk drivers
> as modules, as the clk modules may not load until much later
> after init has started. If a dependent driver loads and gets an
> error instead of EPROBE_DEFER, it won't try to reload later when
> the dependency is met, and will thus fail to load.
>
> This patch reworks some of the logic in
> __driver_deferred_probe_check_state() so that if the
> deferred_probe_timeout value is set, we will return EPROBE_DEFER
> until that timeout expires, which may be after initcalls_done
> is set to true.
>
> Specifically, on db845c, this change (when combined with booting
> using deferred_probe_timeout=30) allows us to set SDM_GPUCC_845,
> QCOM_CLK_RPMH and COMMON_CLK_QCOM as modules and get a working
> system, where as without it the display will fail to load.

I would change the default for deferred_probe_timeout to 30 and then
regulator code can rely on that. Curious, why 30 sec is fine now when
you originally had 2 min? I'd just pick what you think is best. I
doubt Mark had any extensive experiments to come up with 30sec.

> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: Pavel Machek <pavel@xxxxxx>
> Cc: Len Brown <len.brown@xxxxxxxxx>
> Cc: Todd Kjos <tkjos@xxxxxxxxxx>
> Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> Cc: Liam Girdwood <lgirdwood@xxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-pm@xxxxxxxxxxxxxxx
> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
> ---
> v2:
> * Add calls to driver_deferred_probe_trigger() after the two minute timeout,
>   as suggested by Bjorn
> * Minor whitespace cleanups
> * Switch to 30 second timeout to match what the regulator code is doing as
>   suggested by Rob.
> v3:
> * Rework to reuse existing deferred_probe_timeout value, suggested by Rob
> * Dropped Fixes: tags as Rob requested (Not my hill to die on :)
> ---
>  drivers/base/dd.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index b25bcab2a26b..9d916a7b56a6 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -237,13 +237,12 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
>
>  static int __driver_deferred_probe_check_state(struct device *dev)
>  {
> -       if (!initcalls_done)
> -               return -EPROBE_DEFER;
> -
> -       if (!deferred_probe_timeout) {
> +       if (initcalls_done && !deferred_probe_timeout) {
>                 dev_WARN(dev, "deferred probe timeout, ignoring dependency");
>                 return -ETIMEDOUT;
>         }
> +       if (!initcalls_done || deferred_probe_timeout > 0)
> +               return -EPROBE_DEFER;
>
>         return 0;
>  }
> --
> 2.17.1
>