greybus: record type in operation structure

I've gone back and forth on this, but now that I'm looking at
asynchronous operations I know that the asynchronous callback will
want to know what type of operation it is handling, and right now
that's only available in the message header.

So record an operation's type in the operation structure, and use
it in a few spots where the header type was being used previously.
Pass the type to gb_operation_create_incoming() so it can fill
it in after the operation has been created.

Clean up the crap comments above the definition of the operation
structure.

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/staging/greybus/operation.c

@@ -13,6 +13,7 @@
 
 #include "greybus.h"
 
+/* The default amount of time a request is given to complete */
 #define OPERATION_TIMEOUT_DEFAULT	1000	/* milliseconds */
 
 /*
@@ -59,7 +60,10 @@ struct gb_operation_msg_hdr {
 	/* 2 bytes pad, must be zero (ignore when read) */
 } __aligned(sizeof(u64));
 
-/* XXX Could be per-host device, per-module, or even per-connection */
+/*
+ * Protects access to connection operations lists, as well as
+ * updates to operation->errno.
+ */
 static DEFINE_SPINLOCK(gb_operations_lock);
 
 /*
@@ -201,21 +205,18 @@ static void gb_message_cancel(struct gb_message *message)
 static void gb_operation_request_handle(struct gb_operation *operation)
 {
 	struct gb_protocol *protocol = operation->connection->protocol;
-	struct gb_operation_msg_hdr *header;
-
-	header = operation->request->header;
 
 	/*
 	 * If the protocol has no incoming request handler, report
 	 * an error and mark the request bad.
 	 */
 	if (protocol->request_recv) {
-		protocol->request_recv(header->type, operation);
+		protocol->request_recv(operation->type, operation);
 		return;
 	}
 
 	gb_connection_err(operation->connection,
-		"unexpected incoming request type 0x%02hhx\n", header->type);
+		"unexpected incoming request type 0x%02hhx\n", operation->type);
 	if (gb_operation_result_set(operation, -EPROTONOSUPPORT))
 		queue_work(gb_operation_workqueue, &operation->work);
 	else
@@ -444,8 +445,7 @@ bool gb_operation_response_alloc(struct gb_operation *operation,
 	struct gb_message *response;
 	u8 type;
 
-	request_header = operation->request->header;
-	type = request_header->type | GB_OPERATION_TYPE_RESPONSE;
+	type = operation->type | GB_OPERATION_TYPE_RESPONSE;
 	response = gb_operation_message_alloc(hd, type, response_size,
 						GFP_KERNEL);
 	if (!response)
@@ -458,6 +458,7 @@ bool gb_operation_response_alloc(struct gb_operation *operation,
 	 * that's left is the operation id, which we copy from the
 	 * request message header (as-is, in little-endian order).
 	 */
+	request_header = operation->request->header;
 	response->header->operation_id = request_header->operation_id;
 	operation->response = response;
 
@@ -515,9 +516,11 @@ gb_operation_create_common(struct gb_connection *connection, u8 type,
 	operation->request->operation = operation;
 
 	/* Allocate the response buffer for outgoing operations */
-	if (type != GB_OPERATION_TYPE_INVALID)
+	if (type != GB_OPERATION_TYPE_INVALID) {
 		if (!gb_operation_response_alloc(operation, response_size))
 			goto err_request;
+		operation->type = type;
+	}
 	operation->errno = -EBADR;  /* Initial value--means "never set" */
 
 	INIT_WORK(&operation->work, gb_operation_work);
@@ -563,7 +566,7 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection,
 
 static struct gb_operation *
 gb_operation_create_incoming(struct gb_connection *connection, u16 id,
-					void *data, size_t request_size)
+				u8 type, void *data, size_t request_size)
 {
 	struct gb_operation *operation;
 
@@ -572,6 +575,7 @@ gb_operation_create_incoming(struct gb_connection *connection, u16 id,
 					request_size, 0);
 	if (operation) {
 		operation->id = id;
+		operation->type = type;
 		memcpy(operation->request->header, data, request_size);
 	}
 
@@ -779,13 +783,13 @@ EXPORT_SYMBOL_GPL(greybus_data_sent);
  * data into the request buffer and handle the rest via workqueue.
  */
 static void gb_connection_recv_request(struct gb_connection *connection,
-				       u16 operation_id, void *data,
-				       size_t size)
+				       u16 operation_id, u8 type,
+				       void *data, size_t size)
 {
 	struct gb_operation *operation;
 
 	operation = gb_operation_create_incoming(connection, operation_id,
-						data, size);
+						type, data, size);
 	if (!operation) {
 		gb_connection_err(connection, "can't create operation");
 		return;		/* XXX Respond with pre-allocated ENOMEM */
@@ -884,7 +888,7 @@ void gb_connection_recv(struct gb_connection *connection,
 						header->result, data, msg_size);
 	else
 		gb_connection_recv_request(connection, operation_id,
-						data, msg_size);
+						header->type, data, msg_size);
 }
 
 /*

drivers/staging/greybus/operation.h

@@ -56,39 +56,31 @@ struct gb_message {
 
 /*
  * A Greybus operation is a remote procedure call performed over a
- * connection between the AP and a function on Greybus module.
+ * connection between two UniPro interfaces.
+ *
  * Every operation consists of a request message sent to the other
- * end of the connection coupled with a reply returned to the
- * sender.
+ * end of the connection coupled with a reply message returned to
+ * the sender.  Every operation has a type, whose interpretation is
+ * dependent on the protocol associated with the connection.
  *
- * The state for managing active requests on a connection is held in
- * the connection structure.
+ * Only four things in an operation structure are intended to be
+ * directly usable by protocol handlers:  the operation's connection
+ * pointer; the operation type; the request message payload (and
+ * size); and the response message payload (and size).  Note that a
+ * message with a 0-byte payload has a null message payload pointer.
  *
- * YADA YADA
- *
- * submitting each request and providing its matching response to
- * the caller when it arrives.  Operations normally complete
- * asynchronously, and when an operation's response arrives its
- * callback function is executed.  The callback pointer is supplied
- * at the time the operation is submitted; a null callback pointer
- * causes synchronous operation--the caller is blocked until
- * the response arrives.  In addition, it is possible to await
- * the completion of a submitted asynchronous operation.
- *
- * A Greybus device operation includes a Greybus buffer to hold the
- * data sent to the device.  The only field within a Greybus
- * operation that should be used by a caller is the payload pointer,
- * which should be used to populate the request data.  This pointer
- * is guaranteed to be 64-bit aligned.
- * XXX and callback?
+ * In addition, every operation has a result, which is an errno
+ * value.  Protocol handlers access the operation result using
+ * gb_operation_result().
  */
 typedef void (*gb_operation_callback)(struct gb_operation *);
 struct gb_operation {
 	struct gb_connection	*connection;
 	struct gb_message	*request;
 	struct gb_message	*response;
-	u16			id;
+	u8			type;
 
+	u16			id;
 	int			errno;		/* Operation result */
 
 	struct work_struct	work;