From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:48900 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbdK3USq (ORCPT ); Thu, 30 Nov 2017 15:18:46 -0500 Date: Thu, 30 Nov 2017 21:18:42 +0100 From: "Luis R. Rodriguez" To: Greg KH Cc: "Luis R. Rodriguez" , akpm@linux-foundation.org, keescook@chromium.org, mfuzzey@parkeon.com, zohar@linux.vnet.ibm.com, dhowells@redhat.com, pali.rohar@gmail.com, tiwai@suse.de, arend.vanspriel@broadcom.com, zajec5@gmail.com, nbroeking@me.com, markivx@codeaurora.org, stephen.boyd@linaro.org, broonie@kernel.org, dmitry.torokhov@gmail.com, dwmw2@infradead.org, torvalds@linux-foundation.org, Abhay_Salunke@dell.com, bjorn.andersson@linaro.org, jewalt@lgsinnovations.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 10/23] firmware: add helper to copy built-in data to pre-alloc buffer Message-ID: <20171130201842.GL729@wotan.suse.de> References: <20171120182409.27348-1-mcgrof@kernel.org> <20171120182409.27348-11-mcgrof@kernel.org> <20171129101715.GB29506@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171129101715.GB29506@kroah.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Nov 29, 2017 at 11:17:15AM +0100, Greg KH wrote: > On Mon, Nov 20, 2017 at 10:23:56AM -0800, Luis R. Rodriguez wrote: > > This makes it clearer that the parameters passed are only used for > > the preallocated buffer option, ie, when a caller uses: > > > > request_firmware_into_buf() > > > > Otherwise this code won't run. We flip the logic just so the actual > > prellocated buf code is not indented. > > > > Signed-off-by: Luis R. Rodriguez > > --- > > drivers/base/firmware_class.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > > index 4f64410fe7e6..aba3f2cbe2f4 100644 > > --- a/drivers/base/firmware_class.c > > +++ b/drivers/base/firmware_class.c > > @@ -146,6 +146,14 @@ static struct firmware_cache fw_cache; > > extern struct builtin_fw __start_builtin_fw[]; > > extern struct builtin_fw __end_builtin_fw[]; > > > > +static void fw_copy_to_prealloc_buf(struct firmware *fw, > > + void *buf, size_t size) > > +{ > > + if (!buf || size < fw->size) > > + return; > > Shouldn't this return an error? No, its a no-op when these are not set, and its actually *why* I created the function. The parameters are *only* useful for the prealloc buf fw calls. Keeping track of all this while reading the code is actually not easy and this should make it clearer. > > > + memcpy(buf, fw->data, fw->size); > > I'll take this, but it feels really odd... Thanks. Luis