From: Greg Kroah-Hartman Date: Mon, 22 Sep 2014 00:34:28 +0000 (-0700) Subject: greybus: gbuf: clean up logic of who owns what "part" of the gbuf X-Git-Tag: v4.9-rc1~119^2~378^2~21^2~2100 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=3e7736e5c17801e15c0355d905988c03bbc0ba22;p=karo-tx-linux.git 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 961d6b1a6a21..1a47da6fb933 100644 --- a/drivers/staging/greybus/es1-ap-usb.c +++ b/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 */ diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c index 6cc752cdd1db..40174b8f63f4 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 @@ 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); } diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index 17a01bd41ee5..9802ccec79ea 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 @@ 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,