Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1504729 > unrolled thread

[PATCH v6 0/6] firmware: encapsulate firmware loading status

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/6] firmware: encapsulate firmware loading status Daniel Wagner <wagi@monom.org> - 2016-10-20 12:00 +0200
    [PATCH v6 2/6] firmware: encapsulate firmware loading status Daniel Wagner <wagi@monom.org> - 2016-10-20 12:00 +0200
    [PATCH v6 3/6] firmware: rename fw_load_from_user_helper() and _request_firmware_load() Daniel Wagner <wagi@monom.org> - 2016-10-20 12:00 +0200
    [PATCH v6 6/6] firmware: move fw_state_is_{aborted|done}() into UHM section Daniel Wagner <wagi@monom.org> - 2016-10-20 12:00 +0200
    [PATCH v6 4/6] firmware: drop bit ops in favor of simple state machine Daniel Wagner <wagi@monom.org> - 2016-10-20 12:00 +0200

#1504729 — [PATCH v6 0/6] firmware: encapsulate firmware loading status

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

Hi,

Here is a updated version with the input from Luis.

The firmware user helper code tracks the current state of the loading
process via an member of struct firmware_buf and a completion. Let's
encapsulate this simple state machine into struct fw_status. The aim is
to increase readability and reduce the usage of the fw_lock.

I tested this series with fw_userhelper.sh and fw_filesystem.sh under
kvm and also let it run on real hardware. You can get the series as git tree:

https://git.kernel.org/cgit/linux/kernel/git/wagi/linux.git/log/?h=firmware_async-9

cheers,
daniel


This series depends on Luis' "firmware: add SmPL grammar to avoid issues"
series:

http://marc.info/?l=linux-kernel&m=147320896231418&w=2

changes since v5:
  - add enum name (enum fw_status)
  - renamed function prefix and structure fw_umh to fw_state (avoid to
    clash with enum fw_status)
  - add back patch 1 from v3
  - move unused functions for direct firmeware loading into UMH section

changes since v4:
  - replaced "firmware: Move umh locking code into fw_load_from_user_helper()"
    with "firmware: document user mode helper lock usage"
  - changed prefix fw_status_ to fw_umh_
  - fixed a couple of bux pointed out by Ming
  - changed type of fw_umh::status to u8 and updated commit message to
    point out that all states are exclusive

changes since v3:
  - added 'firmware: Move umh locking code into
    fw_load_from_user_helper()'
  - dropped loading_tiemout and firmware_loading_time() for
    !CONFG_FW_LOADER_USER_HELPER
  - rebased on Luis patches

changes since v2:
  - more splitting out
    - first patch factors out all the bit ops into fw_status
    - second patch gets rid of the bit ops
    - third get rid of fw_lock by using swait

changes since v1:
  - moved swait change into its own patch
  - added ifdef section for FW_LOADER_USER_HELPER_FALLBACK
  - updated commit message highlighting the mutex usage drop a bit

Daniel Wagner (6):
  firmware: move umh locking code into fw_load_from_user_helper()
  firmware: encapsulate firmware loading status
  firmware: rename fw_load_from_user_helper() and
    _request_firmware_load()
  firmware: drop bit ops in favor of simple state machine
  firmware: do not use fw_lock for fw_state protection
  firmware: move fw_state_is_{aborted|done}() into UHM section

 drivers/base/firmware_class.c | 226 ++++++++++++++++++++++++------------------
 1 file changed, 131 insertions(+), 95 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1504730 — [PATCH v6 2/6] firmware: encapsulate firmware loading status

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

The firmware loader tracks the current state of the loading
process via unsigned long status and a completion in struct
firmware_buf. Instead of open code tracking the state introduce data
structure which encapsulate the state tracking and synchronization.

While at it also separate UHM states from direct loading states, e.g.
the loading_timeout is only defined when CONFIG_FW_LOADER_USER_HELPER.

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

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d4fee06b67f3..b7a9fa1c5197 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -91,12 +91,73 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
 }
 #endif
 
-enum {
+enum fw_status {
+	FW_STATUS_UNKNOWN,
 	FW_STATUS_LOADING,
 	FW_STATUS_DONE,
-	FW_STATUS_ABORT,
+	FW_STATUS_ABORTED,
 };
 
+/*
+ * Synchronize request_firmware() calls and defered loading of
+ * firmwares via user mode helper. It is also necassary for direct
+ * loading, that is if there are concurrent request for the same
+ * firmwareq.
+ */
+struct fw_state {
+	struct completion completion;
+	unsigned long status;
+};
+
+static void fw_state_init(struct fw_state *fw_st)
+{
+	init_completion(&fw_st->completion);
+	fw_st->status = FW_STATUS_UNKNOWN;
+}
+
+static int __fw_state_check(struct fw_state *fw_st, enum fw_status status)
+{
+	return test_bit(status, &fw_st->status);
+}
+
+static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
+{
+	int ret;
+
+	ret = wait_for_completion_interruptible_timeout(&fw_st->completion,
+							timeout);
+	if (ret != 0 && test_bit(FW_STATUS_ABORTED, &fw_st->status))
+		return -ENOENT;
+
+	return ret;
+}
+
+static void __fw_state_set(struct fw_state *fw_st,
+			 enum fw_status status)
+{
+	set_bit(status, &fw_st->status);
+
+	if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED) {
+		clear_bit(FW_STATUS_LOADING, &fw_st->status);
+		complete_all(&fw_st->completion);
+	}
+}
+
+#define fw_state_start(fw_st)					\
+	__fw_state_set(fw_st, FW_STATUS_LOADING)
+#define fw_state_done(fw_st)					\
+	__fw_state_set(fw_st, FW_STATUS_DONE)
+#define fw_state_is_done(fw_st)					\
+	__fw_state_check(fw_st, FW_STATUS_DONE)
+#define fw_state_wait(fw_st)					\
+	__fw_state_wait_common(fw_st, MAX_SCHEDULE_TIMEOUT)
+
+#ifndef CONFIG_FW_LOADER_USER_HELPER
+
+#define fw_state_is_aborted(fw_st)	false
+
+#else /* CONFIG_FW_LOADER_USER_HELPER */
+
 static int loading_timeout = 60;	/* In seconds */
 
 static inline long firmware_loading_timeout(void)
@@ -104,6 +165,17 @@ static inline long firmware_loading_timeout(void)
 	return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
 }
 
+#define fw_state_aborted(fw_st)					\
+	__fw_state_set(fw_st, FW_STATUS_ABORTED)
+#define fw_state_is_loading(fw_st)				\
+	__fw_state_check(fw_st, FW_STATUS_LOADING)
+#define fw_state_is_aborted(fw_st)				\
+	__fw_state_check(fw_st, FW_STATUS_ABORTED)
+#define fw_state_wait_timeout(fw_st, timeout)			\
+	__fw_state_wait_common(fw_st, timeout)
+
+#endif /* CONFIG_FW_LOADER_USER_HELPER */
+
 /* firmware behavior options */
 #define FW_OPT_UEVENT	(1U << 0)
 #define FW_OPT_NOWAIT	(1U << 1)
@@ -145,9 +217,8 @@ struct firmware_cache {
 struct firmware_buf {
 	struct kref ref;
 	struct list_head list;
-	struct completion completion;
 	struct firmware_cache *fwc;
-	unsigned long status;
+	struct fw_state fw_st;
 	void *data;
 	size_t size;
 	size_t allocated_size;
@@ -205,7 +276,7 @@ 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);
+	fw_state_init(&buf->fw_st);
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 	INIT_LIST_HEAD(&buf->pending_list);
 #endif
@@ -309,8 +380,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_state_done(&buf->fw_st);
 	mutex_unlock(&fw_lock);
 }
 
@@ -478,12 +548,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_state_is_done(&buf->fw_st))
 		return;
 
 	list_del_init(&buf->pending_list);
-	set_bit(FW_STATUS_ABORT, &buf->status);
-	complete_all(&buf->completion);
+	fw_state_aborted(&buf->fw_st);
 }
 
 static void fw_load_abort(struct firmware_priv *fw_priv)
@@ -496,9 +565,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 +664,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_state_is_loading(&fw_priv->buf->fw_st);
 	mutex_unlock(&fw_lock);
 
 	return sprintf(buf, "%d\n", loading);
@@ -653,23 +719,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_state_is_done(&fw_buf->fw_st)) {
 			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_state_start(&fw_buf->fw_st);
 		}
 		break;
 	case 0:
-		if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
+		if (fw_state_is_loading(&fw_buf->fw_st)) {
 			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 +754,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_state_aborted(&fw_buf->fw_st);
 				written = rc;
+			} else {
+				fw_state_done(&fw_buf->fw_st);
 			}
-			complete_all(&fw_buf->completion);
 			break;
 		}
 		/* fallthrough */
@@ -755,7 +819,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_state_is_done(&buf->fw_st)) {
 		ret_count = -ENODEV;
 		goto out;
 	}
@@ -842,7 +906,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_state_is_done(&buf->fw_st)) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -955,8 +1019,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_state_wait_timeout(&buf->fw_st, timeout);
 	if (retval == -ERESTARTSYS || !retval) {
 		mutex_lock(&fw_lock);
 		fw_load_abort(fw_priv);
@@ -965,7 +1028,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 		retval = 0;
 	}
 
-	if (is_fw_load_aborted(buf))
+	if (fw_state_is_aborted(&buf->fw_st))
 		retval = -EAGAIN;
 	else if (buf->is_paged_buf && !buf->data)
 		retval = -ENOMEM;
@@ -1040,29 +1103,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_state_is_done(&buf->fw_st)) {
+		if (fw_state_is_aborted(&buf->fw_st)) {
 			ret = -ENOENT;
 			break;
 		}
 		mutex_unlock(&fw_lock);
-		ret = wait_for_completion_interruptible(&buf->completion);
+		ret = fw_state_wait(&buf->fw_st);
 		mutex_lock(&fw_lock);
 	}
 	mutex_unlock(&fw_lock);
@@ -1120,7 +1179,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_state_is_aborted(&buf->fw_st)) {
 		mutex_unlock(&fw_lock);
 		return -ENOENT;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1504731 — [PATCH v6 3/6] firmware: rename fw_load_from_user_helper() and _request_firmware_load()

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

fw_load_from_user_helper() and _request_firmware_load() are used when
CONFIG_FW_LOADER_USER_HELPER is enabled. In order to clearly mark which
part of the code is depending on UMH we stream line these functions to
match with the rest of the code.

Suggested by Luis.

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

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index b7a9fa1c5197..a6916a7aaeb2 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -986,9 +986,8 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 	return fw_priv;
 }
 
-/* load a firmware via user helper */
-static int _request_firmware_load(struct firmware_priv *fw_priv,
-				  unsigned int opt_flags, long timeout)
+static int _request_firmware_umh(struct firmware_priv *fw_priv,
+				 unsigned int opt_flags, long timeout)
 {
 	int retval = 0;
 	struct device *f_dev = &fw_priv->dev;
@@ -1039,9 +1038,9 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 	return retval;
 }
 
-static int fw_load_from_user_helper(struct firmware *firmware,
-				    const char *name, struct device *device,
-				    unsigned int opt_flags, long timeout)
+static int fw_get_umh_firmware(struct firmware *firmware, const char *name,
+			struct device *device, unsigned int opt_flags,
+			long timeout)
 {
 	struct firmware_priv *fw_priv;
 	int ret;
@@ -1070,7 +1069,7 @@ static int fw_load_from_user_helper(struct firmware *firmware,
 	}
 
 	fw_priv->buf = firmware->priv;
-	ret = _request_firmware_load(fw_priv, opt_flags, timeout);
+	ret = _request_firmware_umh(fw_priv, opt_flags, timeout);
 
 release_lock:
 	usermodehelper_read_unlock();
@@ -1096,9 +1095,9 @@ static void kill_requests_without_uevent(void)
 
 #else /* CONFIG_FW_LOADER_USER_HELPER */
 static inline int
-fw_load_from_user_helper(struct firmware *firmware, const char *name,
-			 struct device *device, unsigned int opt_flags,
-			 long timeout)
+fw_get_umh_firmware(struct firmware *firmware, const char *name,
+		    struct device *device, unsigned int opt_flags,
+		    long timeout)
 {
 	return -ENOENT;
 }
@@ -1242,8 +1241,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 				 name, ret);
 		if (opt_flags & FW_OPT_USERHELPER) {
 			dev_warn(device, "Falling back to user helper\n");
-			ret = fw_load_from_user_helper(fw, name, device,
-						       opt_flags, timeout);
+			ret = fw_get_umh_firmware(fw, name, device,
+						  opt_flags, timeout);
 		}
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1504732 — [PATCH v6 6/6] firmware: move fw_state_is_{aborted|done}() into UHM section

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

For direct firmware loading we do not need to read the status after
fw_state_wait() returns. It either returns success or the error
code. Also assign_firmware_buf() is only called if there was no
error (especially ABORTED has been detected by
fw_get_umh_firmware()).

So we are allowed to move the remaining fw_state_is_{aborted|done}()
defintion into the !CONFIG_FW_LOADER_USER_HELPER section.

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

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 4f1755537314..813587f2ccbe 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -116,11 +116,6 @@ static void fw_state_init(struct fw_state *fw_st)
 	fw_st->status = FW_STATUS_UNKNOWN;
 }
 
-static int __fw_state_check(struct fw_state *fw_st, enum fw_status status)
-{
-	return fw_st->status == status;
-}
-
 static inline bool __fw_state_is_done(enum fw_status status)
 {
 	return status == FW_STATUS_DONE || status == FW_STATUS_ABORTED;
@@ -152,16 +147,10 @@ static void __fw_state_set(struct fw_state *fw_st,
 	__fw_state_set(fw_st, FW_STATUS_LOADING)
 #define fw_state_done(fw_st)					\
 	__fw_state_set(fw_st, FW_STATUS_DONE)
-#define fw_state_is_done(fw_st)					\
-	__fw_state_check(fw_st, FW_STATUS_DONE)
 #define fw_state_wait(fw_st)					\
 	__fw_state_wait_common(fw_st, MAX_SCHEDULE_TIMEOUT)
 
-#ifndef CONFIG_FW_LOADER_USER_HELPER
-
-#define fw_state_is_aborted(fw_st)	false
-
-#else /* CONFIG_FW_LOADER_USER_HELPER */
+#ifdef CONFIG_FW_LOADER_USER_HELPER
 
 static int loading_timeout = 60;	/* In seconds */
 
@@ -170,8 +159,15 @@ static inline long firmware_loading_timeout(void)
 	return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
 }
 
+static int __fw_state_check(struct fw_state *fw_st, enum fw_status status)
+{
+	return fw_st->status == status;
+}
+
 #define fw_state_aborted(fw_st)					\
 	__fw_state_set(fw_st, FW_STATUS_ABORTED)
+#define fw_state_is_done(fw_st)					\
+	__fw_state_check(fw_st, FW_STATUS_DONE)
 #define fw_state_is_loading(fw_st)				\
 	__fw_state_check(fw_st, FW_STATUS_LOADING)
 #define fw_state_is_aborted(fw_st)				\
@@ -1156,7 +1152,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 || fw_state_is_aborted(&buf->fw_st)) {
+	if (!buf->size) {
 		mutex_unlock(&fw_lock);
 		return -ENOENT;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1504733 — [PATCH v6 4/6] firmware: drop bit ops in favor of simple state machine

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

We track the state of the firmware loading with bit ops.  Since the
state machine has only a few states and they are all mutual exclusive
there are only a few simple state transition we can model this simplify.

	   UNKNOWN -> LOADING -> DONE | ABORTED

Because we don't use any bit ops on fw_state::status anymore we are able
to change the data type to enum fw_status and update the function
arguments accordingly.

READ_ONCE() and WRITE_ONCE() are propably not needed because there are a
lot of load and stores around fw_st->status. But let's make it explicit
and not be sorry later.

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

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index a6916a7aaeb2..a1dab6a792f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -106,7 +106,7 @@ enum fw_status {
  */
 struct fw_state {
 	struct completion completion;
-	unsigned long status;
+	enum fw_status status;
 };
 
 static void fw_state_init(struct fw_state *fw_st)
@@ -117,7 +117,7 @@ static void fw_state_init(struct fw_state *fw_st)
 
 static int __fw_state_check(struct fw_state *fw_st, enum fw_status status)
 {
-	return test_bit(status, &fw_st->status);
+	return fw_st->status == status;
 }
 
 static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
@@ -126,7 +126,7 @@ static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
 
 	ret = wait_for_completion_interruptible_timeout(&fw_st->completion,
 							timeout);
-	if (ret != 0 && test_bit(FW_STATUS_ABORTED, &fw_st->status))
+	if (ret != 0 && READ_ONCE(fw_st->status) == FW_STATUS_ABORTED)
 		return -ENOENT;
 
 	return ret;
@@ -135,12 +135,10 @@ static int __fw_state_wait_common(struct fw_state *fw_st, long timeout)
 static void __fw_state_set(struct fw_state *fw_st,
 			 enum fw_status status)
 {
-	set_bit(status, &fw_st->status);
+	WRITE_ONCE(fw_st->status, status);
 
-	if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED) {
-		clear_bit(FW_STATUS_LOADING, &fw_st->status);
+	if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
 		complete_all(&fw_st->completion);
-	}
 }
 
 #define fw_state_start(fw_st)					\
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web