mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@suse.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arend van Spriel <arend@broadcom.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ming Lei <ming.lei@canonical.com>, Takashi Iwai <tiwai@suse.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Liam Girdwood <liam.r.girdwood@linux.intel.com>,
	"Jie, Yang" <yang.jie@intel.com>,
	"joonas.lahtinen@linux.intel.com"
	<joonas.lahtinen@linux.intel.com>, Tom Gundersen <teg@jklm.no>,
	Al Viro <viro@zeniv.linux.org.uk>, Kay Sievers <kay@vrfy.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Luis Rodriguez <mcgrof@do-not-panic.com>,
	lkml <linux-kernel@vger.kernel.org>,
	yalin wang <yalin.wang2010@gmail.com>
Subject: Re: Problems loading firmware using built-in drivers with kernels that use initramfs.
Date: Thu, 3 Sep 2015 01:22:49 +0200	[thread overview]
Message-ID: <20150902232249.GK8051@wotan.suse.de> (raw)
In-Reply-To: <CAKdAkRSWghMptgD3hfr6VJBrAyzLw+p842zSNovHKSqJiGOP1w@mail.gmail.com>

On Wed, Sep 02, 2015 at 04:13:51PM -0700, Dmitry Torokhov wrote:
> On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
> > On 09/02/2015 08:58 PM, Luis R. Rodriguez wrote:
> >>
> >> On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
> >>>
> >>> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
> >>>>
> >>>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
> >>>>>
> >>>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
> >>>>>>
> >>>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
> >>>>>> <arend@broadcom.com> wrote:
> >>>>>>>
> >>>>>>> Does this mean a built-in driver can not get firmware from initramfs
> >>>>>>> or
> >>>>>>> built in the kernel early. Seems a bit too aggressive. The problem
> >>>>>>> stated in
> >>>>>>> this thread is when the firmware is not on initramfs but only on the
> >>>>>>> rootfs.
> >>>>>>
> >>>>>>
> >>>>>> Yes, strictly speaking, user mode request can't be handled with defer
> >>>>>> probe
> >>>>>> during booting because we don't know how the user helper handles the
> >>>>>> request,
> >>>>>
> >>>>>
> >>>>> FWIW I have a strategy in mind to help us compartamentalize the user
> >>>>> mode
> >>>>> helper only to the dell-rbu driver, and as such phase out that code
> >>>>> eventually
> >>>>> completely. Its part of the goals I have with the extensible firmware
> >>>>> API I've
> >>>>> been proposing.
> >>>>>
> >>>>>> that said even checking if the firmware exists in current path doesn't
> >>>>>> make sense for user mode request.
> >>>>>>
> >>>>>> So the patch should have used defer proble for direct load only
> >>>>>> during booting.
> >>>>>
> >>>>>
> >>>>> What exact guarantees would we be giving to callers if they follow up
> >>>>> on probe
> >>>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
> >>>>> / probe
> >>>>> (note that unless you're using async probe since we batch both so it
> >>>>> doesn't really
> >>>>> matter where you place your code) all together and then for the few
> >>>>> remaining
> >>>>> stragglers understand the requirements and provide an interface that
> >>>>> lets them
> >>>>> claim their requirements and try to meets them.
> >>>>>
> >>>>> A grammatical hunt for drivers who call fw API on init / probe can be
> >>>>> completed, although I know the hunt needs a bit more fine tuning it
> >>>>> surely can
> >>>>> be completed. If we don't have many callers the compexity added for
> >>>>> only a
> >>>>> few callers with rather loose criteria seems rather unnecessary,
> >>>>> specially if
> >>>>> we can change the drivers and make these driver sthe exception rather
> >>>>> than
> >>>>> a norm.
> >>>>>
> >>>>> Then as for drivers *needing* the fw at probe why not have a proper
> >>>>> interface
> >>>>> that does guarantee they get the requirements they ask for first ? For
> >>>>> instance
> >>>>> a new probe type specified by the driver could enable the core to wait
> >>>>> for say
> >>>>> an event and then tirgger a probe, kind of how we ended up defining
> >>>>> the async
> >>>>> probe type preference:
> >>>>>
> >>>>> static struct some_bus_driver some_driver = {
> >>>>>          .probe = some_probe,
> >>>>>          .id_table = some_id,
> >>>>>          .driver = {
> >>>>>                  .name = DEVICE_NAME,
> >>>>>                  .pm = &some_pm_ops,
> >>>>>                  .probe_type = PROBE_PREFER_POST_FOO,
> >>>>>          },
> >>>>> };
> >>>>>
> >>>>> Then we just don't try just hoping for completion but rather can do
> >>>>> something
> >>>>> about the criteria passed.
> >>>
> >>>
> >>> So should the probe type indicate some event or should it just
> >>> indicate what the driver needs, ie. .probe_type =
> >>> PROBE_TYPE_NEED_FW.
> >>
> >>
> >> Right so this is an open question. I suggested something like the above
> >> since the deferred probe documentation on drivers/base/dd.c states:
> >>
> >>   * Sometimes driver probe order matters, but the kernel doesn't always
> >> have
> >>   * dependency information
> >>
> >> I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
> >> kernel where some work can be done to not only list the dependency
> >> the information from the driver but also we know we can get it from
> >> the kernel. In this case I do believe we could not only express the
> >> requirement but also wait for it in the kernel. Before we do that
> >> though I think it'd be good to do a grammar hunt to determine exactly
> >> how popular all this fw on probe needed really is.
> >
> >
> > Ok. So some background why we need it in brcm80211 drivers. So as a wireless
> > network device driver the answer we got when asking for an event to load
> > firware is upon IF_UP for a registered net device. Because we try to do
> > things smart we query the firmware running on the device for capabilities
> > before we can register the net device hence we request the firmware during
> > probe. This may be specific to wireless drivers (Intel has same approach if
> > not mistaken) but I suspect there may be more.
> 
> We have the same issue with input devices: before we can register one
> we need to set their capabilities and to know their capabilities we
> quite often need to load their firmware/config and query the device.

Should Arend's driver use async probe then?

IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
preemptively hacky. Sadly I can't think of clear and clever way for the kernel
to know when firmware will be ready either...  Would userspace know? Should the
kernel learn this from userspace ?

For instance, if init is going to use initramfs it can pivot_root() and later
send us a smoke signal when done, and it it doesn't it can also send us a smoke
signal. In the absence of such a hint being implemented I suppose -EPROBE_DEFER,
async probe or a notifier on pivot_root() is best effort we can do, but again
*eh*.

  Luis

  reply	other threads:[~2015-09-02 23:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1440449403.2469.35.camel@loki>
     [not found] ` <1440489900.2419.4.camel@loki>
     [not found]   ` <rbhuamrc6ajy3l1qereb60l8.1440494220682@email.android.com>
2015-08-25 19:34     ` Luis R. Rodriguez
2015-08-25 19:46       ` Takashi Iwai
2015-08-25 19:58         ` Dmitry Torokhov
2015-08-25 20:26           ` Linus Torvalds
2015-08-26  5:30             ` yalin wang
2015-08-26  5:12           ` Jie, Yang
2015-08-26  5:32             ` Takashi Iwai
2015-08-26  6:17               ` Jie, Yang
2015-08-26  8:06                 ` Liam Girdwood
2015-08-26  8:29                   ` Jie, Yang
2015-08-26  9:00                     ` Liam Girdwood
2015-08-27  1:50                       ` Lin, Mengdong
2015-08-27  7:09                         ` Liam Girdwood
2015-08-26 18:07                   ` Linus Torvalds
2015-08-27  0:55                     ` Ming Lei
2015-08-29  1:11                       ` Luis R. Rodriguez
2015-08-29  4:09                         ` Ming Lei
2015-08-29  7:11                           ` Takashi Iwai
2015-08-29  8:50                             ` Arend van Spriel
2015-08-29 10:38                               ` Ming Lei
2015-08-30  8:25                                 ` Arend van Spriel
2015-08-30 18:11                                   ` Linus Torvalds
2015-12-17 19:24                                     ` Luis R. Rodriguez
2015-08-31 14:21                                   ` Ming Lei
2015-09-02  1:19                                     ` Luis R. Rodriguez
2015-09-02 12:09                                       ` Arend van Spriel
2015-09-02 12:13                                         ` Arend van Spriel
2015-09-02 18:58                                           ` Luis R. Rodriguez
2015-09-02 21:03                                             ` Arend van Spriel
2015-09-02 23:13                                               ` Dmitry Torokhov
2015-09-02 23:22                                                 ` Luis R. Rodriguez [this message]
2015-09-02 23:29                                                   ` Dmitry Torokhov
2015-09-02 23:46                                                     ` Luis R. Rodriguez
2015-09-03 17:23                                                       ` Arend van Spriel
2015-09-03 17:33                                                         ` Dmitry Torokhov
2015-10-16 19:35                                                           ` Luis R. Rodriguez
2015-10-16 21:05                                                             ` Luis R. Rodriguez
2015-10-17  8:31                                                             ` Arend van Spriel
2015-09-02 20:43                                           ` Dmitry Torokhov
2015-09-02  0:39                           ` Luis R. Rodriguez

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=20150902232249.GK8051@wotan.suse.de \
    --to=mcgrof@suse.com \
    --cc=arend@broadcom.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=kay@vrfy.org \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@do-not-panic.com \
    --cc=ming.lei@canonical.com \
    --cc=teg@jklm.no \
    --cc=tiwai@suse.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yalin.wang2010@gmail.com \
    --cc=yang.jie@intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®