Skip to contentSkip to conversation

driver core: Try to improve and cleanup driver_deferred_probe_check_state()

PATCH Superseded John Stultz 5 participants 21 messages Feb 20, 2020 – Feb 21, 2020

Subsystems (3)
Driver core Pin control Voltage and current regulator framework

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

John Stultz

author
#0 lore raw

v4 cover driver core: Try to improve and cleanup driver_deferred_probe_check_state()

This series tries to improve and cleanup the driver_deferred_probe_check_state() code in the driver core.

This is most important for being able to support modules dependencies which may be loaded by userland, far after late_initcall is done. For instance, this series allows us to use successfully various clk drivers as modules on the db845c board. And without it, those drivers have to be statically built in to work.

The first patch (or two) is really the most critical for me, but as I was working to understand the code (and with some prodding), it seemed a further cleanup was in order.

If folks have concerns about the tail end of the patch set, I'm fine to defer that. The main issue for me is the inability to load module dependencies after init starts.

New in v4:
* Split earlier version of the patches apart
* Defaulting deferred_probe_timeout to 30 seconds if modules are enabled.
* Deeper cleanup on driver_deferred_probe_check_state_continue()

Feedback would be greatly appreciated!

thanks
-john

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]

John Stultz (6):
driver core: Fix driver_deferred_probe_check_state() logic driver core: Set deferred_probe_timeout to a longer default if
CONFIG_MODULES is set
pinctrl: Remove use of driver_deferred_probe_check_state_continue() driver core: Remove driver_deferred_probe_check_state_continue() driver core: Rename deferred_probe_timeout and make it global
regulator: Use driver_deferred_probe_timeout for
regulator_init_complete_work

 drivers/base/dd.c             | 78 ++++++++++++-----------------------
 drivers/pinctrl/devicetree.c  |  9 ++--
 drivers/regulator/core.c      | 25 ++++++-----
 include/linux/device/driver.h |  2 +-
 4 files changed, 47 insertions(+), 67 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v4 1/6 driver core: Fix driver_deferred_probe_check_state() logic

driver_deferred_probe_check_state() has some uninituitive behavior.

* From boot to late_initcall, it returns -EPROBE_DEFER

* From late_initcall to the deferred_probe_timeout (if set) it returns -ENODEV

* If the deferred_probe_timeout it set, after it fires, it returns -ETIMEDOUT

This is a bit confusing, as its useful to have the function return -EPROBE_DEFER while the timeout is still running. This behavior has resulted in the somwhat duplicative driver_deferred_probe_check_state_continue() function being added.

Thus this patch tries to improve the logic, so that it behaves as such:

* If deferred_probe_timeout is set, it returns -EPROBE_DEFER until the timeout, afterwhich it returns -ETIMEDOUT.

* If deferred_probe_timeout is not set (-1), it returns -EPROBE_DEFER until late_initcall, after which it returns

This will make the deferred_probe_timeout value much more functional, and will allow us to consolidate the driver_deferred_probe_check_state() and driver_deferred_probe_check_state_continue() logic in a later patch.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I8349b7a403ce8cbce485ea0a0a5512fddffb635c

---
v4:
* Simplified logic suggested by Andy Shevchenko
* Clarified commit message to focus on logic change

---
 drivers/base/dd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/base/dd.c +6 -4
index b25bcab2a26b..bb383dca39c1 100644
@@ -237,7 +237,7 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
 static int __driver_deferred_probe_check_state(struct device *dev)
 {
-	if (!initcalls_done)
+	if (!initcalls_done || deferred_probe_timeout > 0)
 		return -EPROBE_DEFER;
 
 	if (!deferred_probe_timeout) {

@@ -252,9 +252,11 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check
  *
- * Returns -ENODEV if init is done and all built-in drivers have had a chance
- * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
- * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
+ * Returnes -EPROBE_DEFER if initcalls have not completed, or the deferred
+ * probe timeout is set, but not expried.
+ * Returns -ETIMEDOUT if the probe timeout was set and has expired.
+ * Returns -ENODEV if initcalls have completed and the deferred probe timeout
+ * was not set.
  *
  * Drivers or subsystems can opt-in to calling this function instead of directly
  * returning -EPROBE_DEFER.
Signature · 1 line
2.17.1
maintainer
#2 lore raw

Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic

Quoted 61 lines · John Stultz

driver_deferred_probe_check_state() has some uninituitive behavior.

* From boot to late_initcall, it returns -EPROBE_DEFER

* From late_initcall to the deferred_probe_timeout (if set) it returns -ENODEV

* If the deferred_probe_timeout it set, after it fires, it returns -ETIMEDOUT

This is a bit confusing, as its useful to have the function return -EPROBE_DEFER while the timeout is still running. This behavior has resulted in the somwhat duplicative driver_deferred_probe_check_state_continue() function being added.

Thus this patch tries to improve the logic, so that it behaves as such:

* If deferred_probe_timeout is set, it returns -EPROBE_DEFER until the timeout, afterwhich it returns -ETIMEDOUT.

* If deferred_probe_timeout is not set (-1), it returns -EPROBE_DEFER until late_initcall, after which it returns

This will make the deferred_probe_timeout value much more functional, and will allow us to consolidate the driver_deferred_probe_check_state() and driver_deferred_probe_check_state_continue() logic in a later patch.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I8349b7a403ce8cbce485ea0a0a5512fddffb635c

---
v4:
* Simplified logic suggested by Andy Shevchenko
* Clarified commit message to focus on logic change

---
 drivers/base/dd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/base/dd.c +0 -0
index b25bcab2a26b..bb383dca39c1 100644

drivers/base/dd.c __setup("deferred_probe_timeout=", deferred_probe_timeout_setup); · line 241

Quoted 6 lines
drivers/base/dd.c +1 -1
@@ -237,7 +237,7 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
 static int __driver_deferred_probe_check_state(struct device *dev)
 {
-       if (!initcalls_done)
+       if (!initcalls_done || deferred_probe_timeout > 0)
                return -EPROBE_DEFER;

Makes sense to me.

drivers/base/dd.c static int __driver_deferred_probe_check_state(struct device *dev) · line 255

Quoted 10 lines
drivers/base/dd.c +1 -3
        if (!deferred_probe_timeout) {

@@ -252,9 +252,11 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check
  *
- * Returns -ENODEV if init is done and all built-in drivers have had a chance
- * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
- * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
+ * Returnes -EPROBE_DEFER if initcalls have not completed, or the deferred

s/Returnes/Returns/

And I would write

* Return:
* -EPROBE_DEFER if initcalls have not completed, or the deferred
* probe timeout is set, but not expried.
* -ETIMEDOUT if the deferred probe timeout was set and has expired.
* -ENODEV if initcalls have completed and the deferred probe timeout was not set.

Quoted 8 lines
drivers/base/dd.c +4 -1
+ * probe timeout is set, but not expried.
+ * Returns -ETIMEDOUT if the probe timeout was set and has expired.
+ * Returns -ENODEV if initcalls have completed and the deferred probe timeout
+ * was not set.
  *
  * Drivers or subsystems can opt-in to calling this function instead of directly
  * returning -EPROBE_DEFER.
--
Reviewed-by on v4 1/6
#3 lore raw

Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic

In reply to John Stultz
Quoted 31 lines · John Stultz

driver_deferred_probe_check_state() has some uninituitive behavior.

* From boot to late_initcall, it returns -EPROBE_DEFER

* From late_initcall to the deferred_probe_timeout (if set) it returns -ENODEV

* If the deferred_probe_timeout it set, after it fires, it returns -ETIMEDOUT

This is a bit confusing, as its useful to have the function return -EPROBE_DEFER while the timeout is still running. This behavior has resulted in the somwhat duplicative driver_deferred_probe_check_state_continue() function being added.

Thus this patch tries to improve the logic, so that it behaves as such:

* If deferred_probe_timeout is set, it returns -EPROBE_DEFER until the timeout, afterwhich it returns -ETIMEDOUT.

* If deferred_probe_timeout is not set (-1), it returns -EPROBE_DEFER until late_initcall, after which it returns

This will make the deferred_probe_timeout value much more functional, and will allow us to consolidate the driver_deferred_probe_check_state() and driver_deferred_probe_check_state_continue() logic in a later patch.

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I8349b7a403ce8cbce485ea0a0a5512fddffb635c

Please drop the Change-Id.

Quoted 22 lines

---
v4:
* Simplified logic suggested by Andy Shevchenko
* Clarified commit message to focus on logic change

---
 drivers/base/dd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/base/dd.c +1 -1
index b25bcab2a26b..bb383dca39c1 100644
@@ -237,7 +237,7 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
 static int __driver_deferred_probe_check_state(struct device *dev)
 {
-	if (!initcalls_done)
+	if (!initcalls_done || deferred_probe_timeout > 0)
 		return -EPROBE_DEFER;
 
 	if (!deferred_probe_timeout) {

drivers/base/dd.c static int __driver_deferred_probe_check_state(struct device *dev) · line 255

Quoted 7 lines
drivers/base/dd.c +1 -3
@@ -252,9 +252,11 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check
  *
- * Returns -ENODEV if init is done and all built-in drivers have had a chance
- * to probe (i.e. initcalls are done), -ETIMEDOUT if deferred probe debug
- * timeout has expired, or -EPROBE_DEFER if none of those conditions are met.
+ * Returnes -EPROBE_DEFER if initcalls have not completed, or the deferred

As pointed out by Rafael, this should be Return:

With that addressed, you have my

Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 10 lines
drivers/base/dd.c +4 -0
+ * probe timeout is set, but not expried.
+ * Returns -ETIMEDOUT if the probe timeout was set and has expired.
+ * Returns -ENODEV if initcalls have completed and the deferred probe timeout
+ * was not set.
  *
  * Drivers or subsystems can opt-in to calling this function instead of directly
  * returning -EPROBE_DEFER.
Signature · 1 line
2.17.1
#4 lore raw

Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic

In reply to John Stultz

Hi!

Quoted 3 lines

* If deferred_probe_timeout is not set (-1), it returns -EPROBE_DEFER until late_initcall, after which it returns

You may want to complete the sentence here :-).

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#5 lore raw

Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic

Quoted 9 lines · Pavel Machek

Hi!

Quoted 3 lines

* If deferred_probe_timeout is not set (-1), it returns -EPROBE_DEFER until late_initcall, after which it returns

You may want to complete the sentence here :-).

Yes. I somehow cut the line accidentally. Its already fixed in my tree. :)

thanks for the review!
-john

author
#6 lore raw

PATCH v4 2/6 driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set

In reply to John Stultz

When using modules, its common for the modules not to be loaded until quite late by userland. With the current code, driver_deferred_probe_check_state() will stop returning EPROBE_DEFER after late_initcall, which can cause module dependency resolution to fail after that.

So allow a longer window of 30 seconds (picked somewhat arbitrarily, but influenced by the similar regulator core timeout value) in the case where modules are enabled.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I9c5a02a54915ff53f9f14d49c601f41d7105e05e

---
v4:
* Split out into its own patch as suggested by Mark
* Made change conditional on CONFIG_MODULES

---
 drivers/base/dd.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/base/dd.c +9 -0
index bb383dca39c1..fa138f24e2d3 100644
@@ -224,7 +224,16 @@ static int deferred_devs_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(deferred_devs);
 
+#ifdef CONFIG_MODULES
+/*
+ * In the case of modules, set the default probe timeout to
+ * 30 seconds to give userland some time to load needed modules
+ */
+static int deferred_probe_timeout = 30;
+#else
+/* In the case of !modules, no probe timeout needed */
 static int deferred_probe_timeout = -1;
+#endif
 static int __init deferred_probe_timeout_setup(char *str)
 {
 	int timeout;
Signature · 1 line
2.17.1
maintainer Reviewed-by on v4 2/6
#7 lore raw

Re: [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set

Quoted 43 lines · John Stultz

When using modules, its common for the modules not to be loaded until quite late by userland. With the current code, driver_deferred_probe_check_state() will stop returning EPROBE_DEFER after late_initcall, which can cause module dependency resolution to fail after that.

So allow a longer window of 30 seconds (picked somewhat arbitrarily, but influenced by the similar regulator core timeout value) in the case where modules are enabled.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I9c5a02a54915ff53f9f14d49c601f41d7105e05e

---
v4:
* Split out into its own patch as suggested by Mark
* Made change conditional on CONFIG_MODULES

---
 drivers/base/dd.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/base/dd.c +0 -0
index bb383dca39c1..fa138f24e2d3 100644
@@ -224,7 +224,16 @@ static int deferred_devs_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(deferred_devs);

drivers/base/dd.c static int deferred_devs_show(struct seq_file *s, void *data) · line 236

Quoted 10 lines
drivers/base/dd.c +9 -0
+#ifdef CONFIG_MODULES
+/*
+ * In the case of modules, set the default probe timeout to
+ * 30 seconds to give userland some time to load needed modules
+ */
+static int deferred_probe_timeout = 30;
+#else
+/* In the case of !modules, no probe timeout needed */
 static int deferred_probe_timeout = -1;
+#endif

Looks reasonable to me.

Reviewed-by Rafael J. Wysocki <[email protected]>
Quoted 4 lines
drivers/base/dd.c +0 -1
 static int __init deferred_probe_timeout_setup(char *str)
 {
        int timeout;
--
Reviewed-by on v4 2/6
#8 lore raw

Re: [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set

In reply to John Stultz
Quoted 10 lines · John Stultz

When using modules, its common for the modules not to be loaded until quite late by userland. With the current code, driver_deferred_probe_check_state() will stop returning EPROBE_DEFER after late_initcall, which can cause module dependency resolution to fail after that.

So allow a longer window of 30 seconds (picked somewhat arbitrarily, but influenced by the similar regulator core timeout value) in the case where modules are enabled.

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I9c5a02a54915ff53f9f14d49c601f41d7105e05e

Change-Id...

Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 32 lines

---
v4:
* Split out into its own patch as suggested by Mark
* Made change conditional on CONFIG_MODULES

---
 drivers/base/dd.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/base/dd.c +9 -0
index bb383dca39c1..fa138f24e2d3 100644
@@ -224,7 +224,16 @@ static int deferred_devs_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(deferred_devs);
 
+#ifdef CONFIG_MODULES
+/*
+ * In the case of modules, set the default probe timeout to
+ * 30 seconds to give userland some time to load needed modules
+ */
+static int deferred_probe_timeout = 30;
+#else
+/* In the case of !modules, no probe timeout needed */
 static int deferred_probe_timeout = -1;
+#endif
 static int __init deferred_probe_timeout_setup(char *str)
 {
 	int timeout;
Signature · 1 line
2.17.1
author
#9 lore raw

PATCH v4 3/6 pinctrl: Remove use of driver_deferred_probe_check_state_continue()

In reply to John Stultz

With the earlier sanity fixes to driver_deferred_probe_check_state() it should be usable for the pinctrl logic here.

So tweak the logic to use driver_deferred_probe_check_state() instead of driver_deferred_probe_check_state_continue()

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id If72682e0a7641b33edf56f188fc067c68bbc571e
---
 drivers/pinctrl/devicetree.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/pinctrl/devicetree.c +5 -4
index 9357f7c46cf3..1ed20ac2243f 100644
@@ -127,11 +127,12 @@ static int dt_to_map_one_config(struct pinctrl *p,
 		np_pctldev = of_get_next_parent(np_pctldev);
 		if (!np_pctldev || of_node_is_root(np_pctldev)) {
 			of_node_put(np_pctldev);
+			ret = driver_deferred_probe_check_state(p->dev);
 			/* keep deferring if modules are enabled unless we've timed out */
-			if (IS_ENABLED(CONFIG_MODULES) && !allow_default)
-				return driver_deferred_probe_check_state_continue(p->dev);
-
-			return driver_deferred_probe_check_state(p->dev);
+			if (IS_ENABLED(CONFIG_MODULES) && !allow_default &&
+			    (ret == -ENODEV))
+				ret = -EPROBE_DEFER;
+			return ret;
 		}
 		/* If we're creating a hog we can use the passed pctldev */
 		if (hog_pctldev && (np_pctldev == p->dev->of_node)) {
Signature · 1 line
2.17.1
Acked-by on v4 3/6
#10 lore raw

Re: [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue()

Quoted 7 lines · John Stultz

With the earlier sanity fixes to driver_deferred_probe_check_state() it should be usable for the pinctrl logic here.

So tweak the logic to use driver_deferred_probe_check_state() instead of driver_deferred_probe_check_state_continue()

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id If72682e0a7641b33edf56f188fc067c68bbc571e

I sure trust that you know what you're doing here.

Acked-by Linus Walleij <[email protected]>

I assume you will merge this through device core?

Yours,
Linus Walleij

author
#11 lore raw

Re: [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue()

Quoted 26 lines · Linus Walleij
Quoted 7 lines · John Stultz

With the earlier sanity fixes to driver_deferred_probe_check_state() it should be usable for the pinctrl logic here.

So tweak the logic to use driver_deferred_probe_check_state() instead of driver_deferred_probe_check_state_continue()

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id If72682e0a7641b33edf56f188fc067c68bbc571e
Quoted 1 line

I sure trust that you know what you're doing here.

Classic mistake. :)

Quoted 3 lines
Acked-by Linus Walleij <[email protected]>

I assume you will merge this through device core?

I guess? I'm going to resend it again as I think its a reasonable cleanup, but the urgent need for it from me may be solved by of_devlink Saravana's patch instead.

thanks
-john

author
#12 lore raw

PATCH v4 4/6 driver core: Remove driver_deferred_probe_check_state_continue()

In reply to John Stultz

Now that driver_deferred_probe_check_state() works better, and we've converted the only user of driver_deferred_probe_check_state_continue() we can simply remove it and simplify some of the logic.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id Id5cd5e9264cfb0fbd70a702715174cc4b10006f4
---
 drivers/base/dd.c             | 49 +++++------------------------------
 include/linux/device/driver.h |  1 -
 2 files changed, 6 insertions(+), 44 deletions(-)
drivers/base/dd.c +6 -43
index fa138f24e2d3..408e4da081da 100644
@@ -244,19 +244,6 @@ static int __init deferred_probe_timeout_setup(char *str)
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
-static int __driver_deferred_probe_check_state(struct device *dev)
-{
-	if (!initcalls_done || deferred_probe_timeout > 0)
-		return -EPROBE_DEFER;
-
-	if (!deferred_probe_timeout) {
-		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
 /**
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check

@@ -272,43 +259,19 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-	int ret;
+	if (!initcalls_done || deferred_probe_timeout > 0)
+		return -EPROBE_DEFER;
 
-	ret = __driver_deferred_probe_check_state(dev);
-	if (ret < 0)
-		return ret;
+	if (!deferred_probe_timeout) {
+		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
+		return -ETIMEDOUT;
+	}
 
 	dev_warn(dev, "ignoring dependency for device, assuming no driver");
 
 	return -ENODEV;
 }
 
-/**
- * driver_deferred_probe_check_state_continue() - check deferred probe state
- * @dev: device to check
- *
- * Returns -ETIMEDOUT if deferred probe debug timeout has expired, or
- * -EPROBE_DEFER otherwise.
- *
- * Drivers or subsystems can opt-in to calling this function instead of
- * directly returning -EPROBE_DEFER.
- *
- * This is similar to driver_deferred_probe_check_state(), but it allows the
- * subsystem to keep deferring probe after built-in drivers have had a chance
- * to probe. One scenario where that is useful is if built-in drivers rely on
- * resources that are provided by modular drivers.
- */
-int driver_deferred_probe_check_state_continue(struct device *dev)
-{
-	int ret;
-
-	ret = __driver_deferred_probe_check_state(dev);
-	if (ret < 0)
-		return ret;
-
-	return -EPROBE_DEFER;
-}
-
 static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
 	struct device_private *private, *p;
include/linux/device/driver.h +0 -1
index 1188260f9a02..5242afabfaba 100644
@@ -238,7 +238,6 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
-int driver_deferred_probe_check_state_continue(struct device *dev);
 void driver_init(void);
 
 /**
Signature · 1 line
2.17.1
maintainer Reviewed-by on v4 4/6
#13 lore raw

Re: [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()

Quoted 6 lines · John Stultz

Now that driver_deferred_probe_check_state() works better, and we've converted the only user of driver_deferred_probe_check_state_continue() we can simply remove it and simplify some of the logic.

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id Id5cd5e9264cfb0fbd70a702715174cc4b10006f4

Nice!

Reviewed-by Rafael J. Wysocki <[email protected]>
Quoted 92 lines
---
 drivers/base/dd.c             | 49 +++++------------------------------
 include/linux/device/driver.h |  1 -
 2 files changed, 6 insertions(+), 44 deletions(-)
drivers/base/dd.c +6 -43
index fa138f24e2d3..408e4da081da 100644
@@ -244,19 +244,6 @@ static int __init deferred_probe_timeout_setup(char *str)
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
-static int __driver_deferred_probe_check_state(struct device *dev)
-{
-       if (!initcalls_done || deferred_probe_timeout > 0)
-               return -EPROBE_DEFER;
-
-       if (!deferred_probe_timeout) {
-               dev_WARN(dev, "deferred probe timeout, ignoring dependency");
-               return -ETIMEDOUT;
-       }
-
-       return 0;
-}
-
 /**
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check

@@ -272,43 +259,19 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-       int ret;
+       if (!initcalls_done || deferred_probe_timeout > 0)
+               return -EPROBE_DEFER;
 
-       ret = __driver_deferred_probe_check_state(dev);
-       if (ret < 0)
-               return ret;
+       if (!deferred_probe_timeout) {
+               dev_WARN(dev, "deferred probe timeout, ignoring dependency");
+               return -ETIMEDOUT;
+       }
 
        dev_warn(dev, "ignoring dependency for device, assuming no driver");
 
        return -ENODEV;
 }
 
-/**
- * driver_deferred_probe_check_state_continue() - check deferred probe state
- * @dev: device to check
- *
- * Returns -ETIMEDOUT if deferred probe debug timeout has expired, or
- * -EPROBE_DEFER otherwise.
- *
- * Drivers or subsystems can opt-in to calling this function instead of
- * directly returning -EPROBE_DEFER.
- *
- * This is similar to driver_deferred_probe_check_state(), but it allows the
- * subsystem to keep deferring probe after built-in drivers have had a chance
- * to probe. One scenario where that is useful is if built-in drivers rely on
- * resources that are provided by modular drivers.
- */
-int driver_deferred_probe_check_state_continue(struct device *dev)
-{
-       int ret;
-
-       ret = __driver_deferred_probe_check_state(dev);
-       if (ret < 0)
-               return ret;
-
-       return -EPROBE_DEFER;
-}
-
 static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
        struct device_private *private, *p;
include/linux/device/driver.h +0 -2
index 1188260f9a02..5242afabfaba 100644
@@ -238,7 +238,6 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
-int driver_deferred_probe_check_state_continue(struct device *dev);
 void driver_init(void);
 
 /**
--
Reviewed-by on v4 4/6
#14 lore raw

Re: [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()

In reply to John Stultz
Quoted 5 lines · John Stultz

Now that driver_deferred_probe_check_state() works better, and we've converted the only user of driver_deferred_probe_check_state_continue() we can simply remove it and simplify some of the logic.

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id Id5cd5e9264cfb0fbd70a702715174cc4b10006f4

Change-Id...

Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 94 lines
---
 drivers/base/dd.c             | 49 +++++------------------------------
 include/linux/device/driver.h |  1 -
 2 files changed, 6 insertions(+), 44 deletions(-)
drivers/base/dd.c +6 -43
index fa138f24e2d3..408e4da081da 100644
@@ -244,19 +244,6 @@ static int __init deferred_probe_timeout_setup(char *str)
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
 
-static int __driver_deferred_probe_check_state(struct device *dev)
-{
-	if (!initcalls_done || deferred_probe_timeout > 0)
-		return -EPROBE_DEFER;
-
-	if (!deferred_probe_timeout) {
-		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
-}
-
 /**
  * driver_deferred_probe_check_state() - Check deferred probe state
  * @dev: device to check

@@ -272,43 +259,19 @@ static int __driver_deferred_probe_check_state(struct device *dev)
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-	int ret;
+	if (!initcalls_done || deferred_probe_timeout > 0)
+		return -EPROBE_DEFER;
 
-	ret = __driver_deferred_probe_check_state(dev);
-	if (ret < 0)
-		return ret;
+	if (!deferred_probe_timeout) {
+		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
+		return -ETIMEDOUT;
+	}
 
 	dev_warn(dev, "ignoring dependency for device, assuming no driver");
 
 	return -ENODEV;
 }
 
-/**
- * driver_deferred_probe_check_state_continue() - check deferred probe state
- * @dev: device to check
- *
- * Returns -ETIMEDOUT if deferred probe debug timeout has expired, or
- * -EPROBE_DEFER otherwise.
- *
- * Drivers or subsystems can opt-in to calling this function instead of
- * directly returning -EPROBE_DEFER.
- *
- * This is similar to driver_deferred_probe_check_state(), but it allows the
- * subsystem to keep deferring probe after built-in drivers have had a chance
- * to probe. One scenario where that is useful is if built-in drivers rely on
- * resources that are provided by modular drivers.
- */
-int driver_deferred_probe_check_state_continue(struct device *dev)
-{
-	int ret;
-
-	ret = __driver_deferred_probe_check_state(dev);
-	if (ret < 0)
-		return ret;
-
-	return -EPROBE_DEFER;
-}
-
 static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
 	struct device_private *private, *p;
include/linux/device/driver.h +0 -1
index 1188260f9a02..5242afabfaba 100644
@@ -238,7 +238,6 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
-int driver_deferred_probe_check_state_continue(struct device *dev);
 void driver_init(void);
 
 /**
Signature · 1 line
2.17.1
author
#15 lore raw

PATCH v4 5/6 driver core: Rename deferred_probe_timeout and make it global

In reply to John Stultz

Since other subsystems (like regulator) have similar arbitrary timeouts for how long they try to resolve driver dependencies, rename deferred_probe_timeout to driver_deferred_probe_timeout and set it as global, so it can be shared.

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I92ee3b392004ecc9217c5337b54eda48c2d7f3ee

---
v4:
* Split out into its own patch as suggested by Mark
* Renamed deferred_probe_timeout as suggested by Greg

---
 drivers/base/dd.c             | 18 ++++++++++--------
 include/linux/device/driver.h |  1 +
 2 files changed, 11 insertions(+), 8 deletions(-)
drivers/base/dd.c +10 -8
index 408e4da081da..39f1ce6d4f1c 100644
@@ -229,17 +229,19 @@ DEFINE_SHOW_ATTRIBUTE(deferred_devs);
  * In the case of modules, set the default probe timeout to
  * 30 seconds to give userland some time to load needed modules
  */
-static int deferred_probe_timeout = 30;
+int driver_deferred_probe_timeout = 30;
 #else
 /* In the case of !modules, no probe timeout needed */
-static int deferred_probe_timeout = -1;
+int driver_deferred_probe_timeout = -1;
 #endif
+EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
+
 static int __init deferred_probe_timeout_setup(char *str)
 {
 	int timeout;
 
 	if (!kstrtoint(str, 10, &timeout))
-		deferred_probe_timeout = timeout;
+		driver_deferred_probe_timeout = timeout;
 	return 1;
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);

@@ -259,10 +261,10 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-	if (!initcalls_done || deferred_probe_timeout > 0)
+	if (!initcalls_done || driver_deferred_probe_timeout > 0)
 		return -EPROBE_DEFER;
 
-	if (!deferred_probe_timeout) {
+	if (!driver_deferred_probe_timeout) {
 		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
 		return -ETIMEDOUT;
 	}

@@ -276,7 +278,7 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
 	struct device_private *private, *p;
 
-	deferred_probe_timeout = 0;
+	driver_deferred_probe_timeout = 0;
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
 

@@ -310,9 +312,9 @@ static int deferred_probe_initcall(void)
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
 
-	if (deferred_probe_timeout > 0) {
+	if (driver_deferred_probe_timeout > 0) {
 		schedule_delayed_work(&deferred_probe_timeout_work,
-			deferred_probe_timeout * HZ);
+			driver_deferred_probe_timeout * HZ);
 	}
 	return 0;
 }
include/linux/device/driver.h +1 -0
index 5242afabfaba..ee7ba5b5417e 100644
@@ -236,6 +236,7 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 }
 #endif
 
+extern int driver_deferred_probe_timeout;
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
 void driver_init(void);
Signature · 1 line
2.17.1
maintainer Reviewed-by on v4 5/6
#16 lore raw

Re: [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global

Quoted 5 lines · John Stultz

Since other subsystems (like regulator) have similar arbitrary timeouts for how long they try to resolve driver dependencies, rename deferred_probe_timeout to driver_deferred_probe_timeout and set it as global, so it can be shared.

Fair enough.

Quoted 8 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]>
Quoted 8 lines
Cc Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I92ee3b392004ecc9217c5337b54eda48c2d7f3ee
Reviewed-by Rafael J. Wysocki <[email protected]>
Quoted 85 lines

---
v4:
* Split out into its own patch as suggested by Mark
* Renamed deferred_probe_timeout as suggested by Greg

---
 drivers/base/dd.c             | 18 ++++++++++--------
 include/linux/device/driver.h |  1 +
 2 files changed, 11 insertions(+), 8 deletions(-)
drivers/base/dd.c +10 -8
index 408e4da081da..39f1ce6d4f1c 100644
@@ -229,17 +229,19 @@ DEFINE_SHOW_ATTRIBUTE(deferred_devs);
  * In the case of modules, set the default probe timeout to
  * 30 seconds to give userland some time to load needed modules
  */
-static int deferred_probe_timeout = 30;
+int driver_deferred_probe_timeout = 30;
 #else
 /* In the case of !modules, no probe timeout needed */
-static int deferred_probe_timeout = -1;
+int driver_deferred_probe_timeout = -1;
 #endif
+EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
+
 static int __init deferred_probe_timeout_setup(char *str)
 {
        int timeout;
 
        if (!kstrtoint(str, 10, &timeout))
-               deferred_probe_timeout = timeout;
+               driver_deferred_probe_timeout = timeout;
        return 1;
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);

@@ -259,10 +261,10 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-       if (!initcalls_done || deferred_probe_timeout > 0)
+       if (!initcalls_done || driver_deferred_probe_timeout > 0)
                return -EPROBE_DEFER;
 
-       if (!deferred_probe_timeout) {
+       if (!driver_deferred_probe_timeout) {
                dev_WARN(dev, "deferred probe timeout, ignoring dependency");
                return -ETIMEDOUT;
        }

@@ -276,7 +278,7 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
        struct device_private *private, *p;
 
-       deferred_probe_timeout = 0;
+       driver_deferred_probe_timeout = 0;
        driver_deferred_probe_trigger();
        flush_work(&deferred_probe_work);
 

@@ -310,9 +312,9 @@ static int deferred_probe_initcall(void)
        driver_deferred_probe_trigger();
        flush_work(&deferred_probe_work);
 
-       if (deferred_probe_timeout > 0) {
+       if (driver_deferred_probe_timeout > 0) {
                schedule_delayed_work(&deferred_probe_timeout_work,
-                       deferred_probe_timeout * HZ);
+                       driver_deferred_probe_timeout * HZ);
        }
        return 0;
 }
include/linux/device/driver.h +1 -1
index 5242afabfaba..ee7ba5b5417e 100644
@@ -236,6 +236,7 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 }
 #endif
 
+extern int driver_deferred_probe_timeout;
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
 void driver_init(void);
--

2.17.1

Reviewed-by on v4 5/6
#17 lore raw

Re: [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global

In reply to John Stultz
Quoted 5 lines · John Stultz

Since other subsystems (like regulator) have similar arbitrary timeouts for how long they try to resolve driver dependencies, rename deferred_probe_timeout to driver_deferred_probe_timeout and set it as global, so it can be shared.

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I92ee3b392004ecc9217c5337b54eda48c2d7f3ee

Change-Id...

Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 85 lines

---
v4:
* Split out into its own patch as suggested by Mark
* Renamed deferred_probe_timeout as suggested by Greg

---
 drivers/base/dd.c             | 18 ++++++++++--------
 include/linux/device/driver.h |  1 +
 2 files changed, 11 insertions(+), 8 deletions(-)
drivers/base/dd.c +10 -8
index 408e4da081da..39f1ce6d4f1c 100644
@@ -229,17 +229,19 @@ DEFINE_SHOW_ATTRIBUTE(deferred_devs);
  * In the case of modules, set the default probe timeout to
  * 30 seconds to give userland some time to load needed modules
  */
-static int deferred_probe_timeout = 30;
+int driver_deferred_probe_timeout = 30;
 #else
 /* In the case of !modules, no probe timeout needed */
-static int deferred_probe_timeout = -1;
+int driver_deferred_probe_timeout = -1;
 #endif
+EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
+
 static int __init deferred_probe_timeout_setup(char *str)
 {
 	int timeout;
 
 	if (!kstrtoint(str, 10, &timeout))
-		deferred_probe_timeout = timeout;
+		driver_deferred_probe_timeout = timeout;
 	return 1;
 }
 __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);

@@ -259,10 +261,10 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
  */
 int driver_deferred_probe_check_state(struct device *dev)
 {
-	if (!initcalls_done || deferred_probe_timeout > 0)
+	if (!initcalls_done || driver_deferred_probe_timeout > 0)
 		return -EPROBE_DEFER;
 
-	if (!deferred_probe_timeout) {
+	if (!driver_deferred_probe_timeout) {
 		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
 		return -ETIMEDOUT;
 	}

@@ -276,7 +278,7 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
 {
 	struct device_private *private, *p;
 
-	deferred_probe_timeout = 0;
+	driver_deferred_probe_timeout = 0;
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
 

@@ -310,9 +312,9 @@ static int deferred_probe_initcall(void)
 	driver_deferred_probe_trigger();
 	flush_work(&deferred_probe_work);
 
-	if (deferred_probe_timeout > 0) {
+	if (driver_deferred_probe_timeout > 0) {
 		schedule_delayed_work(&deferred_probe_timeout_work,
-			deferred_probe_timeout * HZ);
+			driver_deferred_probe_timeout * HZ);
 	}
 	return 0;
 }
include/linux/device/driver.h +1 -0
index 5242afabfaba..ee7ba5b5417e 100644
@@ -236,6 +236,7 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
 }
 #endif
 
+extern int driver_deferred_probe_timeout;
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
 void driver_init(void);
Signature · 1 line
2.17.1
author
#18 lore raw

PATCH v4 6/6 regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work

In reply to John Stultz

The regulator_init_complete_work logic defers the cleanup for an arbitrary 30 seconds of time to allow modules loaded by userland to start.

This arbitrary timeout is similar to the driver_deferred_probe_timeout value, and its been suggested we align these so users have a method to extend the timeouts as needed.

So this patch changes the logic to use the driver_deferred_probe_timeout value if it is set, otherwise we directly call the regulator_init_complete_work_function().

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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I9fa2411abbb91ed4dd0edc41e8cc8583577c005b

---
v4:
* Split out into its own patch, as suggested by Mark

---
 drivers/regulator/core.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/regulator/core.c +14 -11
index d015d99cb59d..394e7b11576a 100644
@@ -5767,18 +5767,21 @@ static int __init regulator_init_complete(void)
 		has_full_constraints = true;
 
 	/*
-	 * We punt completion for an arbitrary amount of time since
-	 * systems like distros will load many drivers from userspace
-	 * so consumers might not always be ready yet, this is
-	 * particularly an issue with laptops where this might bounce
-	 * the display off then on.  Ideally we'd get a notification
-	 * from userspace when this happens but we don't so just wait
-	 * a bit and hope we waited long enough.  It'd be better if
-	 * we'd only do this on systems that need it, and a kernel
-	 * command line option might be useful.
+	 * If driver_deferred_probe_timeout is set, we punt
+	 * completion for that many seconds since systems like
+	 * distros will load many drivers from userspace so consumers
+	 * might not always be ready yet, this is particularly an
+	 * issue with laptops where this might bounce the display off
+	 * then on.  Ideally we'd get a notification from userspace
+	 * when this happens but we don't so just wait a bit and hope
+	 * we waited long enough.  It'd be better if we'd only do
+	 * this on systems that need it.
 	 */
-	schedule_delayed_work(&regulator_init_complete_work,
-			      msecs_to_jiffies(30000));
+	if (driver_deferred_probe_timeout >= 0)
+		schedule_delayed_work(&regulator_init_complete_work,
+				      driver_deferred_probe_timeout * HZ);
+	else
+		regulator_init_complete_work_function(NULL);
 
 	return 0;
 }
Signature · 1 line
2.17.1
#19 lore raw

Re: [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work

Quoted 13 lines · John Stultz

The regulator_init_complete_work logic defers the cleanup for an arbitrary 30 seconds of time to allow modules loaded by userland to start.

This arbitrary timeout is similar to the driver_deferred_probe_timeout value, and its been suggested we align these so users have a method to extend the timeouts as needed.

So this patch changes the logic to use the driver_deferred_probe_timeout value if it is set, otherwise we directly call the regulator_init_complete_work_function().

Quoted 16 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 Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Signed-off-by John Stultz <[email protected]>
Change-Id I9fa2411abbb91ed4dd0edc41e8cc8583577c005b

Change-Id...

Quoted 31 lines

---
v4:
* Split out into its own patch, as suggested by Mark

---
 drivers/regulator/core.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/regulator/core.c +7 -9
index d015d99cb59d..394e7b11576a 100644
@@ -5767,18 +5767,21 @@ static int __init regulator_init_complete(void)
 		has_full_constraints = true;
 
 	/*
-	 * We punt completion for an arbitrary amount of time since
-	 * systems like distros will load many drivers from userspace
-	 * so consumers might not always be ready yet, this is
-	 * particularly an issue with laptops where this might bounce
-	 * the display off then on.  Ideally we'd get a notification
-	 * from userspace when this happens but we don't so just wait
-	 * a bit and hope we waited long enough.  It'd be better if
-	 * we'd only do this on systems that need it, and a kernel
-	 * command line option might be useful.
+	 * If driver_deferred_probe_timeout is set, we punt
+	 * completion for that many seconds since systems like
+	 * distros will load many drivers from userspace so consumers
+	 * might not always be ready yet, this is particularly an
+	 * issue with laptops where this might bounce the display off
+	 * then on.  Ideally we'd get a notification from userspace
+	 * when this happens but we don't so just wait a bit and hope

drivers/regulator/core.c static int __init regulator_init_complete(void) · line 5784

Quoted 10 lines
drivers/regulator/core.c +7 -2
+	 * we waited long enough.  It'd be better if we'd only do
+	 * this on systems that need it.
 	 */
-	schedule_delayed_work(&regulator_init_complete_work,
-			      msecs_to_jiffies(30000));
+	if (driver_deferred_probe_timeout >= 0)
+		schedule_delayed_work(&regulator_init_complete_work,
+				      driver_deferred_probe_timeout * HZ);
+	else
+		regulator_init_complete_work_function(NULL);

Why not schedule_delayed_work(..., 0) in this case, to get it off the initcall context and to avoid the difference in execution paths?

Regards,
Bjorn

Quoted 6 lines
drivers/regulator/core.c +0 -0
 	return 0;
 }
Signature · 1 line
2.17.1
maintainer
#20 lore raw
In reply to John Stultz
Quoted 3 lines · John Stultz

This series tries to improve and cleanup the driver_deferred_probe_check_state() code in the driver core.

"Do. Or do not. There is no try."
- Master Yoda