Skip to contentSkip to conversation

driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall

PATCH In review John Stultz 3 participants 4 messages Feb 14, 2020 – Feb 15, 2020 Driver core

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 driver core: Extend returning EPROBE_DEFER for two minutes after late_initcall

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 <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Ulf Hansson <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Len Brown <[email protected]>
Cc Todd Kjos <[email protected]>
Cc Bjorn Andersson <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Fixes e01afc3250255 ("PM / Domains: Stop deferring probe at the end of initcall")
Fixes 25b4e70dcce9 ("driver core: allow stopping deferred probe after init")

---
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(-)
drivers/base/dd.c +11 -1
index d811e60610d3..0f519ef3b257 100644
@@ -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
Signature · 1 line
2.17.1
#1 lore raw
Quoted 13 lines · John Stultz

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.

Quoted 3 lines

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

Reviewed-by on v2 1/1
#2 lore raw
In reply to John Stultz
Quoted 16 lines · John Stultz

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.

Quoted 9 lines

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

Regards,
Bjorn

Quoted 57 lines
Cc Rob Herring <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Ulf Hansson <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Len Brown <[email protected]>
Cc Todd Kjos <[email protected]>
Cc Bjorn Andersson <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Fixes e01afc3250255 ("PM / Domains: Stop deferring probe at the end of initcall")
Fixes 25b4e70dcce9 ("driver core: allow stopping deferred probe after init")

---
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(-)
drivers/base/dd.c +11 -1
index d811e60610d3..0f519ef3b257 100644
@@ -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
Signature · 1 line
2.17.1
#3 lore raw
In reply to John Stultz
Quoted 13 lines · John Stultz

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.

Quoted 7 lines

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