From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751445AbbCJHcs (ORCPT ); Tue, 10 Mar 2015 03:32:48 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:42932 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750872AbbCJHcq (ORCPT ); Tue, 10 Mar 2015 03:32:46 -0400 MIME-Version: 1.0 In-Reply-To: <1425916192-31600-2-git-send-email-zahari.doychev@linux.com> References: <1425916192-31600-1-git-send-email-zahari.doychev@linux.com> <1425916192-31600-2-git-send-email-zahari.doychev@linux.com> Date: Tue, 10 Mar 2015 15:32:43 +0800 Message-ID: Subject: Re: [PATCH] drivers: base: fw: fix ret value when loading fw From: Ming Lei To: Zahari Doychev Cc: Greg Kroah-Hartman , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 9, 2015 at 11:49 PM, Zahari Doychev wrote: > When using the user mode helper to load firmwares the function _request_firmware > gets a positive return value from fw_load_from_user_helper and because of this > the firmware buffer is not assigned. This happens only when the return value > is zero. This patch fixes this problem in _request_firmware_load. When the > completion is ready the return value is set to zero. > > Signed-off-by: Zahari Doychev Acked-by: Ming Lei > --- > drivers/base/firmware_class.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > index 6c5c9ed..9642e5f 100644 > --- a/drivers/base/firmware_class.c > +++ b/drivers/base/firmware_class.c > @@ -920,6 +920,10 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, > else if (!buf->data) > retval = -ENOMEM; > > + /* wait for completion was successful so return ok */ > + if (retval > 0) > + retval = 0; Suggest to move the check backward and handle return value from wait_for_completion_interruptible_timeout() in one place, like below: if (retval == -ERESTARTSYS || !retval) { ... } else if (retval > 0) { retval = 0; } > + > device_remove_file(f_dev, &dev_attr_loading); > err_del_bin_attr: > device_remove_bin_file(f_dev, &firmware_attr_data); > -- > 2.3.0 >