Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall
@ 2020-02-14 23:32 John Stultz
  2020-02-14 23:34 ` John Stultz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: John Stultz @ 2020-02-14 23:32 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Rob Herring, Rafael J. Wysocki, Kevin Hilman,
	Ulf Hansson, Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
	Greg Kroah-Hartman, linux-pm

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.

Instead of reverting that patch, this patch tries to extend the
time that EPROBE_DEFER is retruned by 30 seconds, to (hopefully)
ensure that everything has had a chance to load.

30 seconds was chosen to match the similar timeout used by the
regulator code here in commit 55576cf18537 ("regulator: Defer
init completion for a while after late_initcall")

Specifically, on db845c, this change 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.

Cc: Rob Herring <robh@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Todd Kjos <tkjos@google.com>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Fixes: e01afc3250255 ("PM / Domains: Stop deferring probe at the end of initcall")
Fixes: 25b4e70dcce9 ("driver core: allow stopping deferred probe after init")
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
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.
---
 drivers/base/dd.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d811e60610d3..0f519ef3b257 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -311,6 +311,15 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
 }
 static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
 
+static void deferred_initcall_done_work_func(struct work_struct *work)
+{
+	initcalls_done = true;
+	driver_deferred_probe_trigger();
+	flush_work(&deferred_probe_work);
+}
+static DECLARE_DELAYED_WORK(deferred_initcall_done_work,
+			    deferred_initcall_done_work_func);
+
 /**
  * deferred_probe_initcall() - Enable probing of deferred devices
  *
@@ -327,7 +336,8 @@ static int deferred_probe_initcall(void)
 	driver_deferred_probe_trigger();
 	/* Sort as many dependencies as possible before exiting initcalls */
 	flush_work(&deferred_probe_work);
-	initcalls_done = true;
+	schedule_delayed_work(&deferred_initcall_done_work,
+			      msecs_to_jiffies(30000));
 
 	/*
 	 * Trigger deferred probe again, this time we won't defer anything
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall
  2020-02-14 23:32 [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall John Stultz
@ 2020-02-14 23:34 ` John Stultz
  2020-02-15  7:51 ` Bjorn Andersson
  2020-02-15 21:32 ` Rob Herring
  2 siblings, 0 replies; 4+ messages in thread
From: John Stultz @ 2020-02-14 23:34 UTC (permalink / raw)
  To: lkml
  Cc: Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
	Greg Kroah-Hartman, Linux PM list

On Fri, Feb 14, 2020 at 3:32 PM John Stultz <john.stultz@linaro.org> 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.
>
> Instead of reverting that patch, this patch tries to extend the
> time that EPROBE_DEFER is retruned by 30 seconds, to (hopefully)
> ensure that everything has had a chance to load.

Oh, and of course I forgot to fix the subject line and didn't see it
until git send-email was done.  That should be 30 seconds, not two
minutes. I'll fix that up for the next version.

Apologies!
-john

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall
  2020-02-14 23:32 [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall John Stultz
  2020-02-14 23:34 ` John Stultz
@ 2020-02-15  7:51 ` Bjorn Andersson
  2020-02-15 21:32 ` Rob Herring
  2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2020-02-15  7:51 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Pavel Machek, Len Brown, Todd Kjos, Greg Kroah-Hartman, linux-pm

On Fri 14 Feb 15:32 PST 2020, John Stultz 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.
> 
> Instead of reverting that patch, this patch tries to extend the
> time that EPROBE_DEFER is retruned by 30 seconds, to (hopefully)
> ensure that everything has had a chance to load.
> 
> 30 seconds was chosen to match the similar timeout used by the
> regulator code here in commit 55576cf18537 ("regulator: Defer
> init completion for a while after late_initcall")
> 
> Specifically, on db845c, this change 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.
> 

With the $subject adjustment

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Cc: Rob Herring <robh@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Fixes: e01afc3250255 ("PM / Domains: Stop deferring probe at the end of initcall")
> Fixes: 25b4e70dcce9 ("driver core: allow stopping deferred probe after init")
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> 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.
> ---
>  drivers/base/dd.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index d811e60610d3..0f519ef3b257 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -311,6 +311,15 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
>  }
>  static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
>  
> +static void deferred_initcall_done_work_func(struct work_struct *work)
> +{
> +	initcalls_done = true;
> +	driver_deferred_probe_trigger();
> +	flush_work(&deferred_probe_work);
> +}
> +static DECLARE_DELAYED_WORK(deferred_initcall_done_work,
> +			    deferred_initcall_done_work_func);
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
> @@ -327,7 +336,8 @@ static int deferred_probe_initcall(void)
>  	driver_deferred_probe_trigger();
>  	/* Sort as many dependencies as possible before exiting initcalls */
>  	flush_work(&deferred_probe_work);
> -	initcalls_done = true;
> +	schedule_delayed_work(&deferred_initcall_done_work,
> +			      msecs_to_jiffies(30000));
>  
>  	/*
>  	 * Trigger deferred probe again, this time we won't defer anything
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall
  2020-02-14 23:32 [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall John Stultz
  2020-02-14 23:34 ` John Stultz
  2020-02-15  7:51 ` Bjorn Andersson
@ 2020-02-15 21:32 ` Rob Herring
  2 siblings, 0 replies; 4+ messages in thread
From: Rob Herring @ 2020-02-15 21:32 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, Pavel Machek,
	Len Brown, Todd Kjos, Bjorn Andersson, Greg Kroah-Hartman,
	open list:THERMAL

On Fri, Feb 14, 2020 at 5:32 PM John Stultz <john.stultz@linaro.org> 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.
>
> Instead of reverting that patch, this patch tries to extend the
> time that EPROBE_DEFER is retruned by 30 seconds, to (hopefully)
> ensure that everything has had a chance to load.
>
> 30 seconds was chosen to match the similar timeout used by the
> regulator code here in commit 55576cf18537 ("regulator: Defer
> init completion for a while after late_initcall")

My intention was not to copy the timeout value, but use the same
timeout (which should be configurable on the command line IMO). There
doesn't seem to be an easy way to hook the regulator code into the
deferred probe code though. Maybe just make the timeout value a global
var.

Rob

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 23:32 [PATCH v2] driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall John Stultz
2020-02-14 23:34 ` John Stultz
2020-02-15  7:51 ` Bjorn Andersson
2020-02-15 21:32 ` Rob Herring

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git