mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Ming Lei <ming.lei@canonical.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org
Subject: [PATCH] firwmare: Correct handling of fw_state_wait() return value
Date: Tue,  6 Dec 2016 17:01:45 -0800	[thread overview]
Message-ID: <1481072505-2732-1-git-send-email-bjorn.andersson@linaro.org> (raw)

When request_firmware() finds an already open firmware object it will
wait for that object to become fully loaded and then check the status.
As __fw_state_wait_common() succeeds the timeout value returned will be
truncated in _request_firmware_prepare() and interpreted as -EPERM.

Prior to "firmware: do not use fw_lock for fw_state protection" the code
did test if we where in the "done" state before sleeping, causing this
particular code path to succeed, in some cases.

As the callers are interested in the result of the wait and not the
remaining timeout the return value of __fw_state_wait_common() is
changed to signal "done" or "error", which simplifies the logic in
_request_firmware_load() as well.

Fixes: 5b029624948d ("firmware: do not use fw_lock for fw_state protection")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

The regression was introduced in next-20161201

 drivers/base/firmware_class.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index eb95cf7c3b28..4497d263209f 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -127,7 +127,7 @@ static inline bool __fw_state_is_done(enum fw_status status)
 	return status == FW_STATUS_DONE || status == FW_STATUS_ABORTED;
 }
 
-static long __fw_state_wait_common(struct fw_state *fw_st, long timeout)
+static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
 {
 	long ret;
 
@@ -136,8 +136,10 @@ static long __fw_state_wait_common(struct fw_state *fw_st, long timeout)
 				timeout);
 	if (ret != 0 && fw_st->status == FW_STATUS_ABORTED)
 		return -ENOENT;
+	if (!ret)
+		return -ETIMEDOUT;
 
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 static void __fw_state_set(struct fw_state *fw_st,
@@ -1017,14 +1019,11 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 		timeout = MAX_JIFFY_OFFSET;
 	}
 
-	timeout = fw_state_wait_timeout(&buf->fw_st, timeout);
-	if (timeout == -ERESTARTSYS || !timeout) {
-		retval = timeout;
+	retval = fw_state_wait_timeout(&buf->fw_st, timeout);
+	if (retval < 0) {
 		mutex_lock(&fw_lock);
 		fw_load_abort(fw_priv);
 		mutex_unlock(&fw_lock);
-	} else if (timeout > 0) {
-		retval = 0;
 	}
 
 	if (fw_state_is_aborted(&buf->fw_st))
-- 
2.5.0

             reply	other threads:[~2016-12-07  1:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-07  1:01 Bjorn Andersson [this message]
2016-12-08 14:47 ` Daniel Wagner
2016-12-08 15:28   ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1481072505-2732-1-git-send-email-bjorn.andersson@linaro.org \
    --to=bjorn.andersson@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=ming.lei@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®