lkml.org 
[lkml]   [2012]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC] firmware loader: retry _nowait requests when userhelper is not yet available
    Date
    On Friday 16 March 2012 23:19:53 Rafael J. Wysocki wrote:
    > > On 03/04/2012 01:52 AM, Christian Lamparter wrote:
    > >
    > > > During resume, the userhelper might not be available. However for
    > > > drivers which use the request_firmware_nowait interface, this will
    > > > only lead to a pointless WARNING and a device which no longer works
    > > > after the resume [since it couldn't get the firmware, because the
    > > > userhelper was not available to take the request].
    > > >
    > > > In order to solve this "chicken or egg" dilemma, the code now
    > > > retries _nowait requests at one second intervals until the
    > > > "loading_timeout" time is up.
    > > >
    > > > ---
    > > > I'm aware about the previous "request_firmware* in probe" discussions.
    > > > Unfortunately, the hardware needs firmware so there is no other way
    > > > around it. So please, I just wanted to know what the general opinion
    > > > about the idea behind this patch is.
    >
    > BTW, I wonder what comments on this patch were posted?
    Only Alan Cox was kind enough to drop me a few words.

    Why? Do you think it is actually sane from a specific POV?
    [Don't tell me you do :D !]

    Really, it wasn't until:
    "[PATCH] firmware loader: don't cancel _nowait requests when helper is not yet available"
    https://lkml.org/lkml/2012/3/9/612
    that things started to pick up.

    Regards,
    Chr

    > > > ---
    > > > drivers/base/firmware_class.c | 24 +++++++++++++++++++++++-
    > > > 1 files changed, 23 insertions(+), 1 deletions(-)
    > > >
    > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
    > > > index 6c9387d..9f70096 100644
    > > > --- a/drivers/base/firmware_class.c
    > > > +++ b/drivers/base/firmware_class.c
    > > > @@ -19,6 +19,7 @@
    > > > #include <linux/kthread.h>
    > > > #include <linux/highmem.h>
    > > > #include <linux/firmware.h>
    > > > +#include <linux/delay.h>
    > > > #include <linux/slab.h>
    > > >
    > > > #define to_dev(obj) container_of(obj, struct device, kobj)
    > > > @@ -535,6 +536,11 @@ static int _request_firmware(const struct firmware **firmware_p,
    > > >
    > > > read_lock_usermodehelper();
    > > >
    > > > + if (nowait && usermodehelper_is_disabled()) {
    > > > + retval = -EBUSY;
    > > > + goto out;
    > > > + }
    > > > +
    > > > if (WARN_ON(usermodehelper_is_disabled())) {
    > > > dev_err(device, "firmware: %s will not be loaded\n", name);
    > > > retval = -EBUSY;
    > > > @@ -633,7 +639,7 @@ static int request_firmware_work_func(void *arg)
    > > > {
    > > > struct firmware_work *fw_work = arg;
    > > > const struct firmware *fw;
    > > > - int ret;
    > > > + int ret, timeout = loading_timeout;
    > > >
    > > > if (!arg) {
    > > > WARN_ON(1);
    > > > @@ -642,6 +648,22 @@ static int request_firmware_work_func(void *arg)
    > > >
    > > > ret = _request_firmware(&fw, fw_work->name, fw_work->device,
    > > > fw_work->uevent, true);
    > > > +
    > > > + while (ret == -EBUSY) {
    > > > + /*
    > > > + * Try to retrieve the firmware within the loading timeout.
    > > > + * To stick with the loading timeout convention from above:
    > > > + * loading_timeout = 0 means 'try forever' as well.
    > > > + */
    > > > +
    > > > + msleep(1000);
    > > > + ret = _request_firmware(&fw, fw_work->name, fw_work->device,
    > > > + fw_work->uevent, true);
    > > > +
    > > > + if (timeout != 0 && timeout-- == 1)
    > >
    > > > + break;
    > >
    > > > + };
    > >
    > > > +
    > >
    > > > fw_work->cont(fw, fw_work->context);
    > > >
    > > > module_put(fw_work->module);
    > >


    \
     
     \ /
      Last update: 2012-03-16 23:29    [W:0.028 / U:30.336 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site