diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index 6197167a67d8..046ed2a99f45 100644 --- a/drivers/staging/greybus/operation.c +++ b/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); } /* diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index a79e88a3b314..a173aa9feb17 100644 --- a/drivers/staging/greybus/operation.h +++ b/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;