From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932311Ab3E0MWs (ORCPT ); Mon, 27 May 2013 08:22:48 -0400 Received: from cantor2.suse.de ([195.135.220.15]:56571 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932110Ab3E0MWq (ORCPT ); Mon, 27 May 2013 08:22:46 -0400 Date: Mon, 27 May 2013 14:23:02 +0200 Message-ID: From: Takashi Iwai To: anish singh Cc: Ming Lei , Greg Kroah-Hartman , linux-kernel-mail Subject: Re: [PATCH 1/2] driver core: firmware loader: don't cache FW_ACTION_NOHOTPLUG firmware In-Reply-To: References: <1369650604-27941-1-git-send-email-ming.lei@canonical.com> <1369650604-27941-2-git-send-email-ming.lei@canonical.com> 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.2 (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 At Mon, 27 May 2013 17:26:22 +0530, anish singh wrote: > > On Mon, May 27, 2013 at 4:00 PM, Ming Lei wrote: > > Generally there are only two drivers which don't need uevent to > > handle firmware loading, so don't cache these firmwares during > Sorry but this statement confuses me i.e. "drivers which don't need > uevent to handle firmware loading". Does this mean that driver is > dependent on uevent to load the firmware? No. > or does this mean > that driver generates uevent to userspace and userpace in turn > provides the firmware? No. The userspace doesn't require uevent, and the driver doesn't generate uevent, either. The userspace just loads the file when ready. See Documentation/dell_rbu.txt for example. (And yes, it's a bad design.) Takashi > > suspend for these drivers since doing that may block firmware > > loading forever. > Explanation about why would it block would really help me or > for that matter anyone who reads this commit. Or may be > a url which discussed this problem. > > > > Both the two drivers are involved in private firmware images, so > > they don't hit in direct loading too. > > > > Cc: Takashi Iwai > > Signed-off-by: Ming Lei > > --- > > drivers/base/firmware_class.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > index e650c25..64e7870 100644 > > --- a/drivers/base/firmware_class.c > > +++ b/drivers/base/firmware_class.c > > @@ -993,7 +993,8 @@ _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 int assign_firmware_buf(struct firmware *fw, struct device *device, > > + bool skip_cache) > > { > > struct firmware_buf *buf = fw->priv; > > > > @@ -1010,7 +1011,7 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device) > > * device may has been deleted already, but the problem > > * should be fixed in devres or driver core. > > */ > > - if (device) > > + if (device && !skip_cache) > > fw_add_devm_name(device, buf->fw_id); > > > > /* > > @@ -1066,8 +1067,10 @@ _request_firmware(const struct firmware **firmware_p, const char *name, > > if (!fw_get_filesystem_firmware(device, fw->priv)) > > ret = fw_load_from_user_helper(fw, name, device, > > uevent, nowait, timeout); > > + > > + /* don't cache firmware handled without uevent */ > > if (!ret) > > - ret = assign_firmware_buf(fw, device); > > + ret = assign_firmware_buf(fw, device, !uevent); > > > > usermodehelper_read_unlock(); > > > > -- > > 1.7.9.5 > > > > -- > > 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/ >