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...
diff --git a/drivers/staging/greybus/es1-ap-usb.c b/drivers/staging/greybus/es1-ap-usb.c
index 961d6b1..1a47da6 100644
--- a/drivers/staging/greybus/es1-ap-usb.c
+++ b/drivers/staging/greybus/es1-ap-usb.c
@@ -90,7 +90,7 @@
* 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 @@
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 @@
}
/* 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 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 */
diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c
index 6cc752c..40174b8 100644
--- a/drivers/staging/greybus/gbuf.c
+++ b/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 @@
{
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 @@
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 @@
{
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 @@
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 @@
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 @@
return;
}
gbuf->hdpriv = hd;
+ gbuf->direction = GBUF_DIRECTION_IN;
/*
* FIXME:
@@ -217,29 +235,16 @@
}
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 @@
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);
}
diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h
index 17a01bd..9802cce 100644
--- a/drivers/staging/greybus/greybus.h
+++ b/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 @@
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 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,