All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <jstultz@google.com>
To: Javier Martinez Canillas <javierm@redhat.com>
Cc: linux-kernel@vger.kernel.org,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Douglas Anderson <dianders@chromium.org>,
	Saravana Kannan <saravanak@google.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	linux-arm-msm@vger.kernel.org,
	Peter Robinson <pbrobinson@redhat.com>,
	Steev Klimaszewski <steev@kali.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Enric Balletbo i Serra <eballetbo@redhat.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Brian Masney <bmasney@redhat.com>, Rob Herring <robh@kernel.org>
Subject: Re: [PATCH v2 3/4] driver core: Add fw_devlink.timeout param to stop waiting for devlinks
Date: Thu, 17 Nov 2022 11:07:52 -0800	[thread overview]
Message-ID: <CANDhNCr7ZwbCDK1ftigLK_S2qASj1yfenUG1WPaiYbjr5M9x3w@mail.gmail.com> (raw)
In-Reply-To: <20221116120159.519908-1-javierm@redhat.com>

On Wed, Nov 16, 2022 at 4:02 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
>
> Currently, the probe deferral timeout does two things:
>
> 1) Call to fw_devlink_drivers_done() to relax the device dependencies and
>    allow drivers to be probed if these dependencies are optional.
>
> 2) Disable the probe deferral mechanism so that drivers will fail to probe
>    if the required dependencies are not present, instead of adding them to
>    the deferred probe pending list.
>
> But there is no need to couple these two, for example the probe deferral
> can be used even when the device links are disable (i.e: fw_devlink=off).
>
> So let's add a separate fw_devlink.timeout command line parameter to allow
> relaxing the device links and prevent drivers to wait for these to probe.

I'm probably being dim, but it's not immediately clear from this
description *why* this is useful. Maybe add some words on the tangible
benefit of splitting this up?

I'd also push a little bit back on why we need to split this into a
separate boot option. Since it's not obvious as to when a user would
want to use fw_devlink.timeout vs probe_deferral_timeout.
The extra complexity of remembering which timeout is for what might
become a burden to users and developers.

>
> +       fw_devlink.timeout=
> +                       [KNL] Debugging option to set a timeout in seconds for
> +                       drivers to give up waiting on dependencies and to probe
> +                       these are optional. A timeout of 0 will timeout at the
> +                       end of initcalls. If the time out hasn't expired, it'll
> +                       be restarted by each successful driver registration.
> +

This sounds pretty close to like the deferred_probe_timeout option.
I'd suggest some words to make the distinction more clear.

thanks
-john

  parent reply	other threads:[~2022-11-17 19:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-16 11:53 [PATCH v2 0/4] driver core: Decouple device links enforcing and probe deferral timeouts Javier Martinez Canillas
2022-11-16 11:53 ` [PATCH v2 1/4] driver core: Make driver_deferred_probe_timeout a static variable Javier Martinez Canillas
2022-11-16 12:57   ` Andrew Halaney
2022-11-17 19:14   ` John Stultz
2022-12-12  8:50     ` Javier Martinez Canillas
2022-12-12  8:59       ` Greg Kroah-Hartman
2022-12-12  9:06         ` Javier Martinez Canillas
2022-11-16 12:00 ` [PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled Javier Martinez Canillas
2022-11-16 16:39   ` Andrew Halaney
2022-11-16 16:51     ` Javier Martinez Canillas
2022-11-16 12:01 ` [PATCH v2 3/4] driver core: Add fw_devlink.timeout param to stop waiting for devlinks Javier Martinez Canillas
2022-11-16 16:09   ` Randy Dunlap
2022-11-16 19:05   ` Andrew Halaney
2022-11-17 15:19   ` Andrew Halaney
2022-11-17 18:55     ` Javier Martinez Canillas
2022-11-17 19:07   ` John Stultz [this message]
2022-11-17 19:16     ` Javier Martinez Canillas
2022-11-16 12:02 ` [PATCH v2 4/4] driver core: Disable driver deferred probe timeout by default Javier Martinez Canillas
2022-11-16 19:15   ` Andrew Halaney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANDhNCr7ZwbCDK1ftigLK_S2qASj1yfenUG1WPaiYbjr5M9x3w@mail.gmail.com \
    --to=jstultz@google.com \
    --cc=andersson@kernel.org \
    --cc=bmasney@redhat.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=eballetbo@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=javierm@redhat.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbrobinson@redhat.com \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=saravanak@google.com \
    --cc=steev@kali.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.