At Wed, 20 May 2015 11:46:31 +0200,
Marcel Holtmann wrote:
> 
> Hi Oliver,
> 
> >> The data is cached in RAM.  More specifically, the former loaded
> >> firmware files are reloaded and saved at suspend for each device
> >> object.  See fw_pm_notify() in firmware_class.c.
> > 
> > OK, this may be a stupid idea, but do we know the firmware
> > was successfully loaded in the first place?
> > Also btusb is in the habit of falling back to a generic
> > firmware in some places. It seems to me that caching
> > firmware is conceptually not enough, but we'd also need
> > to record the absence of firmware images.
> 
> in a lot of cases the firmware is optional. The device will operate fine 
> without the firmware. There are a few devices where the firmware is required, 
> but for many it just contains patches.
> 
> It would be nice if we could tell request_firmware() if it is optional or 
> mandatory firmware. Or if it should just cache the status of a missing 
> firmware as well.

OK, below is a quick hack to record the failed f/w files, too.
Not sure whether this helps, though.  Proper tests are appreciated.


Takashi

---
From: Takashi Iwai <ti...@suse.de>
Subject: [PATCH] firmware: cache failed firmwares, too

Signed-off-by: Takashi Iwai <ti...@suse.de>
---
 drivers/base/firmware_class.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 171841ad1008..a15af7289c94 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1035,6 +1035,8 @@ _request_firmware_prepare(struct firmware **firmware_p, 
const char *name,
        firmware->priv = buf;
 
        if (ret > 0) {
+               if (buf->size == -1UL)
+                       return -ENOENT; /* already recorded as failure */
                ret = sync_cached_firmware_buf(buf);
                if (!ret) {
                        fw_set_page_data(buf, firmware);
@@ -1047,17 +1049,12 @@ _request_firmware_prepare(struct firmware **firmware_p, 
const char *name,
        return 1; /* need to load */
 }
 
-static int assign_firmware_buf(struct firmware *fw, struct device *device,
+static void assign_firmware_buf(struct firmware *fw, struct device *device,
                               unsigned int opt_flags)
 {
        struct firmware_buf *buf = fw->priv;
 
        mutex_lock(&fw_lock);
-       if (!buf->size || is_fw_load_aborted(buf)) {
-               mutex_unlock(&fw_lock);
-               return -ENOENT;
-       }
-
        /*
         * add firmware name into devres list so that we can auto cache
         * and uncache firmware for device.
@@ -1079,9 +1076,9 @@ static int assign_firmware_buf(struct firmware *fw, 
struct device *device,
        }
 
        /* pass the pages buffer to driver at the last minute */
-       fw_set_page_data(buf, fw);
+       if (buf->size != -1UL)
+               fw_set_page_data(buf, fw);
        mutex_unlock(&fw_lock);
-       return 0;
 }
 
 /* called from request_firmware() and request_firmware_work_func() */
@@ -1124,6 +1121,9 @@ _request_firmware(const struct firmware **firmware_p, 
const char *name,
 
        ret = fw_get_filesystem_firmware(device, fw->priv);
        if (ret) {
+               struct firmware_buf *buf = fw->priv;
+
+               buf->size = -1UL; /* failed */
                if (!(opt_flags & FW_OPT_NO_WARN))
                        dev_warn(device,
                                 "Direct firmware load for %s failed with error 
%d\n",
@@ -1132,12 +1132,12 @@ _request_firmware(const struct firmware **firmware_p, 
const char *name,
                        dev_warn(device, "Falling back to user helper\n");
                        ret = fw_load_from_user_helper(fw, name, device,
                                                       opt_flags, timeout);
+                       if (ret)
+                               buf->size = -1UL; /* failed */
                }
        }
 
-       if (!ret)
-               ret = assign_firmware_buf(fw, device, opt_flags);
-
+       assign_firmware_buf(fw, device, opt_flags);
        usermodehelper_read_unlock();
 
  out:
@@ -1435,17 +1435,8 @@ static void __async_dev_cache_fw_image(void *fw_entry,
                                       async_cookie_t cookie)
 {
        struct fw_cache_entry *fce = fw_entry;
-       struct firmware_cache *fwc = &fw_cache;
-       int ret;
-
-       ret = cache_firmware(fce->name);
-       if (ret) {
-               spin_lock(&fwc->name_lock);
-               list_del(&fce->list);
-               spin_unlock(&fwc->name_lock);
 
-               free_fw_cache_entry(fce);
-       }
+       cache_firmware(fce->name);
 }
 
 /* called with dev->devres_lock held */
-- 
2.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to