greybus: gbuf: clean up logic of who owns what "part" of the gbuf
authorGreg Kroah-Hartman <greg@kroah.com>
Mon, 22 Sep 2014 00:34:28 +0000 (17:34 -0700)
committerGreg Kroah-Hartman <greg@kroah.com>
Mon, 22 Sep 2014 00:34:28 +0000 (17:34 -0700)
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
drivers/staging/greybus/gbuf.c
drivers/staging/greybus/greybus.h

index 961d6b1..1a47da6 100644 (file)
@@ -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 */
index 6cc752c..40174b8 100644 (file)
@@ -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);
 }
index 17a01bd..9802cce 100644 (file)
        .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,