Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753385AbbETMop (ORCPT ); Wed, 20 May 2015 08:44:45 -0400 Received: from cantor2.suse.de ([195.135.220.15]:45050 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751876AbbETMom (ORCPT ); Wed, 20 May 2015 08:44:42 -0400 Date: Wed, 20 May 2015 14:44:37 +0200 Message-ID: From: Takashi Iwai To: Marcel Holtmann Cc: Oliver Neukum , Ming Lei , "David S. Miller" , Laura Abbott , Johan Hedberg , "Rafael J. Wysocki" , "Gustavo F. Padovan" , Laura Abbott , Alan Stern , "bluez mailin list (linux-bluetooth@vger.kernel.org)" , Linux Kernel Mailing List , USB list , netdev Subject: Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable In-Reply-To: <9BCD6CFF-0356-4A0F-97D4-776C0A2E436A@holtmann.org> References: <1432057375.3970.4.camel@suse.com> <1432111237.8287.3.camel@suse.com> <9BCD6CFF-0356-4A0F-97D4-776C0A2E436A@holtmann.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4364 Lines: 132 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 Subject: [PATCH] firmware: cache failed firmwares, too Signed-off-by: Takashi Iwai --- 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/