greybus: Merge branch 'master' into vibrator-gb

drivers/staging/greybus/connection.c

@@ -40,7 +40,7 @@ void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id,
 			"nonexistent connection (%zu bytes dropped)\n", length);
 		return;
 	}
-	gb_connection_operation_recv(connection, data, length);
+	gb_connection_recv(connection, data, length);
 }
 EXPORT_SYMBOL_GPL(greybus_cport_in);
 

drivers/staging/greybus/core.c

@@ -169,11 +169,9 @@ struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver
 	 * Validate that the driver implements all of the callbacks
 	 * so that we don't have to every time we make them.
 	 */
-	if ((!driver->alloc_gbuf_data) ||
-	    (!driver->free_gbuf_data) ||
-	    (!driver->submit_svc) ||
-	    (!driver->submit_gbuf) ||
-	    (!driver->kill_gbuf)) {
+	if ((!driver->buffer_alloc) || (!driver->buffer_free) ||
+	    (!driver->buffer_send) || (!driver->buffer_cancel) ||
+	    (!driver->submit_svc)) {
 		pr_err("Must implement all greybus_host_driver callbacks!\n");
 		return NULL;
 	}

drivers/staging/greybus/es1-ap-usb.c

@@ -20,7 +20,6 @@
 #define ES1_SVC_MSG_SIZE	(sizeof(struct svc_msg) + SZ_64K)
 #define ES1_GBUF_MSG_SIZE	PAGE_SIZE
 
-
 static const struct usb_device_id id_table[] = {
 	/* Made up numbers for the SVC USB Bridge in ES1 */
 	{ USB_DEVICE(0xffff, 0x0001) },
@@ -86,70 +85,47 @@ static inline struct es1_ap_dev *hd_to_es1(struct greybus_host_device *hd)
 static void cport_out_callback(struct urb *urb);
 
 /*
- * Allocate the actual buffer for this gbuf and device and cport
- *
- * We are responsible for setting the following fields in a struct gbuf:
- *	void *hcpriv;
- *	void *transfer_buffer;
- *	u32 transfer_buffer_length;
+ * Allocate a buffer to be sent via UniPro.
  */
-static int alloc_gbuf_data(struct gbuf *gbuf, unsigned int size,
-				gfp_t gfp_mask)
+static void *buffer_alloc(unsigned int size, gfp_t gfp_mask)
 {
-	u32 cport_reserve = gbuf->dest_cport_id == CPORT_ID_BAD ? 0 : 1;
 	u8 *buffer;
 
-	if (gbuf->transfer_buffer)
-		return -EALREADY;
-
 	if (size > ES1_GBUF_MSG_SIZE) {
 		pr_err("guf was asked to be bigger than %ld!\n",
 		       ES1_GBUF_MSG_SIZE);
 	}
 
-	/* For ES2 we need to figure out what cport is going to what endpoint,
-	 * but for ES1, it's so dirt simple, we don't have a choice...
+	/*
+	 * For ES1 we need to insert a byte at the front of the data
+	 * to indicate the destination CPort id.  We only need one
+	 * extra byte, but we allocate four extra bytes to allow the
+	 * buffer returned to be aligned on a four-byte boundary.
 	 *
-	 * Also, do a "slow" allocation now, if we need speed, use a cache
+	 * This is only needed for outbound data, but we handle
+	 * buffers for inbound data the same way for consistency.
 	 *
-	 * For ES1 outbound buffers need to insert their target
-	 * CPort Id before the data; set aside an extra byte for
-	 * that purpose in that case.
+	 * XXX Do we need to indicate the destination device id too?
 	 */
-	buffer = kzalloc(cport_reserve + size, gfp_mask);
-	if (!buffer)
-		return -ENOMEM;
+	buffer = kzalloc(GB_BUFFER_ALIGN + size, gfp_mask);
+	if (buffer)
+		buffer += GB_BUFFER_ALIGN;
 
-	/* Insert the cport id for outbound buffers */
-	if (cport_reserve) {
-		if (gbuf->dest_cport_id > (u16)U8_MAX) {
-			pr_err("gbuf->dest_cport_id (%hd) is out of range!\n",
-				gbuf->dest_cport_id);
-			kfree(buffer);
-			return -EINVAL;
-		}
-		*buffer++ = gbuf->dest_cport_id;
-	}
-	gbuf->transfer_buffer = buffer;
-	gbuf->transfer_buffer_length = size;
-
-	return 0;
+	return buffer;
 }
 
-/* Free the memory we allocated with a gbuf */
-static void free_gbuf_data(struct gbuf *gbuf)
+/* Free a previously-allocated buffer */
+static void buffer_free(void *buffer)
 {
-	u8 *transfer_buffer = gbuf->transfer_buffer;
+	u8 *allocated = buffer;
 
-	/* Can be called with a NULL transfer_buffer on some error paths */
-	if (!transfer_buffer)
+	/* Can be called with a NULL buffer on some error paths */
+	if (!allocated)
 		return;
 
-	/* Account for the cport id in outbound buffers */
-	if (gbuf->dest_cport_id != CPORT_ID_BAD)
-		transfer_buffer--;	/* Back up to cport id */
-	kfree(transfer_buffer);
-	gbuf->transfer_buffer = NULL;
+	/* Account for the space set aside for the prepended cport id */
+	allocated -= GB_BUFFER_ALIGN;
+	kfree(allocated);
 }
 
 #define ES1_TIMEOUT	500	/* 500 ms for the SVC to do something */
@@ -207,53 +183,86 @@ static struct urb *next_free_urb(struct es1_ap_dev *es1, gfp_t gfp_mask)
 	return urb;
 }
 
-static int submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask)
+/*
+ * Returns an opaque cookie value if successful, or a pointer coded
+ * error otherwise.  If the caller wishes to cancel the in-flight
+ * buffer, it must supply the returned cookie to the cancel routine.
+ */
+static void *buffer_send(struct greybus_host_device *hd, u16 dest_cport_id,
+			void *buffer, size_t buffer_size, gfp_t gfp_mask)
 {
-	struct greybus_host_device *hd = gbuf->hd;
 	struct es1_ap_dev *es1 = hd_to_es1(hd);
 	struct usb_device *udev = es1->usb_dev;
+	u8 *transfer_buffer = buffer;
+	int transfer_buffer_size;
 	int retval;
-	u8 *transfer_buffer;
-	u8 *buffer;
 	struct urb *urb;
 
-	transfer_buffer = gbuf->transfer_buffer;
-	if (!transfer_buffer)
-		return -EINVAL;
-	buffer = &transfer_buffer[-1];	/* yes, we mean -1 */
+	if (!buffer) {
+		pr_err("null buffer supplied to send\n");
+		return ERR_PTR(-EINVAL);
+	}
+	if (buffer_size > (size_t)INT_MAX) {
+		pr_err("bad buffer size (%zu) supplied to send\n", buffer_size);
+		return ERR_PTR(-EINVAL);
+	}
+	transfer_buffer--;
+	transfer_buffer_size = buffer_size + 1;
+
+	/*
+	 * The data actually transferred will include an indication
+	 * of where the data should be sent.  Do one last check of
+	 * the target CPort id before filling it in.
+	 */
+	if (dest_cport_id == CPORT_ID_BAD) {
+		pr_err("request to send inbound data buffer\n");
+		return ERR_PTR(-EINVAL);
+	}
+	if (dest_cport_id > (u16)U8_MAX) {
+		pr_err("dest_cport_id (%hd) is out of range for ES1\n",
+			dest_cport_id);
+		return ERR_PTR(-EINVAL);
+	}
+	/* OK, the destination is fine; record it in the transfer buffer */
+	*transfer_buffer = dest_cport_id;
 
 	/* Find a free urb */
 	urb = next_free_urb(es1, gfp_mask);
 	if (!urb)
-		return -ENOMEM;
-
-	gbuf->hcd_data = urb;
+		return ERR_PTR(-ENOMEM);
 
 	usb_fill_bulk_urb(urb, udev,
 			  usb_sndbulkpipe(udev, es1->cport_out_endpoint),
-			  buffer, gbuf->transfer_buffer_length + 1,
-			  cport_out_callback, gbuf);
+			  transfer_buffer, transfer_buffer_size,
+			  cport_out_callback, hd);
 	retval = usb_submit_urb(urb, gfp_mask);
-	return retval;
+	if (retval) {
+		pr_err("error %d submitting URB\n", retval);
+		return ERR_PTR(retval);
+	}
+
+	return urb;
 }
 
-static void kill_gbuf(struct gbuf *gbuf)
+static void buffer_cancel(void *cookie)
 {
-	struct urb *urb = gbuf->hcd_data;
-
-	if (!urb)
-		return;
+	struct urb *urb = cookie;
 
+	/*
+	 * We really should be defensive and track all outstanding
+	 * (sent) buffers rather than trusting the cookie provided
+	 * is valid.  For the time being, this will do.
+	 */
 	usb_kill_urb(urb);
 }
 
 static struct greybus_host_driver es1_driver = {
 	.hd_priv_size		= sizeof(struct es1_ap_dev),
-	.alloc_gbuf_data	= alloc_gbuf_data,
-	.free_gbuf_data		= free_gbuf_data,
+	.buffer_alloc		= buffer_alloc,
+	.buffer_free		= buffer_free,
+	.buffer_send		= buffer_send,
+	.buffer_cancel		= buffer_cancel,
 	.submit_svc		= submit_svc,
-	.submit_gbuf		= submit_gbuf,
-	.kill_gbuf		= kill_gbuf,
 };
 
 /* Common function to report consistent warnings based on URB status */
@@ -329,7 +338,8 @@ static void ap_disconnect(struct usb_interface *interface)
 /* Callback for when we get a SVC message */
 static void svc_in_callback(struct urb *urb)
 {
-	struct es1_ap_dev *es1 = urb->context;
+	struct greybus_host_device *hd = urb->context;
+	struct es1_ap_dev *es1 = hd_to_es1(hd);
 	struct device *dev = &urb->dev->dev;
 	int status = check_urb_status(urb);
 	int retval;
@@ -355,8 +365,9 @@ static void svc_in_callback(struct urb *urb)
 
 static void cport_in_callback(struct urb *urb)
 {
+	struct greybus_host_device *hd = urb->context;
+	struct es1_ap_dev *es1 = hd_to_es1(hd);
 	struct device *dev = &urb->dev->dev;
-	struct es1_ap_dev *es1 = urb->context;
 	int status = check_urb_status(urb);
 	int retval;
 	u8 cport;
@@ -396,15 +407,12 @@ static void cport_in_callback(struct urb *urb)
 
 static void cport_out_callback(struct urb *urb)
 {
-	struct gbuf *gbuf = urb->context;
-	struct es1_ap_dev *es1 = hd_to_es1(gbuf->hd);
+	struct greybus_host_device *hd = urb->context;
+	struct es1_ap_dev *es1 = hd_to_es1(hd);
 	unsigned long flags;
+	/* int status = check_urb_status(urb); */
 	int i;
 
-	/* Record whether the transfer was successful */
-	gbuf->status = check_urb_status(urb);
-	gbuf->hcd_data = NULL;
-
 	/*
 	 * See if this was an urb in our pool, if so mark it "free", otherwise
 	 * we need to free it ourselves.
@@ -423,6 +431,8 @@ static void cport_out_callback(struct urb *urb)
 	usb_free_urb(urb);
 
 	/*
+	 * Rest assured Greg, this craziness is getting fixed.
+	 *
 	 * Yes, you are right, we aren't telling anyone that the urb finished.
 	 * "That's crazy!  How does this all even work?" you might be saying.
 	 * The "magic" is the idea that greybus works on the "operation" level,
@@ -529,7 +539,7 @@ static int ap_probe(struct usb_interface *interface,
 	usb_fill_int_urb(es1->svc_urb, udev,
 			 usb_rcvintpipe(udev, es1->svc_endpoint),
 			 es1->svc_buffer, ES1_SVC_MSG_SIZE, svc_in_callback,
-			 es1, svc_interval);
+			 hd, svc_interval);
 	retval = usb_submit_urb(es1->svc_urb, GFP_KERNEL);
 	if (retval)
 		goto error;
@@ -549,7 +559,7 @@ static int ap_probe(struct usb_interface *interface,
 		usb_fill_bulk_urb(urb, udev,
 				  usb_rcvbulkpipe(udev, es1->cport_in_endpoint),
 				  buffer, ES1_GBUF_MSG_SIZE, cport_in_callback,
-				  es1);
+				  hd);
 		es1->cport_in_urb[i] = urb;
 		es1->cport_in_buffer[i] = buffer;
 		retval = usb_submit_urb(urb, GFP_KERNEL);

drivers/staging/greybus/greybus.h

@@ -68,7 +68,9 @@
 
 struct greybus_host_device;
 struct svc_msg;
-struct gbuf;
+
+/* Buffers allocated from the host driver will be aligned to this multiple */
+#define GB_BUFFER_ALIGN	sizeof(u32)
 
 /* Greybus "Host driver" structure, needed by a host controller driver to be
  * able to handle both SVC control as well as "real" greybus messages
@@ -76,13 +78,13 @@ struct gbuf;
 struct greybus_host_driver {
 	size_t	hd_priv_size;
 
-	int (*alloc_gbuf_data)(struct gbuf *gbuf, unsigned int size,
-					gfp_t gfp_mask);
-	void (*free_gbuf_data)(struct gbuf *gbuf);
+	void *(*buffer_alloc)(unsigned int size, gfp_t gfp_mask);
+	void (*buffer_free)(void *buffer);
+	void *(*buffer_send)(struct greybus_host_device *hd, u16 dest_cport_id,
+			void *buffer, size_t buffer_size, gfp_t gfp_mask);
+	void (*buffer_cancel)(void *cookie);
 	int (*submit_svc)(struct svc_msg *svc_msg,
 			    struct greybus_host_device *hd);
-	int (*submit_gbuf)(struct gbuf *gbuf, gfp_t gfp_mask);
-	void (*kill_gbuf)(struct gbuf *gbuf);
 };
 
 struct greybus_host_device {
@@ -153,8 +155,6 @@ int gb_ap_init(void);
 void gb_ap_exit(void);
 int gb_debugfs_init(void);
 void gb_debugfs_cleanup(void);
-int gb_gbuf_init(void);
-void gb_gbuf_exit(void);
 
 extern struct bus_type greybus_bus_type;
 extern const struct attribute_group *greybus_module_groups[];

drivers/staging/greybus/operation.c

@@ -72,7 +72,7 @@ static void gb_pending_operation_insert(struct gb_operation *operation)
 	spin_unlock_irq(&gb_operations_lock);
 
 	/* Store the operation id in the request header */
-	header = operation->request.gbuf.transfer_buffer;
+	header = operation->request.buffer;
 	header->id = cpu_to_le16(operation->id);
 }
 
@@ -103,20 +103,37 @@ gb_pending_operation_find(struct gb_connection *connection, u16 id)
 	return found ? operation : NULL;
 }
 
-static int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask)
+static int gb_message_send(struct gb_message *message, gfp_t gfp_mask)
 {
-	gbuf->status = -EINPROGRESS;
-
-	return gbuf->hd->driver->submit_gbuf(gbuf, gfp_mask);
+	struct gb_connection *connection = message->operation->connection;
+	u16 dest_cport_id = connection->interface_cport_id;
+
+	message->status = -EINPROGRESS;
+	message->cookie = connection->hd->driver->buffer_send(connection->hd,
+					dest_cport_id,
+					message->buffer,
+					message->buffer_size,
+					gfp_mask);
+	if (IS_ERR(message->cookie)) {
+		message->status = PTR_ERR(message->cookie);
+		message->cookie = NULL;
+
+		return message->status;
+	}
+	return 0;
 }
 
-static void greybus_kill_gbuf(struct gbuf *gbuf)
+static void gb_message_cancel(struct gb_message *message)
 {
-	if (gbuf->status != -EINPROGRESS)
+	struct greybus_host_device *hd;
+
+	if (message->status != -EINPROGRESS)
 		return;
 
-	gbuf->hd->driver->kill_gbuf(gbuf);
+	hd = message->operation->connection->hd;
+	hd->driver->buffer_cancel(message->cookie);
 }
+
 /*
  * An operations's response message has arrived.  If no callback was
  * supplied it was submitted for asynchronous completion, so we notify
@@ -139,7 +156,7 @@ int gb_operation_wait(struct gb_operation *operation)
 	ret = wait_for_completion_interruptible(&operation->completion);
 	/* If interrupted, cancel the in-flight buffer */
 	if (ret < 0)
-		greybus_kill_gbuf(&operation->request.gbuf);
+		gb_message_cancel(&operation->request);
 	return ret;
 
 }
@@ -149,7 +166,7 @@ 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.gbuf.transfer_buffer;
+	header = operation->request.buffer;
 
 	/*
 	 * If the protocol has no incoming request handler, report
@@ -179,7 +196,7 @@ static void gb_operation_recv_work(struct work_struct *recv_work)
 	bool incoming_request;
 
 	operation = container_of(recv_work, struct gb_operation, recv_work);
-	incoming_request = operation->response.gbuf.transfer_buffer == NULL;
+	incoming_request = operation->response.buffer == NULL;
 	if (incoming_request)
 		gb_operation_request_handle(operation);
 	gb_operation_complete(operation);
@@ -213,16 +230,12 @@ static void operation_timeout(struct work_struct *work)
  */
 static int gb_operation_message_init(struct gb_operation *operation,
 					u8 type, size_t size,
-					bool request, bool data_out)
+					bool request, gfp_t gfp_flags)
 {
 	struct gb_connection *connection = operation->connection;
 	struct greybus_host_device *hd = connection->hd;
 	struct gb_message *message;
 	struct gb_operation_msg_hdr *header;
-	struct gbuf *gbuf;
-	gfp_t gfp_flags = data_out ? GFP_KERNEL : GFP_ATOMIC;
-	u16 dest_cport_id;
-	int ret;
 
 	if (size > GB_OPERATION_MESSAGE_SIZE_MAX)
 		return -E2BIG;
@@ -234,22 +247,15 @@ static int gb_operation_message_init(struct gb_operation *operation,
 		message = &operation->response;
 		type |= GB_OPERATION_TYPE_RESPONSE;
 	}
-	gbuf = &message->gbuf;
-
-	if (data_out)
-		dest_cport_id = connection->interface_cport_id;
-	else
-		dest_cport_id = CPORT_ID_BAD;
 
-	gbuf->hd = hd;
-	gbuf->dest_cport_id = dest_cport_id;
-	gbuf->status = -EBADR;	/* Initial value--means "never set" */
-	ret = hd->driver->alloc_gbuf_data(gbuf, size, gfp_flags);
-	if (ret)
-		return ret;
+	message->buffer = hd->driver->buffer_alloc(size, gfp_flags);
+	if (!message->buffer)
+		return -ENOMEM;
+	message->buffer_size = size;
+	message->status = -EBADR;	/* Initial value--means "never set" */
 
 	/* Fill in the header structure */
-	header = (struct gb_operation_msg_hdr *)gbuf->transfer_buffer;
+	header = message->buffer;
 	header->size = cpu_to_le16(size);
 	header->id = 0;		/* Filled in when submitted */
 	header->type = type;
@@ -262,9 +268,15 @@ static int gb_operation_message_init(struct gb_operation *operation,
 
 static void gb_operation_message_exit(struct gb_message *message)
 {
+	struct greybus_host_device *hd;
+
+	hd = message->operation->connection->hd;
+	hd->driver->buffer_free(message->buffer);
+
 	message->operation = NULL;
 	message->payload = NULL;
-	message->gbuf.hd->driver->free_gbuf_data(&message->gbuf);
+	message->buffer = NULL;
+	message->buffer_size = 0;
 }
 
 /*
@@ -298,13 +310,13 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection,
 	operation->connection = connection;
 
 	ret = gb_operation_message_init(operation, type, request_size,
-						true, outgoing);
+						true, gfp_flags);
 	if (ret)
 		goto err_cache;
 
 	if (outgoing) {
 		ret = gb_operation_message_init(operation, type, response_size,
-						false, false);
+						false, GFP_KERNEL);
 		if (ret)
 			goto err_request;
 	}
@@ -377,11 +389,11 @@ int gb_operation_request_send(struct gb_operation *operation,
 	 * XXX
 	 * I think the order of operations is going to be
 	 * significant, and if so, we may need a mutex to surround
-	 * setting the operation id and submitting the gbuf.
+	 * setting the operation id and submitting the buffer.
 	 */
 	operation->callback = callback;
 	gb_pending_operation_insert(operation);
-	ret = greybus_submit_gbuf(&operation->request.gbuf, GFP_KERNEL);
+	ret = gb_message_send(&operation->request, GFP_KERNEL);
 	if (ret)
 		return ret;
 
@@ -418,12 +430,12 @@ int gb_operation_response_send(struct gb_operation *operation)
  * data into the buffer and do remaining handling via a work queue.
  *
  */
-void gb_connection_operation_recv(struct gb_connection *connection,
+void gb_connection_recv(struct gb_connection *connection,
 				void *data, size_t size)
 {
 	struct gb_operation_msg_hdr *header;
 	struct gb_operation *operation;
-	struct gbuf *gbuf;
+	struct gb_message *message;
 	u16 msg_size;
 
 	if (connection->state != GB_CONNECTION_STATE_ENABLED)
@@ -446,8 +458,8 @@ void gb_connection_operation_recv(struct gb_connection *connection,
 		}
 		cancel_delayed_work(&operation->timeout_work);
 		gb_pending_operation_remove(operation);
-		gbuf = &operation->response.gbuf;
-		if (size > gbuf->transfer_buffer_length) {
+		message = &operation->response;
+		if (size > message->buffer_size) {
 			operation->result = GB_OP_OVERFLOW;
 			gb_connection_err(connection, "recv buffer too small");
 			return;
@@ -461,10 +473,10 @@ void gb_connection_operation_recv(struct gb_connection *connection,
 			gb_connection_err(connection, "can't create operation");
 			return;
 		}
-		gbuf = &operation->request.gbuf;
+		message = &operation->request;
 	}
 
-	memcpy(gbuf->transfer_buffer, data, msg_size);
+	memcpy(message->buffer, data, msg_size);
 
 	/* The rest will be handled in work queue context */
 	queue_work(gb_operation_recv_workqueue, &operation->recv_work);
@@ -476,9 +488,9 @@ void gb_connection_operation_recv(struct gb_connection *connection,
 void gb_operation_cancel(struct gb_operation *operation)
 {
 	operation->canceled = true;
-	greybus_kill_gbuf(&operation->request.gbuf);
-	if (operation->response.gbuf.transfer_buffer)
-		greybus_kill_gbuf(&operation->response.gbuf);
+	gb_message_cancel(&operation->request);
+	if (operation->response.buffer)
+		gb_message_cancel(&operation->response);
 }
 
 int gb_operation_init(void)

drivers/staging/greybus/operation.h

@@ -24,21 +24,16 @@ enum gb_operation_status {
 	GB_OP_TIMEOUT		= 0xff,
 };
 
-struct gbuf {
-	struct greybus_host_device *hd;
-	u16 dest_cport_id;		/* Destination CPort id */
-	int status;
-
-	void *transfer_buffer;
-	u32 transfer_buffer_length;
-
-	void *hcd_data;			/* for the HCD to track the gbuf */
-};
-
 struct gb_message {
 	void			*payload;
+
 	struct gb_operation	*operation;
-	struct gbuf		gbuf;
+	int			status;
+
+	void			*buffer;
+	size_t			buffer_size;
+
+	void			*cookie;
 };
 
 /*
@@ -87,7 +82,7 @@ struct gb_operation {
 	struct list_head	links;	/* connection->{operations,pending} */
 };
 
-void gb_connection_operation_recv(struct gb_connection *connection,
+void gb_connection_recv(struct gb_connection *connection,
 					void *data, size_t size);
 
 struct gb_operation *gb_operation_create(struct gb_connection *connection,