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 <tiwai@xxxxxxx>
Subject: [PATCH] firmware: cache failed firmwares, too
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
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-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
