greybus: define -EILSEQ to mean implementation error
Reserve operation result code -EILSEQ to represent that the code
that implements an operation is broken. This is used (initially)
for any attempt to set the result to -EBADR (which is reserved for
an operation in initial state), or for an attempt to set the result
of an operation that is *not* in initial state to -EINPROGRESS.
Note that we still use -EIO gb_operation_status_map() to represent a
gb_operation_result value that isn't recognized.
In gb_operation_result(), warn if operation->errno is -EBADR. That
is another value that indicates the operation is not in a state
where it's valid to query an operation's result.
Update a bunch of comments above gb_operation_result_set() to
explain constraints on operation->errno.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 8a023cb..905c6de 100644
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -70,44 +70,70 @@
static DEFINE_SPINLOCK(gb_operations_lock);
/*
- * Set an operation's result. Initially an outgoing operation's
- * errno value is -EBADR. If no error occurs before sending the
- * request message the only valid value operation->errno can be
- * set to is -EINPROGRESS, indicating the request has been (or
- * rather is about to be) sent. At that point nobody should
- * be looking at the result until the reponse arrives.
+ * Set an operation's result.
+ *
+ * Initially an outgoing operation's errno value is -EBADR.
+ * If no error occurs before sending the request message the only
+ * valid value operation->errno can be set to is -EINPROGRESS,
+ * indicating the request has been (or rather is about to be) sent.
+ * At that point nobody should be looking at the result until the
+ * reponse arrives.
*
* The first time the result gets set after the request has been
* sent, that result "sticks." That is, if two concurrent threads
* race to set the result, the first one wins. The return value
* tells the caller whether its result was recorded; if not the
- * has nothing more to do.
+ * caller has nothing more to do.
+ *
+ * The result value -EILSEQ is reserved to signal an implementation
+ * error; if it's ever observed, the code performing the request has
+ * done something fundamentally wrong. It is an error to try to set
+ * the result to -EBADR, and attempts to do so result in a warning,
+ * and -EILSEQ is used instead. Similarly, the only valid result
+ * value to set for an operation in initial state is -EINPROGRESS.
+ * Attempts to do otherwise will also record a (successful) -EILSEQ
+ * operation result.
*/
static bool gb_operation_result_set(struct gb_operation *operation, int result)
{
int prev;
- /* Nobody should be setting -EBADR */
- if (WARN_ON(result == -EBADR))
- return false;
-
- /* Are we sending the request message? */
if (result == -EINPROGRESS) {
- /* Yes, but verify the result has not already been set */
+ /*
+ * -EINPROGRESS is used to indicate the request is
+ * in flight. It should be the first result value
+ * set after the initial -EBADR. Issue a warning
+ * and record an implementation error if it's
+ * set at any other time.
+ */
spin_lock_irq(&gb_operations_lock);
prev = operation->errno;
if (prev == -EBADR)
operation->errno = result;
+ else
+ operation->errno = -EILSEQ;
spin_unlock_irq(&gb_operations_lock);
+ WARN_ON(prev != -EBADR);
- return !WARN_ON(prev != -EBADR);
+ return true;
}
- /* Trying to set final status; only the first one succeeds */
+ /*
+ * The first result value set after a request has been sent
+ * will be the final result of the operation. Subsequent
+ * attempts to set the result are ignored.
+ *
+ * Note that -EBADR is a reserved "initial state" result
+ * value. Attempts to set this value result in a warning,
+ * and the result code is set to -EILSEQ instead.
+ */
+ if (WARN_ON(result == -EBADR))
+ result = -EILSEQ; /* Nobody should be setting -EBADR */
+
spin_lock_irq(&gb_operations_lock);
prev = operation->errno;
if (prev == -EINPROGRESS)
- operation->errno = result;
+ operation->errno = result; /* First and final result */
spin_unlock_irq(&gb_operations_lock);
return prev == -EINPROGRESS;
@@ -117,6 +143,7 @@
{
int result = operation->errno;
+ WARN_ON(result == -EBADR);
WARN_ON(result == -EINPROGRESS);
return result;