* [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-20 10:28 ` Rafael J. Wysocki
` (2 more replies)
2020-02-20 5:04 ` [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set John Stultz
` (5 subsequent siblings)
6 siblings, 3 replies; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
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(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index b25bcab2a26b..bb383dca39c1 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -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.
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic
2020-02-20 5:04 ` [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic John Stultz
@ 2020-02-20 10:28 ` Rafael J. Wysocki
2020-02-20 15:55 ` Bjorn Andersson
2020-02-20 23:40 ` Pavel Machek
2 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-02-20 10:28 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, Linux PM
On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
>
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> 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(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index b25bcab2a26b..bb383dca39c1 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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.
>
> 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.
> + * 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.
> --
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic
2020-02-20 5:04 ` [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic John Stultz
2020-02-20 10:28 ` Rafael J. Wysocki
@ 2020-02-20 15:55 ` Bjorn Andersson
2020-02-20 23:40 ` Pavel Machek
2 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2020-02-20 15:55 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: I8349b7a403ce8cbce485ea0a0a5512fddffb635c
Please drop the Change-Id.
> ---
> 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(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index b25bcab2a26b..bb383dca39c1 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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
As pointed out by Rafael, this should be Return:
With that addressed, you have my
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> + * 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.
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic
2020-02-20 5:04 ` [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic John Stultz
2020-02-20 10:28 ` Rafael J. Wysocki
2020-02-20 15:55 ` Bjorn Andersson
@ 2020-02-20 23:40 ` Pavel Machek
2020-02-20 23:42 ` John Stultz
2 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2020-02-20 23:40 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Len Brown, Todd Kjos, Bjorn Andersson, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
[-- Attachment #1: Type: text/plain, Size: 362 bytes --]
Hi!
> * 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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic
2020-02-20 23:40 ` Pavel Machek
@ 2020-02-20 23:42 ` John Stultz
0 siblings, 0 replies; 21+ messages in thread
From: John Stultz @ 2020-02-20 23:42 UTC (permalink / raw)
To: Pavel Machek
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Len Brown, Todd Kjos, Bjorn Andersson, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, Linux PM list
On Thu, Feb 20, 2020 at 3:40 PM Pavel Machek <pavel@denx.de> wrote:
>
> Hi!
>
> > * 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
2020-02-20 5:04 ` [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-20 10:36 ` Rafael J. Wysocki
2020-02-20 15:55 ` Bjorn Andersson
2020-02-20 5:04 ` [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue() John Stultz
` (4 subsequent siblings)
6 siblings, 2 replies; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
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(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index bb383dca39c1..fa138f24e2d3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -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;
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set
2020-02-20 5:04 ` [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set John Stultz
@ 2020-02-20 10:36 ` Rafael J. Wysocki
2020-02-20 15:55 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-02-20 10:36 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, Linux PM
On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
>
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> 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(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index bb383dca39c1..fa138f24e2d3 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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
Looks reasonable to me.
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> static int __init deferred_probe_timeout_setup(char *str)
> {
> int timeout;
> --
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set
2020-02-20 5:04 ` [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set John Stultz
2020-02-20 10:36 ` Rafael J. Wysocki
@ 2020-02-20 15:55 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2020-02-20 15:55 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: I9c5a02a54915ff53f9f14d49c601f41d7105e05e
Change-Id...
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> 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(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index bb383dca39c1..fa138f24e2d3 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue()
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
2020-02-20 5:04 ` [PATCH v4 1/6] driver core: Fix driver_deferred_probe_check_state() logic John Stultz
2020-02-20 5:04 ` [PATCH v4 2/6] driver core: Set deferred_probe_timeout to a longer default if CONFIG_MODULES is set John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-21 15:21 ` Linus Walleij
2020-02-20 5:04 ` [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue() John Stultz
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
Change-Id: If72682e0a7641b33edf56f188fc067c68bbc571e
---
drivers/pinctrl/devicetree.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index 9357f7c46cf3..1ed20ac2243f 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -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)) {
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue()
2020-02-20 5:04 ` [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue() John Stultz
@ 2020-02-21 15:21 ` Linus Walleij
2020-02-21 17:19 ` John Stultz
0 siblings, 1 reply; 21+ messages in thread
From: Linus Walleij @ 2020-02-21 15:21 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Greg Kroah-Hartman,
Linux PM list
On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: If72682e0a7641b33edf56f188fc067c68bbc571e
I sure trust that you know what you're doing here.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
I assume you will merge this through device core?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue()
2020-02-21 15:21 ` Linus Walleij
@ 2020-02-21 17:19 ` John Stultz
0 siblings, 0 replies; 21+ messages in thread
From: John Stultz @ 2020-02-21 17:19 UTC (permalink / raw)
To: Linus Walleij
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Greg Kroah-Hartman,
Linux PM list
On Fri, Feb 21, 2020 at 7:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
>
> > 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > Change-Id: If72682e0a7641b33edf56f188fc067c68bbc571e
>
> I sure trust that you know what you're doing here.
Classic mistake. :)
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
` (2 preceding siblings ...)
2020-02-20 5:04 ` [PATCH v4 3/6] pinctrl: Remove use of driver_deferred_probe_check_state_continue() John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-20 10:38 ` Rafael J. Wysocki
2020-02-20 15:59 ` Bjorn Andersson
2020-02-20 5:04 ` [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global John Stultz
` (2 subsequent siblings)
6 siblings, 2 replies; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
Change-Id: Id5cd5e9264cfb0fbd70a702715174cc4b10006f4
---
drivers/base/dd.c | 49 +++++------------------------------
include/linux/device/driver.h | 1 -
2 files changed, 6 insertions(+), 44 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index fa138f24e2d3..408e4da081da 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -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;
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 1188260f9a02..5242afabfaba 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -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);
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()
2020-02-20 5:04 ` [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue() John Stultz
@ 2020-02-20 10:38 ` Rafael J. Wysocki
2020-02-20 15:59 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-02-20 10:38 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, Linux PM
On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
>
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: Id5cd5e9264cfb0fbd70a702715174cc4b10006f4
Nice!
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/base/dd.c | 49 +++++------------------------------
> include/linux/device/driver.h | 1 -
> 2 files changed, 6 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index fa138f24e2d3..408e4da081da 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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;
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index 1188260f9a02..5242afabfaba 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -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);
>
> /**
> --
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()
2020-02-20 5:04 ` [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue() John Stultz
2020-02-20 10:38 ` Rafael J. Wysocki
@ 2020-02-20 15:59 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2020-02-20 15:59 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: Id5cd5e9264cfb0fbd70a702715174cc4b10006f4
Change-Id...
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> drivers/base/dd.c | 49 +++++------------------------------
> include/linux/device/driver.h | 1 -
> 2 files changed, 6 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index fa138f24e2d3..408e4da081da 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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;
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index 1188260f9a02..5242afabfaba 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -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);
>
> /**
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
` (3 preceding siblings ...)
2020-02-20 5:04 ` [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue() John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-20 10:42 ` Rafael J. Wysocki
2020-02-20 16:00 ` Bjorn Andersson
2020-02-20 5:04 ` [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work John Stultz
2020-02-20 10:15 ` [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() Rafael J. Wysocki
6 siblings, 2 replies; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
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(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 408e4da081da..39f1ce6d4f1c 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -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;
}
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 5242afabfaba..ee7ba5b5417e 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global
2020-02-20 5:04 ` [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global John Stultz
@ 2020-02-20 10:42 ` Rafael J. Wysocki
2020-02-20 16:00 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-02-20 10:42 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, Linux PM
On Thu, Feb 20, 2020 at 6:05 AM John Stultz <john.stultz@linaro.org> wrote:
>
> 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.
> 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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: I92ee3b392004ecc9217c5337b54eda48c2d7f3ee
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 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(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 408e4da081da..39f1ce6d4f1c 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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;
> }
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index 5242afabfaba..ee7ba5b5417e 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global
2020-02-20 5:04 ` [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global John Stultz
2020-02-20 10:42 ` Rafael J. Wysocki
@ 2020-02-20 16:00 ` Bjorn Andersson
1 sibling, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2020-02-20 16:00 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: I92ee3b392004ecc9217c5337b54eda48c2d7f3ee
Change-Id...
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> 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(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 408e4da081da..39f1ce6d4f1c 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -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;
> }
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index 5242afabfaba..ee7ba5b5417e 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
` (4 preceding siblings ...)
2020-02-20 5:04 ` [PATCH v4 5/6] driver core: Rename deferred_probe_timeout and make it global John Stultz
@ 2020-02-20 5:04 ` John Stultz
2020-02-20 16:05 ` Bjorn Andersson
2020-02-20 10:15 ` [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() Rafael J. Wysocki
6 siblings, 1 reply; 21+ messages in thread
From: John Stultz @ 2020-02-20 5:04 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,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, linux-pm
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 <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: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
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(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d015d99cb59d..394e7b11576a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -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(®ulator_init_complete_work,
- msecs_to_jiffies(30000));
+ if (driver_deferred_probe_timeout >= 0)
+ schedule_delayed_work(®ulator_init_complete_work,
+ driver_deferred_probe_timeout * HZ);
+ else
+ regulator_init_complete_work_function(NULL);
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work
2020-02-20 5:04 ` [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work John Stultz
@ 2020-02-20 16:05 ` Bjorn Andersson
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2020-02-20 16:05 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:
> 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 <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: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: I9fa2411abbb91ed4dd0edc41e8cc8583577c005b
Change-Id...
> ---
> v4:
> * Split out into its own patch, as suggested by Mark
> ---
> drivers/regulator/core.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index d015d99cb59d..394e7b11576a 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -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(®ulator_init_complete_work,
> - msecs_to_jiffies(30000));
> + if (driver_deferred_probe_timeout >= 0)
> + schedule_delayed_work(®ulator_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
>
> return 0;
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state()
2020-02-20 5:04 [PATCH v4 0/6] driver core: Try to improve and cleanup driver_deferred_probe_check_state() John Stultz
` (5 preceding siblings ...)
2020-02-20 5:04 ` [PATCH v4 6/6] regulator: Use driver_deferred_probe_timeout for regulator_init_complete_work John Stultz
@ 2020-02-20 10:15 ` Rafael J. Wysocki
6 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-02-20 10:15 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Pavel Machek, Len Brown, Todd Kjos, Bjorn Andersson,
Liam Girdwood, Mark Brown, Thierry Reding, Linus Walleij,
Greg Kroah-Hartman, Linux PM
On Thu, Feb 20, 2020 at 6:04 AM John Stultz <john.stultz@linaro.org> wrote:
>
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread