mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Daniel Wagner <wagi@monom.org>
To: linux-kernel@vger.kernel.org
Cc: Ming Lei <ming.lei@canonical.com>,
	"Luis R . Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Srivatsa S . Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
	"Rafael J . Wysocki" <rjw@sisk.pl>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Takashi Iwai <tiwai@suse.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Arend van Spriel <arend.vanspriel@broadcom.com>,
	Daniel Wagner <daniel.wagner@bmw-carit.de>
Subject: [PATCH v5 2/5] firmware: encapsulate firmware loading status
Date: Fri,  9 Sep 2016 14:12:21 +0200	[thread overview]
Message-ID: <1473423144-21734-3-git-send-email-wagi@monom.org> (raw)
In-Reply-To: <1473423144-21734-1-git-send-email-wagi@monom.org>

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

The firmware user helper code tracks the current state of the loading
process via unsigned long status and a complection in struct
firmware_buf. We only need this for the usermode helper as such we can
encapsulate all this data into its own data structure.

Cc: Ming Lei <ming.lei@canonical.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
 drivers/base/firmware_class.c | 130 +++++++++++++++++++++++++++++-------------
 1 file changed, 91 insertions(+), 39 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8eba1fb..821babe 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -91,12 +91,6 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
 }
 #endif
 
-enum {
-	FW_STATUS_LOADING,
-	FW_STATUS_DONE,
-	FW_STATUS_ABORT,
-};
-
 static int loading_timeout = 60;	/* In seconds */
 
 static inline long firmware_loading_timeout(void)
@@ -104,6 +98,77 @@ static inline long firmware_loading_timeout(void)
 	return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
 }
 
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+
+enum {
+	FW_UMH_UNKNOWN,
+	FW_UMH_LOADING,
+	FW_UMH_DONE,
+	FW_UMH_ABORTED,
+};
+
+struct fw_umh {
+	struct completion completion;
+	unsigned long status;
+};
+
+static void fw_umh_init(struct fw_umh *fw_umh)
+{
+	init_completion(&fw_umh->completion);
+	fw_umh->status = FW_UMH_UNKNOWN;
+}
+
+static int __fw_umh_check(struct fw_umh *fw_umh, unsigned long status)
+{
+	return test_bit(status, &fw_umh->status);
+}
+
+static int fw_umh_wait_timeout(struct fw_umh *fw_umh, long timeout)
+{
+	int ret;
+
+	ret = wait_for_completion_interruptible_timeout(&fw_umh->completion,
+							timeout);
+	if (ret != 0 && test_bit(FW_UMH_ABORTED, &fw_umh->status))
+		return -ENOENT;
+
+	return ret;
+}
+
+static void __fw_umh_set(struct fw_umh *fw_umh,
+			  unsigned long status)
+{
+	set_bit(status, &fw_umh->status);
+
+	if (status == FW_UMH_DONE || status == FW_UMH_ABORTED) {
+		clear_bit(FW_UMH_LOADING, &fw_umh->status);
+		complete_all(&fw_umh->completion);
+	}
+}
+
+#define fw_umh_start(fw_umh)					\
+	__fw_umh_set(fw_umh, FW_UMH_LOADING)
+#define fw_umh_done(fw_umh)					\
+	__fw_umh_set(fw_umh, FW_UMH_DONE)
+#define fw_umh_aborted(fw_umh)					\
+	__fw_umh_set(fw_umh, FW_UMH_ABORTED)
+#define fw_umh_is_loading(fw_umh)				\
+	__fw_umh_check(fw_umh, FW_UMH_LOADING)
+#define fw_umh_is_done(fw_umh)					\
+	__fw_umh_check(fw_umh, FW_UMH_DONE)
+#define fw_umh_is_aborted(fw_umh)				\
+	__fw_umh_check(fw_umh, FW_UMH_ABORTED)
+
+#else /* CONFIG_FW_LOADER_USER_HELPER */
+
+#define fw_umh_wait_timeout(fw_st, long)	0
+
+#define fw_umh_done(fw_st)
+#define fw_umh_is_done(fw_st)			true
+#define fw_umh_is_aborted(fw_st)		false
+
+#endif /* !CONFIG_FW_LOADER_USER_HELPER */
+
 /* firmware behavior options */
 #define FW_OPT_UEVENT	(1U << 0)
 #define FW_OPT_NOWAIT	(1U << 1)
@@ -145,13 +210,12 @@ struct firmware_cache {
 struct firmware_buf {
 	struct kref ref;
 	struct list_head list;
-	struct completion completion;
 	struct firmware_cache *fwc;
-	unsigned long status;
 	void *data;
 	size_t size;
 	size_t allocated_size;
 #ifdef CONFIG_FW_LOADER_USER_HELPER
+	struct fw_umh fw_umh;
 	bool is_paged_buf;
 	bool need_uevent;
 	struct page **pages;
@@ -205,8 +269,8 @@ static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
 	buf->fwc = fwc;
 	buf->data = dbuf;
 	buf->allocated_size = size;
-	init_completion(&buf->completion);
 #ifdef CONFIG_FW_LOADER_USER_HELPER
+	fw_umh_init(&buf->fw_umh);
 	INIT_LIST_HEAD(&buf->pending_list);
 #endif
 
@@ -309,8 +373,7 @@ static void fw_finish_direct_load(struct device *device,
 				  struct firmware_buf *buf)
 {
 	mutex_lock(&fw_lock);
-	set_bit(FW_STATUS_DONE, &buf->status);
-	complete_all(&buf->completion);
+	fw_umh_done(&buf->fw_umh);
 	mutex_unlock(&fw_lock);
 }
 
@@ -478,12 +541,11 @@ static void __fw_load_abort(struct firmware_buf *buf)
 	 * There is a small window in which user can write to 'loading'
 	 * between loading done and disappearance of 'loading'
 	 */
-	if (test_bit(FW_STATUS_DONE, &buf->status))
+	if (fw_umh_is_done(&buf->fw_umh))
 		return;
 
 	list_del_init(&buf->pending_list);
-	set_bit(FW_STATUS_ABORT, &buf->status);
-	complete_all(&buf->completion);
+	fw_umh_aborted(&buf->fw_umh);
 }
 
 static void fw_load_abort(struct firmware_priv *fw_priv)
@@ -496,9 +558,6 @@ static void fw_load_abort(struct firmware_priv *fw_priv)
 	fw_priv->buf = NULL;
 }
 
-#define is_fw_load_aborted(buf)	\
-	test_bit(FW_STATUS_ABORT, &(buf)->status)
-
 static LIST_HEAD(pending_fw_head);
 
 /* reboot notifier for avoid deadlock with usermode_lock */
@@ -598,7 +657,7 @@ static ssize_t firmware_loading_show(struct device *dev,
 
 	mutex_lock(&fw_lock);
 	if (fw_priv->buf)
-		loading = test_bit(FW_STATUS_LOADING, &fw_priv->buf->status);
+		loading = fw_umh_is_loading(&fw_priv->buf->fw_umh);
 	mutex_unlock(&fw_lock);
 
 	return sprintf(buf, "%d\n", loading);
@@ -653,23 +712,20 @@ static ssize_t firmware_loading_store(struct device *dev,
 	switch (loading) {
 	case 1:
 		/* discarding any previous partial load */
-		if (!test_bit(FW_STATUS_DONE, &fw_buf->status)) {
+		if (!fw_umh_is_done(&fw_buf->fw_umh)) {
 			for (i = 0; i < fw_buf->nr_pages; i++)
 				__free_page(fw_buf->pages[i]);
 			vfree(fw_buf->pages);
 			fw_buf->pages = NULL;
 			fw_buf->page_array_size = 0;
 			fw_buf->nr_pages = 0;
-			set_bit(FW_STATUS_LOADING, &fw_buf->status);
+			fw_umh_start(&fw_buf->fw_umh);
 		}
 		break;
 	case 0:
-		if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
+		if (fw_umh_is_loading(&fw_buf->fw_umh)) {
 			int rc;
 
-			set_bit(FW_STATUS_DONE, &fw_buf->status);
-			clear_bit(FW_STATUS_LOADING, &fw_buf->status);
-
 			/*
 			 * Several loading requests may be pending on
 			 * one same firmware buf, so let all requests
@@ -691,10 +747,11 @@ static ssize_t firmware_loading_store(struct device *dev,
 			 */
 			list_del_init(&fw_buf->pending_list);
 			if (rc) {
-				set_bit(FW_STATUS_ABORT, &fw_buf->status);
+				fw_umh_aborted(&fw_buf->fw_umh);
 				written = rc;
+			} else {
+				fw_umh_done(&fw_buf->fw_umh);
 			}
-			complete_all(&fw_buf->completion);
 			break;
 		}
 		/* fallthrough */
@@ -755,7 +812,7 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
 
 	mutex_lock(&fw_lock);
 	buf = fw_priv->buf;
-	if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) {
+	if (!buf || fw_umh_is_done(&buf->fw_umh)) {
 		ret_count = -ENODEV;
 		goto out;
 	}
@@ -842,7 +899,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 
 	mutex_lock(&fw_lock);
 	buf = fw_priv->buf;
-	if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) {
+	if (!buf || fw_umh_is_done(&buf->fw_umh)) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -955,8 +1012,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 		timeout = MAX_JIFFY_OFFSET;
 	}
 
-	retval = wait_for_completion_interruptible_timeout(&buf->completion,
-			timeout);
+	retval = fw_umh_wait_timeout(&buf->fw_umh, timeout);
 	if (retval == -ERESTARTSYS || !retval) {
 		mutex_lock(&fw_lock);
 		fw_load_abort(fw_priv);
@@ -965,7 +1021,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 		retval = 0;
 	}
 
-	if (is_fw_load_aborted(buf))
+	if (fw_umh_is_aborted(&buf->fw_umh))
 		retval = -EAGAIN;
 	else if (buf->is_paged_buf && !buf->data)
 		retval = -ENOMEM;
@@ -1015,29 +1071,25 @@ fw_load_from_user_helper(struct firmware *firmware, const char *name,
 	return -ENOENT;
 }
 
-/* No abort during direct loading */
-#define is_fw_load_aborted(buf) false
-
 #ifdef CONFIG_PM_SLEEP
 static inline void kill_requests_without_uevent(void) { }
 #endif
 
 #endif /* CONFIG_FW_LOADER_USER_HELPER */
 
-
 /* wait until the shared firmware_buf becomes ready (or error) */
 static int sync_cached_firmware_buf(struct firmware_buf *buf)
 {
 	int ret = 0;
 
 	mutex_lock(&fw_lock);
-	while (!test_bit(FW_STATUS_DONE, &buf->status)) {
-		if (is_fw_load_aborted(buf)) {
+	while (!fw_umh_is_done(&buf->fw_umh)) {
+		if (fw_umh_is_aborted(&buf->fw_umh)) {
 			ret = -ENOENT;
 			break;
 		}
 		mutex_unlock(&fw_lock);
-		ret = wait_for_completion_interruptible(&buf->completion);
+		ret = fw_umh_wait_timeout(&buf->fw_umh, 0);
 		mutex_lock(&fw_lock);
 	}
 	mutex_unlock(&fw_lock);
@@ -1095,7 +1147,7 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device,
 	struct firmware_buf *buf = fw->priv;
 
 	mutex_lock(&fw_lock);
-	if (!buf->size || is_fw_load_aborted(buf)) {
+	if (!buf->size || fw_umh_is_aborted(&buf->fw_umh)) {
 		mutex_unlock(&fw_lock);
 		return -ENOENT;
 	}
-- 
2.7.4

  parent reply	other threads:[~2016-09-09 12:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-09 12:12 [PATCH v5 0/5] " Daniel Wagner
2016-09-09 12:12 ` [PATCH v5 1/5] firmware: document user mode helper lock usage Daniel Wagner
2016-09-09 22:14   ` Luis R. Rodriguez
2016-09-22  2:36     ` Ming Lei
2016-10-05 20:41       ` Luis R. Rodriguez
     [not found]       ` <ab544ba0-2128-055f-3190-6a1a24e879e1@bmw-carit.de>
2016-10-05 20:46         ` Luis R. Rodriguez
     [not found]           ` <2ec51622-f727-e884-1a09-a595a31f4b21@bmw-carit.de>
2016-10-10 18:40             ` Luis R. Rodriguez
2016-09-09 12:12 ` Daniel Wagner [this message]
2016-09-09 22:19   ` [PATCH v5 2/5] firmware: encapsulate firmware loading status Luis R. Rodriguez
2016-09-09 22:24   ` Luis R. Rodriguez
2016-09-13  9:47   ` Daniel Wagner
2016-10-05 20:27     ` Luis R. Rodriguez
2016-10-07 11:41       ` Daniel Wagner
2016-10-10 20:37         ` Luis R. Rodriguez
2016-10-18 13:30           ` Daniel Wagner
2016-10-18 21:54             ` Luis R. Rodriguez
2016-10-19  8:05               ` Daniel Wagner
2016-09-09 12:12 ` [PATCH v5 3/5] firmware: rename fw_load_from_user_helper() and _request_firmware_load() Daniel Wagner
2016-09-09 22:17   ` Luis R. Rodriguez
2016-09-09 12:12 ` [PATCH v5 4/5] firmware: drop bit ops in favor of simple state machine Daniel Wagner
2016-09-09 22:30   ` Luis R. Rodriguez
2016-09-09 12:12 ` [PATCH v5 5/5] firmware: do not use fw_lock for fw_umh protection Daniel Wagner
2016-09-09 17:38 ` [PATCH v5 0/5] firmware: encapsulate firmware loading status 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=1473423144-21734-3-git-send-email-wagi@monom.org \
    --to=wagi@monom.org \
    --cc=arend.vanspriel@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel.vetter@intel.com \
    --cc=daniel.wagner@bmw-carit.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=rjw@sisk.pl \
    --cc=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=tiwai@suse.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®