At Wed, 30 Jan 2013 19:48:15 +0800,
Ming Lei wrote:
>
> On Wed, Jan 30, 2013 at 7:08 PM, Takashi Iwai <ti...@suse.de> wrote:
> > At Wed, 30 Jan 2013 11:53:14 +0100,
> > Takashi Iwai wrote:
> >>
> >> At Wed, 30 Jan 2013 18:50:05 +0800,
> >> Ming Lei wrote:
> >> >
> >> > On Wed, Jan 30, 2013 at 6:31 PM, Takashi Iwai <ti...@suse.de> wrote:
> >> > >
> >> > > But it's supposed to be cached, no?
> >> >
> >> > Generally it will be cached, but some crazy devices might come as new
> >> > device during resume, so we still need to handle the situation.
> >>
> >> In that case, shouldn't we simply return an error instead of
> >> usermodehelper lock (at least for direct loading)?
> >
> > The patch below is what I have in my mind...
> >
> >
> > Takashi
> >
> > ---
> > From: Takashi Iwai <ti...@suse.de>
> > Subject: [PATCH] firmware: Skip usermodehelper_lock for direct loading
> >
> > We don't need a user mode helper lock for the direct fw loading.
> > This also reduces the use of timeout when user mode helper is
> > disabled, thus we can move the code into ifdef block, too.
> >
> > For avoiding the possible call of request_firmware() without caching,
> > add a check of suspend state for the direct loading case, and returns
> > immediately if it's called during suspend/resume. Then it proceeds to
> > the user mode helper if enabled, or returns the error.
> >
> > Signed-off-by: Takashi Iwai <ti...@suse.de>
> > ---
> > drivers/base/firmware_class.c | 97
> > ++++++++++++++++++++++++-------------------
> > 1 file changed, 54 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> > index f1caad7..c973bb0 100644
> > --- a/drivers/base/firmware_class.c
> > +++ b/drivers/base/firmware_class.c
> > @@ -88,13 +88,6 @@ enum {
> > FW_STATUS_ABORT,
> > };
> >
> > -static int loading_timeout = 60; /* In seconds */
> > -
> > -static inline long firmware_loading_timeout(void)
> > -{
> > - return loading_timeout > 0 ? loading_timeout * HZ :
> > MAX_SCHEDULE_TIMEOUT;
> > -}
> > -
> > struct firmware_cache {
> > /* firmware_buf instance will be added into the below list */
> > spinlock_t lock;
> > @@ -315,6 +308,9 @@ static bool fw_get_filesystem_firmware(struct device
> > *device,
> > bool success = false;
> > char *path = __getname();
> >
> > + if (device->power.is_suspended)
> > + return false;
>
> The device which is requesting firmware is resumed does not mean the
> storage device has been resumed, so this check isn't enough.
Well, this practically disables the call of request_firmware() in the
resume callback without caching.
Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
