mbox series

[v4,0/6] Make deferring probe forever optional

Message ID 20180709154153.15742-1-robh@kernel.org
Headers show
Series Make deferring probe forever optional | expand

Message

Rob Herring (Arm) July 9, 2018, 3:41 p.m. UTC
This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.

I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver. I left the dts change out this time.

v2 and v3 of this series can be found here[2][3].

Rob

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html
[2] https://lore.kernel.org/patchwork/project/lkml/list/?series=347413
[3] https://lore.kernel.org/patchwork/project/lkml/list/?series=357344

Rob Herring (6):
  driver core: allow stopping deferred probe after init
  dt-bindings: pinctrl: add a 'pinctrl-use-default' property
  pinctrl: Support stopping deferred probe after initcalls
  iommu: Stop deferring probe at end of initcalls
  iommu: Remove IOMMU_OF_DECLARE
  PM / Domains: Stop deferring probe at the end of initcall

 .../admin-guide/kernel-parameters.txt         |  9 +++
 .../bindings/pinctrl/pinctrl-bindings.txt     |  6 ++
 drivers/base/dd.c                             | 59 +++++++++++++++++++
 drivers/base/power/domain.c                   |  2 +-
 drivers/iommu/arm-smmu-v3.c                   |  2 -
 drivers/iommu/arm-smmu.c                      |  7 ---
 drivers/iommu/exynos-iommu.c                  |  2 -
 drivers/iommu/ipmmu-vmsa.c                    |  3 -
 drivers/iommu/msm_iommu.c                     |  2 -
 drivers/iommu/of_iommu.c                      | 21 +------
 drivers/iommu/qcom_iommu.c                    |  2 -
 drivers/iommu/rockchip-iommu.c                |  2 -
 drivers/pinctrl/devicetree.c                  | 15 +++--
 include/asm-generic/vmlinux.lds.h             |  2 -
 include/linux/device.h                        |  2 +
 include/linux/of_iommu.h                      |  4 --
 16 files changed, 90 insertions(+), 50 deletions(-)

--
2.17.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Russell King (Oracle) July 9, 2018, 3:52 p.m. UTC | #1
On Mon, Jul 09, 2018 at 09:41:48AM -0600, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version. Deferred
> probe issues can be difficult to debug especially if the console has
> dependencies or userspace fails to boot to a shell.
> 
> There are also cases like IOMMUs where only built-in drivers are
> supported, so deferring probe after initcalls is not needed. The IOMMU
> subsystem implemented its own mechanism to handle this using OF_DECLARE
> linker sections.
> 
> This commit adds makes ending deferred probe conditional on initcalls
> being completed or a debug timeout. Subsystems or drivers may opt-in by
> calling driver_deferred_probe_check_init_done() instead of
> unconditionally returning -EPROBE_DEFER. They may use additional
> information from DT or kernel's config to decide whether to continue to
> defer probe or not.
> 
> The timeout mechanism is intended for debug purposes and WARNs loudly.
> The remaining deferred probe pending list will also be dumped after the
> timeout. Not that this timeout won't work for the console which needs
> to be enabled before userspace starts. However, if the console's
> dependencies are resolved, then the kernel log will be printed (as
> opposed to no output).

So what happens if we have a set of modules which use deferred probing
in order to work?

For example, with sound stuff built as modules, and auto-loaded in
parallel by udev, the modules get added in a random order.  The
modules have non-udev obvious dependencies between them (resource
dependencies) which result in deferred probing being necessary to
bring the device up.

Eg,

snd_soc_kirkwood_spdif module declares the ASoC card.
snd_soc_spdif_tx is a codec as a loadable module.
snd_soc_kirkwood is the CPU digital audio interface module.

What I commonly see is this module load order:

snd_soc_kirkwood_spdif, then snd_soc_kirkwood and then snd_soc_spdif_tx.

This results at boot in:

kirkwood-spdif-audio audio-subsystem: ASoC: CPU DAI kirkwood-fe not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CPU DAI kirkwood-fe not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CPU DAI kirkwood-fe not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CPU DAI kirkwood-fe not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CPU DAI kirkwood-fe not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CODEC DAI dit-hifi not registered
kirkwood-spdif-audio audio-subsystem: ASoC: CODEC DAI dit-hifi not registered
kirkwood-spdif-audio audio-subsystem: snd-soc-dummy-dai <-> kirkwood-fe mapping ok
kirkwood-spdif-audio audio-subsystem: multicodec <-> kirkwood-spdif mapping ok

at boot, where most of these are deferred probe attempts.

So, disabling deferred probing after all the kernel-internal initcalls
are run is wrong.  You can have deferred probing required due to
external modules, and this can kick in at any time (think about
hot-pluggable hardware with a driver that's somehow componentised,
like an audio device...)
Rob Herring (Arm) July 9, 2018, 5:47 p.m. UTC | #2
On Mon, Jul 9, 2018 at 9:52 AM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jul 09, 2018 at 09:41:48AM -0600, Rob Herring wrote:
> > Deferred probe will currently wait forever on dependent devices to probe,
> > but sometimes a driver will never exist. It's also not always critical for
> > a driver to exist. Platforms can rely on default configuration from the
> > bootloader or reset defaults for things such as pinctrl and power domains.
> > This is often the case with initial platform support until various drivers
> > get enabled. There's at least 2 scenarios where deferred probe can render
> > a platform broken. Both involve using a DT which has more devices and
> > dependencies than the kernel supports. The 1st case is a driver may be
> > disabled in the kernel config. The 2nd case is the kernel version may
> > simply not have the dependent driver. This can happen if using a newer DT
> > (provided by firmware perhaps) with a stable kernel version. Deferred
> > probe issues can be difficult to debug especially if the console has
> > dependencies or userspace fails to boot to a shell.
> >
> > There are also cases like IOMMUs where only built-in drivers are
> > supported, so deferring probe after initcalls is not needed. The IOMMU
> > subsystem implemented its own mechanism to handle this using OF_DECLARE
> > linker sections.
> >
> > This commit adds makes ending deferred probe conditional on initcalls
> > being completed or a debug timeout. Subsystems or drivers may opt-in by
> > calling driver_deferred_probe_check_init_done() instead of
> > unconditionally returning -EPROBE_DEFER. They may use additional
> > information from DT or kernel's config to decide whether to continue to
> > defer probe or not.
> >
> > The timeout mechanism is intended for debug purposes and WARNs loudly.
> > The remaining deferred probe pending list will also be dumped after the
> > timeout. Not that this timeout won't work for the console which needs
> > to be enabled before userspace starts. However, if the console's
> > dependencies are resolved, then the kernel log will be printed (as
> > opposed to no output).
>
> So what happens if we have a set of modules which use deferred probing
> in order to work?

It is opt-in by subsystem or drivers and mainly intended for
subsystems which can be optional or only support built-in drivers.
However, I don't really envision many other users other than the ones
I converted (pinctrl, iommu, pm-domains). If you look at patch 3,
you'll see it is dependent on !CONFIG_MODULES.

For the timeout, well, that's for debugging only. If you get to the
point of loading sound modules, you probably don't need the timeout.
It's for debugging not booting.

> For example, with sound stuff built as modules, and auto-loaded in
> parallel by udev, the modules get added in a random order.  The
> modules have non-udev obvious dependencies between them (resource
> dependencies) which result in deferred probing being necessary to
> bring the device up.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson July 9, 2018, 10:49 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) July 10, 2018, 2:25 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson July 11, 2018, 9:41 a.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html