diff mbox

[v4,6/6] PM / Domains: Stop deferring probe at the end of initcall

Message ID 20180709154153.15742-7-robh@kernel.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Rob Herring July 9, 2018, 3:41 p.m. UTC
All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).

Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.

Acked-by: "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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v4:
 - Add Rafael's ack

v3:
  - Update to new function name

 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.17.1

Comments

Ulf Hansson July 9, 2018, 10:49 p.m. UTC | #1
On 9 July 2018 at 17:41, Rob Herring <robh@kernel.org> wrote:
> All PM domain drivers must be built-in (at least those using DT), so
> there is no point deferring probe after initcalls are done. Continuing
> to defer probe may prevent booting successfully even if managing PM
> domains is not required. This can happen if the user failed to enable
> the driver or if power-domains are added to a platform's DT, but there
> is not yet a driver (e.g. a new DTB with an old kernel).
>
> Call the driver core function driver_deferred_probe_check_init_done()
> instead of just returning -EPROBE_DEFER to stop deferring probe when
> initcalls are done.
>
> Acked-by: "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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v4:
>  - Add Rafael's ack
>
> v3:
>   - Update to new function name
>
>  drivers/base/power/domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 4925af5c4cf0..8c12213875c6 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2253,7 +2253,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
>                 mutex_unlock(&gpd_list_lock);
>                 dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
>                         __func__, PTR_ERR(pd));
> -               return -EPROBE_DEFER;
> +               return driver_deferred_probe_check_state(dev);
>         }

This isn't the only error path where -EPROBE_DEFER may be returned
during attach in genpd.

Have a look at the callers of __genpd_dev_pm_attach() and also have a
second look inside __genpd_dev_pm_attach() itself. And in Rafael's
tree.

>
>         dev_dbg(dev, "adding to PM domain %s\n", pd->name);
> --
> 2.17.1

Besides the above, I am fine with the approach as such.

Kind regards
Uffe
Rob Herring July 10, 2018, 2:25 p.m. UTC | #2
On Mon, Jul 9, 2018 at 4:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On 9 July 2018 at 17:41, Rob Herring <robh@kernel.org> wrote:
> > All PM domain drivers must be built-in (at least those using DT), so
> > there is no point deferring probe after initcalls are done. Continuing
> > to defer probe may prevent booting successfully even if managing PM
> > domains is not required. This can happen if the user failed to enable
> > the driver or if power-domains are added to a platform's DT, but there
> > is not yet a driver (e.g. a new DTB with an old kernel).
> >
> > Call the driver core function driver_deferred_probe_check_init_done()
> > instead of just returning -EPROBE_DEFER to stop deferring probe when
> > initcalls are done.
> >
> > Acked-by: "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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > v4:
> >  - Add Rafael's ack
> >
> > v3:
> >   - Update to new function name
> >
> >  drivers/base/power/domain.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 4925af5c4cf0..8c12213875c6 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -2253,7 +2253,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> >                 mutex_unlock(&gpd_list_lock);
> >                 dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
> >                         __func__, PTR_ERR(pd));
> > -               return -EPROBE_DEFER;
> > +               return driver_deferred_probe_check_state(dev);
> >         }
>
> This isn't the only error path where -EPROBE_DEFER may be returned
> during attach in genpd.

Yes, but I think this is the only place you can fail to get the PD
device from DT. The case we care about is properties exist in the DT,
but no driver exists.

What would cause deferring in the latter cases?

Rob
Ulf Hansson July 11, 2018, 9:41 a.m. UTC | #3
On 10 July 2018 at 16:25, Rob Herring <robh@kernel.org> wrote:
> On Mon, Jul 9, 2018 at 4:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On 9 July 2018 at 17:41, Rob Herring <robh@kernel.org> wrote:
>> > All PM domain drivers must be built-in (at least those using DT), so
>> > there is no point deferring probe after initcalls are done. Continuing
>> > to defer probe may prevent booting successfully even if managing PM
>> > domains is not required. This can happen if the user failed to enable
>> > the driver or if power-domains are added to a platform's DT, but there
>> > is not yet a driver (e.g. a new DTB with an old kernel).
>> >
>> > Call the driver core function driver_deferred_probe_check_init_done()
>> > instead of just returning -EPROBE_DEFER to stop deferring probe when
>> > initcalls are done.
>> >
>> > Acked-by: "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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: linux-pm@vger.kernel.org
>> > Signed-off-by: Rob Herring <robh@kernel.org>
>> > ---
>> > v4:
>> >  - Add Rafael's ack
>> >
>> > v3:
>> >   - Update to new function name
>> >
>> >  drivers/base/power/domain.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> > index 4925af5c4cf0..8c12213875c6 100644
>> > --- a/drivers/base/power/domain.c
>> > +++ b/drivers/base/power/domain.c
>> > @@ -2253,7 +2253,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
>> >                 mutex_unlock(&gpd_list_lock);
>> >                 dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
>> >                         __func__, PTR_ERR(pd));
>> > -               return -EPROBE_DEFER;
>> > +               return driver_deferred_probe_check_state(dev);
>> >         }
>>
>> This isn't the only error path where -EPROBE_DEFER may be returned
>> during attach in genpd.
>
> Yes, but I think this is the only place you can fail to get the PD
> device from DT. The case we care about is properties exist in the DT,
> but no driver exists.

That's correct. Thanks for clarifying!

>
> What would cause deferring in the latter cases?

To power on the PM domain for example, which often is required to be
able to probe the device.

>
> Rob

Kind regards
Uffe
diff mbox

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 4925af5c4cf0..8c12213875c6 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2253,7 +2253,7 @@  static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return -EPROBE_DEFER;
+		return driver_deferred_probe_check_state(dev);
 	}

 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);