greybus: gbuf: clean up logic of who owns what "part" of the gbuf

Started documenting the gbuf and how a greybus driver and a host
controller driver needs to interact with it, and the rest of the greybus
system.  It's crude documentation, but better than nothing for now...

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

@@ -90,7 +90,7 @@ static void cport_out_callback(struct urb *urb);
  *	void *transfer_buffer;
  *	u32 transfer_buffer_length;
  */
-static int alloc_gbuf(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask)
+static int alloc_gbuf_data(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask)
 {
 	struct es1_ap_dev *es1 = hd_to_es1(gbuf->gdev->hd);
 	u8 *buffer;
@@ -116,6 +116,7 @@ static int alloc_gbuf(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask)
 	buffer[0] = gbuf->cport->number;
 	gbuf->transfer_buffer = &buffer[1];
 	gbuf->transfer_buffer_length = size;
+	gbuf->actual_length = size;
 
 	/* When we send the gbuf, we need this pointer to be here */
 	gbuf->hdpriv = es1;
@@ -124,14 +125,17 @@ static int alloc_gbuf(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask)
 }
 
 /* Free the memory we allocated with a gbuf */
-static void free_gbuf(struct gbuf *gbuf)
+static void free_gbuf_data(struct gbuf *gbuf)
 {
 	u8 *transfer_buffer;
 	u8 *buffer;
 
 	transfer_buffer = gbuf->transfer_buffer;
-	buffer = &transfer_buffer[-1];	/* yes, we mean -1 */
-	kfree(buffer);
+	/* Can be called with a NULL transfer_buffer on some error paths */
+	if (transfer_buffer) {
+		buffer = &transfer_buffer[-1];	/* yes, we mean -1 */
+		kfree(buffer);
+	}
 }
 
 #define ES1_TIMEOUT	500	/* 500 ms for the SVC to do something */
@@ -216,11 +220,11 @@ static int submit_gbuf(struct gbuf *gbuf, struct greybus_host_device *hd,
 }
 
 static struct greybus_host_driver es1_driver = {
-	.hd_priv_size	= sizeof(struct es1_ap_dev),
-	.alloc_gbuf	= alloc_gbuf,
-	.free_gbuf	= free_gbuf,
-	.send_svc_msg	= send_svc_msg,
-	.submit_gbuf	= submit_gbuf,
+	.hd_priv_size		= sizeof(struct es1_ap_dev),
+	.alloc_gbuf_data	= alloc_gbuf_data,
+	.free_gbuf_data		= free_gbuf_data,
+	.send_svc_msg		= send_svc_msg,
+	.submit_gbuf		= submit_gbuf,
 };
 
 /* Callback for when we get a SVC message */

drivers/staging/greybus/gbuf.c

@@ -19,6 +19,9 @@
 
 #include "greybus.h"
 
+
+static struct kmem_cache *gbuf_head_cache;
+
 static struct gbuf *__alloc_gbuf(struct greybus_device *gdev,
 				struct gdev_cport *cport,
 				gbuf_complete_t complete,
@@ -27,11 +30,7 @@ static struct gbuf *__alloc_gbuf(struct greybus_device *gdev,
 {
 	struct gbuf *gbuf;
 
-	/*
-	 * change this to a slab allocation if it's too slow, but for now, let's
-	 * be dumb and simple.
-	 */
-	gbuf = kzalloc(sizeof(*gbuf), gfp_mask);
+	gbuf = kmem_cache_zalloc(gbuf_head_cache, gfp_mask);
 	if (!gbuf)
 		return NULL;
 
@@ -73,10 +72,12 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_device *gdev,
 	if (!gbuf)
 		return NULL;
 
+	gbuf->direction = GBUF_DIRECTION_OUT;
+
 	/* Host controller specific allocation for the actual buffer */
-	retval = gbuf->gdev->hd->driver->alloc_gbuf(gbuf, size, gfp_mask);
+	retval = gbuf->gdev->hd->driver->alloc_gbuf_data(gbuf, size, gfp_mask);
 	if (retval) {
-		kfree(gbuf);
+		greybus_free_gbuf(gbuf);
 		return NULL;
 	}
 
@@ -90,10 +91,15 @@ static void free_gbuf(struct kref *kref)
 {
 	struct gbuf *gbuf = container_of(kref, struct gbuf, kref);
 
-	/* let the host controller free what it wants to */
-	gbuf->gdev->hd->driver->free_gbuf(gbuf);
+	/* If the direction is "out" then the host controller frees the data */
+	if (gbuf->direction == GBUF_DIRECTION_OUT) {
+		gbuf->gdev->hd->driver->free_gbuf_data(gbuf);
+	} else {
+		/* we "own" this in data, so free it ourselves */
+		kfree(gbuf->transfer_buffer);
+	}
 
-	kfree(gbuf);
+	kmem_cache_free(gbuf_head_cache, gbuf);
 }
 
 void greybus_free_gbuf(struct gbuf *gbuf)
@@ -123,6 +129,43 @@ int greybus_kill_gbuf(struct gbuf *gbuf)
 	return -ENOMEM;
 }
 
+struct cport_msg {
+	struct gbuf *gbuf;
+	struct work_struct event;
+};
+
+static struct workqueue_struct *cport_workqueue;
+
+static void cport_process_event(struct work_struct *work)
+{
+	struct cport_msg *cm;
+	struct gbuf *gbuf;
+
+	cm = container_of(work, struct cport_msg, event);
+
+	gbuf = cm->gbuf;
+
+	/* call the gbuf handler */
+	gbuf->complete(gbuf);
+
+	/* free all the memory */
+	greybus_free_gbuf(gbuf);
+	kfree(cm);
+}
+
+static void cport_create_event(struct gbuf *gbuf)
+{
+	struct cport_msg *cm;
+
+	/* Slow alloc, does it matter??? */
+	cm = kmalloc(sizeof(*cm), GFP_ATOMIC);
+
+	/* Queue up the cport message to be handled in user context */
+	cm->gbuf = gbuf;
+	INIT_WORK(&cm->event, cport_process_event);
+	queue_work(cport_workqueue, &cm->event);
+}
+
 #define MAX_CPORTS	1024
 struct gb_cport_handler {
 	gbuf_complete_t handler;
@@ -153,37 +196,11 @@ void gb_deregister_cport_complete(int cport)
 	cport_handler[cport].handler = NULL;
 }
 
-struct cport_msg {
-	struct gbuf *gbuf;
-	struct work_struct event;
-};
-
-static struct workqueue_struct *cport_workqueue;
-
-static void cport_process_event(struct work_struct *work)
-{
-	struct cport_msg *cm;
-	struct gbuf *gbuf;
-
-	cm = container_of(work, struct cport_msg, event);
-
-	gbuf = cm->gbuf;
-
-	/* call the gbuf handler */
-	gbuf->complete(gbuf);
-
-	/* free all the memory */
-	kfree(gbuf->transfer_buffer);
-	kfree(gbuf);
-	kfree(cm);
-}
-
 void greybus_cport_in_data(struct greybus_host_device *hd, int cport, u8 *data,
 			   size_t length)
 {
 	struct gb_cport_handler *ch;
 	struct gbuf *gbuf;
-	struct cport_msg *cm;
 
 	/* first check to see if we have a cport handler for this cport */
 	ch = &cport_handler[cport];
@@ -203,6 +220,7 @@ void greybus_cport_in_data(struct greybus_host_device *hd, int cport, u8 *data,
 		return;
 	}
 	gbuf->hdpriv = hd;
+	gbuf->direction = GBUF_DIRECTION_IN;
 
 	/*
 	 * FIXME:
@@ -217,29 +235,16 @@ void greybus_cport_in_data(struct greybus_host_device *hd, int cport, u8 *data,
 	}
 	memcpy(gbuf->transfer_buffer, data, length);
 	gbuf->transfer_buffer_length = length;
+	gbuf->actual_length = length;
 
-	/* Again with the slow allocate... */
-	cm = kmalloc(sizeof(*cm), GFP_ATOMIC);
-
-	/* Queue up the cport message to be handled in user context */
-	cm->gbuf = gbuf;
-	INIT_WORK(&cm->event, cport_process_event);
-	queue_work(cport_workqueue, &cm->event);
+	cport_create_event(gbuf);
 }
 EXPORT_SYMBOL_GPL(greybus_cport_in_data);
 
 /* Can be called in interrupt context, do the work and get out of here */
 void greybus_gbuf_finished(struct gbuf *gbuf)
 {
-	struct cport_msg *cm;
-
-	/* Again with the slow allocate... */
-	cm = kmalloc(sizeof(*cm), GFP_ATOMIC);
-	cm->gbuf = gbuf;
-	INIT_WORK(&cm->event, cport_process_event);
-	queue_work(cport_workqueue, &cm->event);
-
-	// FIXME - implement
+	cport_create_event(gbuf);
 }
 EXPORT_SYMBOL_GPL(greybus_gbuf_finished);
 
@@ -249,10 +254,13 @@ int gb_gbuf_init(void)
 	if (!cport_workqueue)
 		return -ENOMEM;
 
+	gbuf_head_cache = kmem_cache_create("gbuf_head_cache",
+					    sizeof(struct gbuf), 0, 0, NULL);
 	return 0;
 }
 
 void gb_gbuf_exit(void)
 {
 	destroy_workqueue(cport_workqueue);
+	kmem_cache_destroy(gbuf_head_cache);
 }

drivers/staging/greybus/greybus.h

@@ -36,6 +36,68 @@
 	.serial_number	= (s),
 
 
+/*
+  gbuf
+
+  This is the "main" data structure to send / receive Greybus messages
+
+  There are two different "views" of a gbuf structure:
+    - a greybus driver
+    - a greybus host controller
+
+  A Greybus driver needs to worry about the following:
+    - creating a gbuf
+    - putting data into a gbuf
+    - sending a gbuf to a device
+    - receiving a gbuf from a device
+
+  Creating a gbuf:
+    A greybus driver calls greybus_alloc_gbuf()
+  Putting data into a gbuf:
+    copy data into gbuf->transfer_buffer
+  Send a gbuf:
+    A greybus driver calls greybus_submit_gbuf()
+    The completion function in a gbuf will be called if the gbuf is successful
+    or not.  That completion function runs in user context.  After the
+    completion function is called, the gbuf must not be touched again as the
+    greybus core "owns" it.  But, if a greybus driver wants to "hold on" to a
+    gbuf after the completion function has been called, a reference must be
+    grabbed on the gbuf with a call to greybus_get_gbuf().  When finished with
+    the gbuf, call greybus_free_gbuf() and when the last reference count is
+    dropped, it will be removed from the system.
+  Receive a gbuf:
+    A greybus driver calls gb_register_cport_complete() with a pointer to the
+    callback function to be called for when a gbuf is received from a specific
+    cport and device.  That callback will be made in user context with a gbuf
+    when it is received.  To stop receiving messages, call
+    gb_deregister_cport_complete() for a specific cport.
+
+
+  Greybus Host controller drivers need to provide
+    - a way to allocate the transfer buffer for a gbuf
+    - a way to free the transfer buffer for a gbuf when it is "finished"
+    - a way to submit gbuf for transmissions
+    - notify the core the gbuf is complete
+    - receive gbuf from the wire and submit them to the core
+    - a way to send and receive svc messages
+  Allocate a transfer buffer
+    the host controller function alloc_gbuf_data is called
+  Free a transfer buffer
+    the host controller function free_gbuf_data is called
+  Submit a gbuf to the hardware
+    the host controller function submit_gbuf is called
+  Notify the gbuf is complete
+    the host controller driver must call greybus_gbuf_finished()
+  Submit a SVC message to the hardware
+    the host controller function send_svc_msg is called
+  Receive gbuf messages
+    the host controller driver must call greybus_cport_in_data() with the data
+  Reveive SVC messages from the hardware
+    The host controller driver must call gb_new_ap_msg
+
+
+*/
+
 
 struct gbuf;
 
@@ -66,10 +128,9 @@ struct gbuf {
 	u32 transfer_buffer_length;
 	u32 actual_length;
 
-#if 0
-	struct scatterlist *sg;		// FIXME do we need?
-	int num_sgs;
-#endif
+#define GBUF_DIRECTION_OUT	0
+#define GBUF_DIRECTION_IN	1
+	unsigned int	direction : 1;	/* 0 is out, 1 is in */
 
 	void *context;
 	gbuf_complete_t complete;
@@ -105,8 +166,8 @@ struct svc_msg;
 struct greybus_host_driver {
 	size_t	hd_priv_size;
 
-	int (*alloc_gbuf)(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask);
-	void (*free_gbuf)(struct gbuf *gbuf);
+	int (*alloc_gbuf_data)(struct gbuf *gbuf, unsigned int size, gfp_t gfp_mask);
+	void (*free_gbuf_data)(struct gbuf *gbuf);
 	int (*send_svc_msg)(struct svc_msg *svc_msg,
 			    struct greybus_host_device *hd);
 	int (*submit_gbuf)(struct gbuf *gbuf, struct greybus_host_device *hd,