diff mbox series

[v2,7/9] driver core: Set fw_devlink.strict=1 by default

Message ID 20220601070707.3946847-8-saravanak@google.com
State Accepted
Commit 71066545b48e4259f89481199a0bbc7c35457738
Headers show
Series deferred_probe_timeout logic clean up | expand

Commit Message

Saravana Kannan June 1, 2022, 7:07 a.m. UTC
Now that deferred_probe_timeout is non-zero by default, fw_devlink will
never permanently block the probing of devices. It'll try its best to
probe the devices in the right order and then finally let devices probe
even if their suppliers don't have any drivers.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sascha Hauer June 22, 2022, 7:47 a.m. UTC | #1
On Wed, Jun 01, 2022 at 12:07:03AM -0700, Saravana Kannan wrote:
> Now that deferred_probe_timeout is non-zero by default, fw_devlink will
> never permanently block the probing of devices. It'll try its best to
> probe the devices in the right order and then finally let devices probe
> even if their suppliers don't have any drivers.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/base/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

As mentioned here:

https://lore.kernel.org/lkml/20220622062027.994614-1-peng.fan@oss.nxp.com/

This patch has the effect that console UART devices which have "dmas"
properties specified in the device tree get deferred for 10 to 20
seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
the dma channel is only requested at UART startup time and not at probe
time. dma is not used for the console. Nevertheless with this driver probe
defers until the dma engine driver is available.

It shouldn't go in as-is.

Sascha

> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 61fdfe99b348..977b379a495b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1613,7 +1613,7 @@ static int __init fw_devlink_setup(char *arg)
>  }
>  early_param("fw_devlink", fw_devlink_setup);
>  
> -static bool fw_devlink_strict;
> +static bool fw_devlink_strict = true;
>  static int __init fw_devlink_strict_setup(char *arg)
>  {
>  	return strtobool(arg, &fw_devlink_strict);
> -- 
> 2.36.1.255.ge46751e96f-goog
> 
>
Linus Walleij June 22, 2022, 8:44 a.m. UTC | #2
On Wed, Jun 22, 2022 at 9:48 AM Sascha Hauer <sha@pengutronix.de> wrote:

> This patch has the effect that console UART devices which have "dmas"
> properties specified in the device tree get deferred for 10 to 20
> seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
> the dma channel is only requested at UART startup time and not at probe
> time. dma is not used for the console. Nevertheless with this driver probe
> defers until the dma engine driver is available.
>
> It shouldn't go in as-is.

This affects all machines with the PL011 UART and DMAs specified as
well.

It would be best if the console subsystem could be treated special and
not require DMA devlink to be satisfied before probing.

It seems devlink is not quite aware of the concept of resources that are
necessary to probe vs resources that are nice to have and might be
added after probe. We need a strong devlink for the first category
and maybe a weak devlink for the latter category.

I don't know if this is a generic hardware property for all operating
systems so it could be a DT property such as dma-weak-dependency?
Or maybe compromize and add a linux,dma-weak-dependency;
property?

Yours,
Linus Walleij
Andy Shevchenko June 22, 2022, 10:52 a.m. UTC | #3
On Wed, Jun 22, 2022 at 10:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jun 22, 2022 at 9:48 AM Sascha Hauer <sha@pengutronix.de> wrote:

...

> > This patch has the effect that console UART devices which have "dmas"
> > properties specified in the device tree get deferred for 10 to 20
> > seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
> > the dma channel is only requested at UART startup time and not at probe
> > time. dma is not used for the console. Nevertheless with this driver probe
> > defers until the dma engine driver is available.
> >
> > It shouldn't go in as-is.
>
> This affects all machines with the PL011 UART and DMAs specified as
> well.
>
> It would be best if the console subsystem could be treated special and
> not require DMA devlink to be satisfied before probing.

In 8250 we force disable DMA and PM on kernel consoles, because it's
so-o PITA and has a lot of corner cases we may never chase down.

089b6d365491 serial: 8250_port: Disable DMA operations for kernel console
bedb404e91bb serial: 8250_port: Don't use power management for kernel console


> It seems devlink is not quite aware of the concept of resources that are
> necessary to probe vs resources that are nice to have and might be
> added after probe. We need a strong devlink for the first category
> and maybe a weak devlink for the latter category.
>
> I don't know if this is a generic hardware property for all operating
> systems so it could be a DT property such as dma-weak-dependency?
> Or maybe compromize and add a linux,dma-weak-dependency;
> property?
Saravana Kannan June 22, 2022, 7:40 p.m. UTC | #4
On Wed, Jun 22, 2022 at 1:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Jun 22, 2022 at 9:48 AM Sascha Hauer <sha@pengutronix.de> wrote:
>
> > This patch has the effect that console UART devices which have "dmas"
> > properties specified in the device tree get deferred for 10 to 20
> > seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
> > the dma channel is only requested at UART startup time and not at probe
> > time. dma is not used for the console. Nevertheless with this driver probe
> > defers until the dma engine driver is available.

FYI, if most of the drivers are built in, you could set
deferred_probe_timeout=1 to reduce the impact of this (should drop
down to 1 to 2 seconds). Is that an option until we figure out
something better?

Actually, why isn't earlyconsole being used? That doesn't get blocked
on anything and the main point of that is to have console working from
really early on.

> >
> > It shouldn't go in as-is.
>
> This affects all machines with the PL011 UART and DMAs specified as
> well.
>
> It would be best if the console subsystem could be treated special and
> not require DMA devlink to be satisfied before probing.

If we can mark the console devices somehow before their drivers probe
them, I can make fw_devlink give them special treatment. Is there any
way I could identify them before their drivers probe?

> It seems devlink is not quite aware of the concept of resources that are
> necessary to probe vs resources that are nice to have and might be
> added after probe.

Correct, it can't tell them apart. Which is why it tries its best to
enforce them, get most of them ordered properly and then gives up
enforcing the rest after deferred_probe_timeout= expires. There's a
bit more nuance than what I explained here (explained in earlier
commit texts, LPC talks), but that's the gist of it. That's what's
going on in this case Sascha is pointing out.z

> We need a strong devlink for the first category
> and maybe a weak devlink for the latter category.
>
> I don't know if this is a generic hardware property for all operating
> systems so it could be a DT property such as dma-weak-dependency?
>
> Or maybe compromize and add a linux,dma-weak-dependency;
> property?

The linux,dma-weak-dependency might be an option, but then if the
kernel version changes and we want to enforce it because we now have a
dma driver (not related to Shasha's example) support, then the
fw_devlink still can't enforce it because of that property. But maybe
that's okay? The consumer can try to use dma and defer probe if it
fails?

Another option is to mark console devices in DT with some property and
we can give special treatment for those without waiting for
deferred_probe_timeout= to expire.

-Saravana
Saravana Kannan June 22, 2022, 8:35 p.m. UTC | #5
On Wed, Jun 22, 2022 at 12:40 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Wed, Jun 22, 2022 at 1:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Wed, Jun 22, 2022 at 9:48 AM Sascha Hauer <sha@pengutronix.de> wrote:
> >
> > > This patch has the effect that console UART devices which have "dmas"
> > > properties specified in the device tree get deferred for 10 to 20
> > > seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
> > > the dma channel is only requested at UART startup time and not at probe
> > > time. dma is not used for the console. Nevertheless with this driver probe
> > > defers until the dma engine driver is available.
>
> FYI, if most of the drivers are built in, you could set
> deferred_probe_timeout=1 to reduce the impact of this (should drop
> down to 1 to 2 seconds). Is that an option until we figure out
> something better?
>
> Actually, why isn't earlyconsole being used? That doesn't get blocked
> on anything and the main point of that is to have console working from
> really early on.
>
> > >
> > > It shouldn't go in as-is.
> >
> > This affects all machines with the PL011 UART and DMAs specified as
> > well.
> >
> > It would be best if the console subsystem could be treated special and
> > not require DMA devlink to be satisfied before probing.
>
> If we can mark the console devices somehow before their drivers probe
> them, I can make fw_devlink give them special treatment. Is there any
> way I could identify them before their drivers probe?
>
> > It seems devlink is not quite aware of the concept of resources that are
> > necessary to probe vs resources that are nice to have and might be
> > added after probe.
>
> Correct, it can't tell them apart. Which is why it tries its best to
> enforce them, get most of them ordered properly and then gives up
> enforcing the rest after deferred_probe_timeout= expires. There's a
> bit more nuance than what I explained here (explained in earlier
> commit texts, LPC talks), but that's the gist of it. That's what's
> going on in this case Sascha is pointing out.z
>
> > We need a strong devlink for the first category
> > and maybe a weak devlink for the latter category.
> >
> > I don't know if this is a generic hardware property for all operating
> > systems so it could be a DT property such as dma-weak-dependency?
> >
> > Or maybe compromize and add a linux,dma-weak-dependency;
> > property?
>
> The linux,dma-weak-dependency might be an option, but then if the
> kernel version changes and we want to enforce it because we now have a
> dma driver (not related to Shasha's example) support, then the
> fw_devlink still can't enforce it because of that property. But maybe
> that's okay? The consumer can try to use dma and defer probe if it
> fails?
>
> Another option is to mark console devices in DT with some property and
> we can give special treatment for those without waiting for
> deferred_probe_timeout= to expire.

Heh, looks like there's already a property for that: stdout-path.

Let me send a series that'll use that to give special treatment to
console devices.

-Saravana
Saravana Kannan June 22, 2022, 10:30 p.m. UTC | #6
On Wed, Jun 22, 2022 at 1:35 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Wed, Jun 22, 2022 at 12:40 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Wed, Jun 22, 2022 at 1:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > On Wed, Jun 22, 2022 at 9:48 AM Sascha Hauer <sha@pengutronix.de> wrote:
> > >
> > > > This patch has the effect that console UART devices which have "dmas"
> > > > properties specified in the device tree get deferred for 10 to 20
> > > > seconds. This happens on i.MX and likely on other SoCs as well. On i.MX
> > > > the dma channel is only requested at UART startup time and not at probe
> > > > time. dma is not used for the console. Nevertheless with this driver probe
> > > > defers until the dma engine driver is available.
> >
> > FYI, if most of the drivers are built in, you could set
> > deferred_probe_timeout=1 to reduce the impact of this (should drop
> > down to 1 to 2 seconds). Is that an option until we figure out
> > something better?
> >
> > Actually, why isn't earlyconsole being used? That doesn't get blocked
> > on anything and the main point of that is to have console working from
> > really early on.
> >
> > > >
> > > > It shouldn't go in as-is.
> > >
> > > This affects all machines with the PL011 UART and DMAs specified as
> > > well.
> > >
> > > It would be best if the console subsystem could be treated special and
> > > not require DMA devlink to be satisfied before probing.
> >
> > If we can mark the console devices somehow before their drivers probe
> > them, I can make fw_devlink give them special treatment. Is there any
> > way I could identify them before their drivers probe?
> >
> > > It seems devlink is not quite aware of the concept of resources that are
> > > necessary to probe vs resources that are nice to have and might be
> > > added after probe.
> >
> > Correct, it can't tell them apart. Which is why it tries its best to
> > enforce them, get most of them ordered properly and then gives up
> > enforcing the rest after deferred_probe_timeout= expires. There's a
> > bit more nuance than what I explained here (explained in earlier
> > commit texts, LPC talks), but that's the gist of it. That's what's
> > going on in this case Sascha is pointing out.z
> >
> > > We need a strong devlink for the first category
> > > and maybe a weak devlink for the latter category.
> > >
> > > I don't know if this is a generic hardware property for all operating
> > > systems so it could be a DT property such as dma-weak-dependency?
> > >
> > > Or maybe compromize and add a linux,dma-weak-dependency;
> > > property?
> >
> > The linux,dma-weak-dependency might be an option, but then if the
> > kernel version changes and we want to enforce it because we now have a
> > dma driver (not related to Shasha's example) support, then the
> > fw_devlink still can't enforce it because of that property. But maybe
> > that's okay? The consumer can try to use dma and defer probe if it
> > fails?
> >
> > Another option is to mark console devices in DT with some property and
> > we can give special treatment for those without waiting for
> > deferred_probe_timeout= to expire.
>
> Heh, looks like there's already a property for that: stdout-path.
>
> Let me send a series that'll use that to give special treatment to
> console devices.

Here's the fix.
https://lore.kernel.org/lkml/20220622215912.550419-1-saravanak@google.com/

Sascha, can you give it a shot?

-Saravana
diff mbox series

Patch

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 61fdfe99b348..977b379a495b 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1613,7 +1613,7 @@  static int __init fw_devlink_setup(char *arg)
 }
 early_param("fw_devlink", fw_devlink_setup);
 
-static bool fw_devlink_strict;
+static bool fw_devlink_strict = true;
 static int __init fw_devlink_strict_setup(char *arg)
 {
 	return strtobool(arg, &fw_devlink_strict);