From 6e5dd0bbbb046df2b6a5ba72d74b611c1f15f467 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:28 -0600 Subject: [PATCH 01/10] greybus: kill greybus_{get,put}_gbuf() These functions are never used, so we can get rid of them. Since there's no reference-getting function any more, we no longer need "gbuf_mutex" to avoid racing gets and puts, so get rid of that too. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gbuf.c | 14 +------------- drivers/staging/greybus/greybus.h | 2 -- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c index 6f8873af09e0..92da63257526 100644 --- a/drivers/staging/greybus/gbuf.c +++ b/drivers/staging/greybus/gbuf.c @@ -62,8 +62,6 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, } EXPORT_SYMBOL_GPL(greybus_alloc_gbuf); -static DEFINE_MUTEX(gbuf_mutex); - static void free_gbuf(struct kref *kref) { struct gbuf *gbuf = container_of(kref, struct gbuf, kref); @@ -71,25 +69,15 @@ static void free_gbuf(struct kref *kref) gbuf->hd->driver->free_gbuf_data(gbuf); kmem_cache_free(gbuf_head_cache, gbuf); - mutex_unlock(&gbuf_mutex); } void greybus_free_gbuf(struct gbuf *gbuf) { /* drop the reference count and get out of here */ - kref_put_mutex(&gbuf->kref, free_gbuf, &gbuf_mutex); + kref_put(&gbuf->kref, free_gbuf); } EXPORT_SYMBOL_GPL(greybus_free_gbuf); -struct gbuf *greybus_get_gbuf(struct gbuf *gbuf) -{ - mutex_lock(&gbuf_mutex); - kref_get(&gbuf->kref); - mutex_unlock(&gbuf_mutex); - return gbuf; -} -EXPORT_SYMBOL_GPL(greybus_get_gbuf); - int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask) { gbuf->status = -EINPROGRESS; diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index 90469bb83b27..3af338223609 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -184,8 +184,6 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, u16 dest_cport_id, unsigned int size, gfp_t gfp_mask); void greybus_free_gbuf(struct gbuf *gbuf); -struct gbuf *greybus_get_gbuf(struct gbuf *gbuf); -#define greybus_put_gbuf greybus_free_gbuf int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t mem_flags); void greybus_kill_gbuf(struct gbuf *gbuf); From 2f528c8bf7199c5eba93ea344a502910dc3a2806 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:29 -0600 Subject: [PATCH 02/10] greybus: kill gbuf->kref Since there is only ever one reference to a gbuf, we don't need a kref to figure out when it can be freed. Get rid of the kref and its supporting code. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gbuf.c | 12 +----------- drivers/staging/greybus/greybus.h | 2 -- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c index 92da63257526..1d0dd4acfa2a 100644 --- a/drivers/staging/greybus/gbuf.c +++ b/drivers/staging/greybus/gbuf.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -46,7 +45,6 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, if (!gbuf) return NULL; - kref_init(&gbuf->kref); gbuf->hd = hd; gbuf->dest_cport_id = dest_cport_id; gbuf->status = -EBADR; /* Initial value--means "never set" */ @@ -62,20 +60,12 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, } EXPORT_SYMBOL_GPL(greybus_alloc_gbuf); -static void free_gbuf(struct kref *kref) +void greybus_free_gbuf(struct gbuf *gbuf) { - struct gbuf *gbuf = container_of(kref, struct gbuf, kref); - gbuf->hd->driver->free_gbuf_data(gbuf); kmem_cache_free(gbuf_head_cache, gbuf); } - -void greybus_free_gbuf(struct gbuf *gbuf) -{ - /* drop the reference count and get out of here */ - kref_put(&gbuf->kref, free_gbuf); -} EXPORT_SYMBOL_GPL(greybus_free_gbuf); int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask) diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index 3af338223609..173170065261 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -119,8 +119,6 @@ */ struct gbuf { - struct kref kref; - struct greybus_host_device *hd; u16 dest_cport_id; /* Destination CPort id */ int status; From 3c3cef400ea7eadc87a66a974cd9e388ad99640a Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:30 -0600 Subject: [PATCH 03/10] greybus: move the definition of struct gbuf We no longer need struct gbuf defined in "greybus.h". An upcoming patch will embed a gbuf struct (not a pointer) into the operation structure, and to do that we'll need the struct defined prior to the operation. Just move the gbuf definition into "operation.h". Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus.h | 75 +---------------------------- drivers/staging/greybus/operation.h | 14 +++++- 2 files changed, 14 insertions(+), 75 deletions(-) diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index 173170065261..d6bfe6b2f70f 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -56,80 +56,6 @@ #define HOST_DEV_CPORT_ID_MAX CONFIG_HOST_DEV_CPORT_ID_MAX #define CPORT_ID_BAD U16_MAX /* UniPro max id is 4095 */ -/* - 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() with the data - Reveive SVC messages from the hardware - The host controller driver must call greybus_svc_in - - -*/ - -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 */ -}; - - /* For SP1 hardware, we are going to "hardcode" each device to have all logical * blocks in order to be able to address them as one unified "unit". Then * higher up layers will then be able to talk to them as one logical block and @@ -142,6 +68,7 @@ struct gbuf { struct greybus_host_device; struct svc_msg; +struct gbuf; /* Greybus "Host driver" structure, needed by a host controller driver to be * able to handle both SVC control as well as "real" greybus messages diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index dc15c2f61e30..c19313608e81 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -11,6 +11,8 @@ #include +struct gb_operation; + enum gb_operation_status { GB_OP_SUCCESS = 0, GB_OP_INVALID = 1, @@ -22,6 +24,17 @@ 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 */ +}; + /* * A Greybus operation is a remote procedure call performed over a * connection between the AP and a function on Greybus module. @@ -50,7 +63,6 @@ enum gb_operation_status { * is guaranteed to be 64-bit aligned. * XXX and callback? */ -struct gb_operation; typedef void (*gb_operation_callback)(struct gb_operation *); struct gb_operation { struct gb_connection *connection; From 3690a826fae5102ed5daed2340926885980d51ab Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:31 -0600 Subject: [PATCH 04/10] greybus: define struct gb_message A Greybus buffer (gbuf) is a generic buffer used for data transfer over a Greybus interconnect. We only ever use gbufs in operations, which always involve exactly two of them. The lifetime of a gbuf is therefore directly connected to the lifetime of an operation, so there no real need to manage gbufs separate from operations. This patch begins the process of removing the gbuf abstraction, on favor of a new data type, gb_message. The purpose of a gb_message is--like a gbuf--to represent data to be transferred over Greybus. However a gb_message is oriented toward the more restrictive way we do Greybus transfers--as operation messages (either a request or a response). This patch simply defines the structure in its initial form, and defines the request and response fields in a Greybus operation structure as embedded instances of that type. The gbuf pointer is defined within the gb_message structure, and as a result lots of code needs to be tweaked to reference the request and response gbufs as subfields of the request and response structures. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio-gb.c | 36 ++++++++++++------------- drivers/staging/greybus/i2c-gb.c | 16 +++++------ drivers/staging/greybus/operation.c | 42 +++++++++++++++-------------- drivers/staging/greybus/operation.h | 14 +++++----- drivers/staging/greybus/pwm-gb.c | 28 +++++++++---------- drivers/staging/greybus/uart-gb.c | 16 +++++------ 6 files changed, 78 insertions(+), 74 deletions(-) diff --git a/drivers/staging/greybus/gpio-gb.c b/drivers/staging/greybus/gpio-gb.c index 473df4d47ac1..6f4609da7f97 100644 --- a/drivers/staging/greybus/gpio-gb.c +++ b/drivers/staging/greybus/gpio-gb.c @@ -153,7 +153,7 @@ static int gb_gpio_proto_version_operation(struct gb_gpio_controller *gb_gpio_co goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "version response %hhu", response->status); @@ -199,7 +199,7 @@ static int gb_gpio_line_count_operation(struct gb_gpio_controller *gb_gpio_contr goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "line count response %hhu", response->status); @@ -234,7 +234,7 @@ static int gb_gpio_activate_operation(struct gb_gpio_controller *gb_gpio_control sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -244,7 +244,7 @@ static int gb_gpio_activate_operation(struct gb_gpio_controller *gb_gpio_control goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "activate response %hhu", response->status); @@ -278,7 +278,7 @@ static int gb_gpio_deactivate_operation(struct gb_gpio_controller *gb_gpio_contr sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -288,7 +288,7 @@ static int gb_gpio_deactivate_operation(struct gb_gpio_controller *gb_gpio_contr goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "deactivate response %hhu", response->status); @@ -320,7 +320,7 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *gb_gpio_co sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -330,7 +330,7 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *gb_gpio_co goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "get direction response %hhu", response->status); @@ -369,7 +369,7 @@ static int gb_gpio_direction_in_operation(struct gb_gpio_controller *gb_gpio_con sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -379,7 +379,7 @@ static int gb_gpio_direction_in_operation(struct gb_gpio_controller *gb_gpio_con goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "direction in response %hhu", response->status); @@ -412,7 +412,7 @@ static int gb_gpio_direction_out_operation(struct gb_gpio_controller *gb_gpio_co sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; request->value = value_high ? 1 : 0; @@ -423,7 +423,7 @@ static int gb_gpio_direction_out_operation(struct gb_gpio_controller *gb_gpio_co goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "direction out response %hhu", response->status); @@ -456,7 +456,7 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *gb_gpio_contro sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -466,7 +466,7 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *gb_gpio_contro goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "get value response %hhu", response->status); @@ -507,7 +507,7 @@ static int gb_gpio_set_value_operation(struct gb_gpio_controller *gb_gpio_contro sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; request->value = value_high ? 1 : 0; @@ -518,7 +518,7 @@ static int gb_gpio_set_value_operation(struct gb_gpio_controller *gb_gpio_contro goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "set value response %hhu", response->status); @@ -554,7 +554,7 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *gb_gpio_con sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; request->usec = cpu_to_le16(debounce_usec); @@ -565,7 +565,7 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *gb_gpio_con goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "set debounce response %hhu", response->status); diff --git a/drivers/staging/greybus/i2c-gb.c b/drivers/staging/greybus/i2c-gb.c index c179078b8a2e..3374173b012a 100644 --- a/drivers/staging/greybus/i2c-gb.c +++ b/drivers/staging/greybus/i2c-gb.c @@ -118,7 +118,7 @@ static int gb_i2c_proto_version_operation(struct gb_i2c_device *gb_i2c_dev) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "version response %hhu", response->status); @@ -170,7 +170,7 @@ static int gb_i2c_functionality_operation(struct gb_i2c_device *gb_i2c_dev) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "functionality response %hhu", response->status); @@ -198,7 +198,7 @@ static int gb_i2c_timeout_operation(struct gb_i2c_device *gb_i2c_dev, u16 msec) sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->msec = cpu_to_le16(msec); /* Synchronous operation--no callback */ @@ -208,7 +208,7 @@ static int gb_i2c_timeout_operation(struct gb_i2c_device *gb_i2c_dev, u16 msec) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "timeout response %hhu", response->status); @@ -235,7 +235,7 @@ static int gb_i2c_retries_operation(struct gb_i2c_device *gb_i2c_dev, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->retries = retries; /* Synchronous operation--no callback */ @@ -245,7 +245,7 @@ static int gb_i2c_retries_operation(struct gb_i2c_device *gb_i2c_dev, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "retries response %hhu", response->status); @@ -321,7 +321,7 @@ gb_i2c_transfer_request(struct gb_connection *connection, if (!operation) return NULL; - request = operation->request_payload; + request = operation->request.payload; request->op_count = cpu_to_le16(op_count); /* Fill in the ops array */ op = &request->ops[0]; @@ -380,7 +380,7 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { if (response->status == GB_OP_RETRY) { ret = -EAGAIN; diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index c0161a2c8cee..82f4f2098109 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -71,7 +71,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->transfer_buffer; + header = operation->request.gbuf->transfer_buffer; header->id = cpu_to_le16(operation->id); } @@ -124,7 +124,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); + greybus_kill_gbuf(operation->request.gbuf); return ret; } @@ -134,7 +134,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->transfer_buffer; + header = operation->request.gbuf->transfer_buffer; /* * If the protocol has no incoming request handler, report @@ -164,7 +164,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 == NULL; + incoming_request = operation->response.gbuf == NULL; if (incoming_request) gb_operation_request_handle(operation); gb_operation_complete(operation); @@ -257,23 +257,25 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection, return NULL; operation->connection = connection; - operation->request = gb_operation_gbuf_create(operation, type, + operation->request.gbuf = gb_operation_gbuf_create(operation, type, request_size, outgoing); - if (!operation->request) + if (!operation->request.gbuf) goto err_cache; - operation->request_payload = operation->request->transfer_buffer + + operation->request.operation = operation; + operation->request.payload = operation->request.gbuf->transfer_buffer + sizeof(struct gb_operation_msg_hdr); if (outgoing) { type |= GB_OPERATION_TYPE_RESPONSE; - operation->response = gb_operation_gbuf_create(operation, + operation->response.gbuf = gb_operation_gbuf_create(operation, type, response_size, false); - if (!operation->response) + if (!operation->response.gbuf) goto err_request; - operation->response_payload = - operation->response->transfer_buffer + + operation->response.operation = operation; + operation->response.payload = + operation->response.gbuf->transfer_buffer + sizeof(struct gb_operation_msg_hdr); } @@ -290,7 +292,7 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection, return operation; err_request: - greybus_free_gbuf(operation->request); + greybus_free_gbuf(operation->request.gbuf); err_cache: kmem_cache_free(gb_operation_cache, operation); @@ -311,8 +313,8 @@ static void _gb_operation_destroy(struct kref *kref) list_del(&operation->links); spin_unlock_irq(&gb_operations_lock); - greybus_free_gbuf(operation->response); - greybus_free_gbuf(operation->request); + greybus_free_gbuf(operation->response.gbuf); + greybus_free_gbuf(operation->request.gbuf); kmem_cache_free(gb_operation_cache, operation); } @@ -349,7 +351,7 @@ int gb_operation_request_send(struct gb_operation *operation, */ operation->callback = callback; gb_pending_operation_insert(operation); - ret = greybus_submit_gbuf(operation->request, GFP_KERNEL); + ret = greybus_submit_gbuf(operation->request.gbuf, GFP_KERNEL); if (ret) return ret; @@ -414,7 +416,7 @@ void gb_connection_operation_recv(struct gb_connection *connection, } cancel_delayed_work(&operation->timeout_work); gb_pending_operation_remove(operation); - gbuf = operation->response; + gbuf = operation->response.gbuf; if (size > gbuf->transfer_buffer_length) { operation->result = GB_OP_OVERFLOW; gb_connection_err(connection, "recv buffer too small"); @@ -429,7 +431,7 @@ void gb_connection_operation_recv(struct gb_connection *connection, gb_connection_err(connection, "can't create operation"); return; } - gbuf = operation->request; + gbuf = operation->request.gbuf; } memcpy(gbuf->transfer_buffer, data, msg_size); @@ -444,9 +446,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); - if (operation->response) - greybus_kill_gbuf(operation->response); + greybus_kill_gbuf(operation->request.gbuf); + if (operation->response.gbuf) + greybus_kill_gbuf(operation->response.gbuf); } int gb_operation_init(void) diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index c19313608e81..a18713457ba1 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -35,6 +35,12 @@ struct gbuf { void *hcd_data; /* for the HCD to track the gbuf */ }; +struct gb_message { + void *payload; + struct gb_operation *operation; + struct gbuf *gbuf; +}; + /* * A Greybus operation is a remote procedure call performed over a * connection between the AP and a function on Greybus module. @@ -66,8 +72,8 @@ struct gbuf { typedef void (*gb_operation_callback)(struct gb_operation *); struct gb_operation { struct gb_connection *connection; - struct gbuf *request; - struct gbuf *response; + struct gb_message request; + struct gb_message response; u16 id; bool canceled; @@ -79,10 +85,6 @@ struct gb_operation { struct kref kref; struct list_head links; /* connection->{operations,pending} */ - - /* These are what's used by caller */ - void *request_payload; - void *response_payload; }; void gb_connection_operation_recv(struct gb_connection *connection, diff --git a/drivers/staging/greybus/pwm-gb.c b/drivers/staging/greybus/pwm-gb.c index 44a4f64acd8e..0f465522223a 100644 --- a/drivers/staging/greybus/pwm-gb.c +++ b/drivers/staging/greybus/pwm-gb.c @@ -110,7 +110,7 @@ static int gb_pwm_proto_version_operation(struct gb_pwm_chip *pwmc) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "version response %hhu", response->status); @@ -151,7 +151,7 @@ static int gb_pwm_count_operation(struct gb_pwm_chip *pwmc) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "pwm count response %hhu", response->status); @@ -181,7 +181,7 @@ static int gb_pwm_activate_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -191,7 +191,7 @@ static int gb_pwm_activate_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "activate response %hhu", response->status); @@ -220,7 +220,7 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -230,7 +230,7 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "deactivate response %hhu", response->status); @@ -258,7 +258,7 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; request->duty = duty; request->period = period; @@ -270,7 +270,7 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "config response %hhu", response->status); @@ -299,7 +299,7 @@ static int gb_pwm_set_polarity_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; request->polarity = polarity; @@ -310,7 +310,7 @@ static int gb_pwm_set_polarity_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "set polarity response %hhu", response->status); @@ -339,7 +339,7 @@ static int gb_pwm_enable_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -349,7 +349,7 @@ static int gb_pwm_enable_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "enable response %hhu", response->status); @@ -378,7 +378,7 @@ static int gb_pwm_disable_operation(struct gb_pwm_chip *pwmc, sizeof(*request), sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->which = which; /* Synchronous operation--no callback */ @@ -388,7 +388,7 @@ static int gb_pwm_disable_operation(struct gb_pwm_chip *pwmc, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "disable response %hhu", response->status); diff --git a/drivers/staging/greybus/uart-gb.c b/drivers/staging/greybus/uart-gb.c index a4f745afa885..a8c342eb09d3 100644 --- a/drivers/staging/greybus/uart-gb.c +++ b/drivers/staging/greybus/uart-gb.c @@ -229,7 +229,7 @@ static int send_data(struct gb_tty *tty, u16 size, const u8 *data) sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->size = cpu_to_le16(size); memcpy(&request->data[0], data, size); @@ -241,7 +241,7 @@ static int send_data(struct gb_tty *tty, u16 size, const u8 *data) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "send data response %hhu", response->status); @@ -267,7 +267,7 @@ static int send_line_coding(struct gb_tty *tty, sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; memcpy(&request->line_coding, line_coding, sizeof(*line_coding)); /* Synchronous operation--no callback */ @@ -278,7 +278,7 @@ static int send_line_coding(struct gb_tty *tty, goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "send line coding response %hhu", response->status); @@ -304,7 +304,7 @@ static int send_control(struct gb_tty *tty, u16 control) sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->control = cpu_to_le16(control); /* Synchronous operation--no callback */ @@ -315,7 +315,7 @@ static int send_control(struct gb_tty *tty, u16 control) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "send control response %hhu", response->status); @@ -345,7 +345,7 @@ static int send_break(struct gb_tty *tty, u8 state) sizeof(*response)); if (!operation) return -ENOMEM; - request = operation->request_payload; + request = operation->request.payload; request->state = state; /* Synchronous operation--no callback */ @@ -356,7 +356,7 @@ static int send_break(struct gb_tty *tty, u8 state) goto out; } - response = operation->response_payload; + response = operation->response.payload; if (response->status) { gb_connection_err(connection, "send break response %hhu", response->status); From c7f82d5dc07181b56b9596adab3a3891ace357fd Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:32 -0600 Subject: [PATCH 05/10] greybus: start using struct gb_message This converts some of the operation code to start leveraging the new gb_message type. Instead of creating the request and response gbufs, we initialize (and tear down with a new function) the request and response message structures. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/operation.c | 70 +++++++++++++++++------------ 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index 82f4f2098109..18186fd2db8c 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -23,6 +23,7 @@ /* * XXX This needs to be coordinated with host driver parameters + * XXX May need to reduce to allow for message header within a page */ #define GB_OPERATION_MESSAGE_SIZE_MAX 4096 @@ -196,36 +197,56 @@ static void operation_timeout(struct work_struct *work) * initialize it here (it'll be overwritten by the incoming * message). */ -static struct gbuf *gb_operation_gbuf_create(struct gb_operation *operation, - u8 type, size_t size, - bool data_out) +static int gb_operation_message_init(struct gb_operation *operation, + u8 type, size_t size, + bool request, bool data_out) { struct gb_connection *connection = operation->connection; + 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; if (size > GB_OPERATION_MESSAGE_SIZE_MAX) - return NULL; /* Message too big */ + return -E2BIG; + if (request) { + message = &operation->request; + } else { + message = &operation->response; + type |= GB_OPERATION_TYPE_RESPONSE; + } if (data_out) dest_cport_id = connection->interface_cport_id; else dest_cport_id = CPORT_ID_BAD; + + if (message->gbuf) + return -EALREADY; /* Sanity check */ size += sizeof(*header); - gbuf = greybus_alloc_gbuf(connection->hd, dest_cport_id, + message->gbuf = greybus_alloc_gbuf(connection->hd, dest_cport_id, size, gfp_flags); - if (!gbuf) - return NULL; + if (!message->gbuf) + return -ENOMEM; /* Fill in the header structure */ - header = (struct gb_operation_msg_hdr *)gbuf->transfer_buffer; + header = (struct gb_operation_msg_hdr *)message->gbuf->transfer_buffer; header->size = cpu_to_le16(size); header->id = 0; /* Filled in when submitted */ header->type = type; - return gbuf; + message->payload = header + 1; + message->operation = operation; + + return 0; +} + +static void gb_operation_message_exit(struct gb_message *message) +{ + message->operation = NULL; + message->payload = NULL; + greybus_free_gbuf(message->gbuf); + message->gbuf = NULL; } /* @@ -251,32 +272,23 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection, struct gb_operation *operation; gfp_t gfp_flags = response_size ? GFP_KERNEL : GFP_ATOMIC; bool outgoing = response_size != 0; + int ret; operation = kmem_cache_zalloc(gb_operation_cache, gfp_flags); if (!operation) return NULL; operation->connection = connection; - operation->request.gbuf = gb_operation_gbuf_create(operation, type, - request_size, - outgoing); - if (!operation->request.gbuf) + ret = gb_operation_message_init(operation, type, request_size, + true, outgoing); + if (ret) goto err_cache; - operation->request.operation = operation; - operation->request.payload = operation->request.gbuf->transfer_buffer + - sizeof(struct gb_operation_msg_hdr); if (outgoing) { - type |= GB_OPERATION_TYPE_RESPONSE; - operation->response.gbuf = gb_operation_gbuf_create(operation, - type, response_size, - false); - if (!operation->response.gbuf) + ret = gb_operation_message_init(operation, type, response_size, + false, false); + if (ret) goto err_request; - operation->response.operation = operation; - operation->response.payload = - operation->response.gbuf->transfer_buffer + - sizeof(struct gb_operation_msg_hdr); } INIT_WORK(&operation->recv_work, gb_operation_recv_work); @@ -292,7 +304,7 @@ struct gb_operation *gb_operation_create(struct gb_connection *connection, return operation; err_request: - greybus_free_gbuf(operation->request.gbuf); + gb_operation_message_exit(&operation->request); err_cache: kmem_cache_free(gb_operation_cache, operation); @@ -313,8 +325,8 @@ static void _gb_operation_destroy(struct kref *kref) list_del(&operation->links); spin_unlock_irq(&gb_operations_lock); - greybus_free_gbuf(operation->response.gbuf); - greybus_free_gbuf(operation->request.gbuf); + gb_operation_message_exit(&operation->response); + gb_operation_message_exit(&operation->request); kmem_cache_free(gb_operation_cache, operation); } From bb88896eaf2577b8938f05a8f70c45cee0714a18 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:33 -0600 Subject: [PATCH 06/10] greybus: move gbuf initialization to caller Change greybus_alloc_gbuf() so all it does is allocate the gbuf data structure. Move all of the initialization of the gbuf structure in the caller. Do the inverse in the caller prior to freeing the gbuf structure via greybus_free_gbuf(). Use a null gbuf->transfer_buffer pointer rather than a null gbuf pointer to indicate an unused gbuf. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gbuf.c | 22 +--------------------- drivers/staging/greybus/greybus.h | 3 ++- drivers/staging/greybus/operation.c | 20 ++++++++++++++++---- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c index 1d0dd4acfa2a..5ffd257de68f 100644 --- a/drivers/staging/greybus/gbuf.c +++ b/drivers/staging/greybus/gbuf.c @@ -38,32 +38,12 @@ struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, unsigned int size, gfp_t gfp_mask) { - struct gbuf *gbuf; - int retval; - - gbuf = kmem_cache_zalloc(gbuf_head_cache, gfp_mask); - if (!gbuf) - return NULL; - - gbuf->hd = hd; - gbuf->dest_cport_id = dest_cport_id; - gbuf->status = -EBADR; /* Initial value--means "never set" */ - - /* Host controller specific allocation for the actual buffer */ - retval = hd->driver->alloc_gbuf_data(gbuf, size, gfp_mask); - if (retval) { - kmem_cache_free(gbuf_head_cache, gbuf); - return NULL; - } - - return gbuf; + return kmem_cache_zalloc(gbuf_head_cache, gfp_mask); } EXPORT_SYMBOL_GPL(greybus_alloc_gbuf); void greybus_free_gbuf(struct gbuf *gbuf) { - gbuf->hd->driver->free_gbuf_data(gbuf); - kmem_cache_free(gbuf_head_cache, gbuf); } EXPORT_SYMBOL_GPL(greybus_free_gbuf); diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index d6bfe6b2f70f..e1f918d50df2 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -106,7 +106,8 @@ void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, u8 *data, size_t length); struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, - u16 dest_cport_id, unsigned int size, + u16 dest_cport_id, + unsigned int size, gfp_t gfp_mask); void greybus_free_gbuf(struct gbuf *gbuf); diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index 18186fd2db8c..ab03e3edc807 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -202,10 +202,13 @@ static int gb_operation_message_init(struct gb_operation *operation, bool request, bool data_out) { 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; @@ -224,19 +227,27 @@ static int gb_operation_message_init(struct gb_operation *operation, if (message->gbuf) return -EALREADY; /* Sanity check */ size += sizeof(*header); - message->gbuf = greybus_alloc_gbuf(connection->hd, dest_cport_id, - size, gfp_flags); - if (!message->gbuf) + gbuf = greybus_alloc_gbuf(hd, dest_cport_id, size, gfp_flags); + if (!gbuf) return -ENOMEM; + 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) { + greybus_free_gbuf(gbuf); + return ret; + } /* Fill in the header structure */ - header = (struct gb_operation_msg_hdr *)message->gbuf->transfer_buffer; + header = (struct gb_operation_msg_hdr *)gbuf->transfer_buffer; header->size = cpu_to_le16(size); header->id = 0; /* Filled in when submitted */ header->type = type; message->payload = header + 1; message->operation = operation; + message->gbuf = gbuf; return 0; } @@ -245,6 +256,7 @@ static void gb_operation_message_exit(struct gb_message *message) { message->operation = NULL; message->payload = NULL; + message->gbuf->hd->driver->free_gbuf_data(message->gbuf); greybus_free_gbuf(message->gbuf); message->gbuf = NULL; } From f7935e333ad2f94ccf8dcc8185a2ec836bb81adb Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:34 -0600 Subject: [PATCH 07/10] greybus: use null gbuf->transfer_buffer Make sure gbuf->transfer_buffer gets reset to NULL when the buffer is freed. We can leverage that to do a little extra error checking. We'll also use a null transfer buffer in the next patch to indicate an unused gbuf. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1-ap-usb.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/staging/greybus/es1-ap-usb.c b/drivers/staging/greybus/es1-ap-usb.c index e24012a9b6db..e276f0c4e3cd 100644 --- a/drivers/staging/greybus/es1-ap-usb.c +++ b/drivers/staging/greybus/es1-ap-usb.c @@ -99,6 +99,9 @@ static int alloc_gbuf_data(struct gbuf *gbuf, unsigned int size, 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); @@ -146,6 +149,7 @@ static void free_gbuf_data(struct gbuf *gbuf) if (gbuf->dest_cport_id != CPORT_ID_BAD) transfer_buffer--; /* Back up to cport id */ kfree(transfer_buffer); + gbuf->transfer_buffer = NULL; } #define ES1_TIMEOUT 500 /* 500 ms for the SVC to do something */ @@ -214,6 +218,8 @@ static int submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask) struct urb *urb; transfer_buffer = gbuf->transfer_buffer; + if (!transfer_buffer) + return -EINVAL; buffer = &transfer_buffer[-1]; /* yes, we mean -1 */ /* Find a free urb */ From bc46fabccdf4565f1228d4d775680d9c85934024 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:35 -0600 Subject: [PATCH 08/10] greybus: embed gbufs into operation message structure Embed the gbuf structures for operation messages into the message structure rather than pointing to a dynamically allocated one. Use a null gbuf->transfer_buffer pointer rather than a null gbuf pointer to indicate an unused gbuf. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/operation.c | 38 ++++++++++++----------------- drivers/staging/greybus/operation.h | 2 +- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index ab03e3edc807..7eaea71f8604 100644 --- a/drivers/staging/greybus/operation.c +++ b/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.gbuf.transfer_buffer; header->id = cpu_to_le16(operation->id); } @@ -125,7 +125,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); + greybus_kill_gbuf(&operation->request.gbuf); return ret; } @@ -135,7 +135,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.gbuf.transfer_buffer; /* * If the protocol has no incoming request handler, report @@ -165,7 +165,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 == NULL; + incoming_request = operation->response.gbuf.transfer_buffer == NULL; if (incoming_request) gb_operation_request_handle(operation); gb_operation_complete(operation); @@ -212,6 +212,7 @@ static int gb_operation_message_init(struct gb_operation *operation, if (size > GB_OPERATION_MESSAGE_SIZE_MAX) return -E2BIG; + size += sizeof(*header); if (request) { message = &operation->request; @@ -219,25 +220,19 @@ 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; - if (message->gbuf) - return -EALREADY; /* Sanity check */ - size += sizeof(*header); - gbuf = greybus_alloc_gbuf(hd, dest_cport_id, size, gfp_flags); - if (!gbuf) - return -ENOMEM; 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) { - greybus_free_gbuf(gbuf); + if (ret) return ret; - } /* Fill in the header structure */ header = (struct gb_operation_msg_hdr *)gbuf->transfer_buffer; @@ -247,7 +242,6 @@ static int gb_operation_message_init(struct gb_operation *operation, message->payload = header + 1; message->operation = operation; - message->gbuf = gbuf; return 0; } @@ -256,9 +250,7 @@ static void gb_operation_message_exit(struct gb_message *message) { message->operation = NULL; message->payload = NULL; - message->gbuf->hd->driver->free_gbuf_data(message->gbuf); - greybus_free_gbuf(message->gbuf); - message->gbuf = NULL; + message->gbuf.hd->driver->free_gbuf_data(&message->gbuf); } /* @@ -375,7 +367,7 @@ int gb_operation_request_send(struct gb_operation *operation, */ operation->callback = callback; gb_pending_operation_insert(operation); - ret = greybus_submit_gbuf(operation->request.gbuf, GFP_KERNEL); + ret = greybus_submit_gbuf(&operation->request.gbuf, GFP_KERNEL); if (ret) return ret; @@ -440,7 +432,7 @@ void gb_connection_operation_recv(struct gb_connection *connection, } cancel_delayed_work(&operation->timeout_work); gb_pending_operation_remove(operation); - gbuf = operation->response.gbuf; + gbuf = &operation->response.gbuf; if (size > gbuf->transfer_buffer_length) { operation->result = GB_OP_OVERFLOW; gb_connection_err(connection, "recv buffer too small"); @@ -455,7 +447,7 @@ void gb_connection_operation_recv(struct gb_connection *connection, gb_connection_err(connection, "can't create operation"); return; } - gbuf = operation->request.gbuf; + gbuf = &operation->request.gbuf; } memcpy(gbuf->transfer_buffer, data, msg_size); @@ -470,9 +462,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) - greybus_kill_gbuf(operation->response.gbuf); + greybus_kill_gbuf(&operation->request.gbuf); + if (operation->response.gbuf.transfer_buffer) + greybus_kill_gbuf(&operation->response.gbuf); } int gb_operation_init(void) diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index a18713457ba1..f43531dbcf33 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -38,7 +38,7 @@ struct gbuf { struct gb_message { void *payload; struct gb_operation *operation; - struct gbuf *gbuf; + struct gbuf gbuf; }; /* From 4e5007e5c27e012dd50db4c96cb9f57d235df1ee Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:36 -0600 Subject: [PATCH 09/10] greybus: kill the gbuf slab cache Nobody dynamically allocates gbufs any more, so we can get rid of the allocation and free routines, as as the slab cache and its related code. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/core.c | 9 ------- drivers/staging/greybus/gbuf.c | 45 ------------------------------- drivers/staging/greybus/greybus.h | 6 ----- 3 files changed, 60 deletions(-) diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c index 26e4b44bdf44..588e62412fd3 100644 --- a/drivers/staging/greybus/core.c +++ b/drivers/staging/greybus/core.c @@ -226,12 +226,6 @@ static int __init gb_init(void) goto error_ap; } - retval = gb_gbuf_init(); - if (retval) { - pr_err("gb_gbuf_init failed\n"); - goto error_gbuf; - } - retval = gb_operation_init(); if (retval) { pr_err("gb_operation_init failed\n"); @@ -250,8 +244,6 @@ static int __init gb_init(void) error_protocol: gb_operation_exit(); error_operation: - gb_gbuf_exit(); -error_gbuf: gb_ap_exit(); error_ap: bus_unregister(&greybus_bus_type); @@ -265,7 +257,6 @@ static void __exit gb_exit(void) { gb_protocol_exit(); gb_operation_exit(); - gb_gbuf_exit(); gb_ap_exit(); bus_unregister(&greybus_bus_type); gb_debugfs_cleanup(); diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c index 5ffd257de68f..d47cf367e412 100644 --- a/drivers/staging/greybus/gbuf.c +++ b/drivers/staging/greybus/gbuf.c @@ -13,41 +13,9 @@ #include #include #include -#include #include "greybus.h" -static struct kmem_cache *gbuf_head_cache; - -/** - * greybus_alloc_gbuf - allocate a greybus buffer - * - * @gmod: greybus device that wants to allocate this - * @cport: cport to send the data to - * @complete: callback when the gbuf is finished with - * @size: size of the buffer - * @gfp_mask: allocation mask - * - * TODO: someday it will be nice to handle DMA, but for now, due to the - * architecture we are stuck with, the greybus core has to allocate the buffer - * that the driver can then fill up with the data to be sent out. Curse - * hardware designers for this issue... - */ -struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, - u16 dest_cport_id, - unsigned int size, - gfp_t gfp_mask) -{ - return kmem_cache_zalloc(gbuf_head_cache, gfp_mask); -} -EXPORT_SYMBOL_GPL(greybus_alloc_gbuf); - -void greybus_free_gbuf(struct gbuf *gbuf) -{ - kmem_cache_free(gbuf_head_cache, gbuf); -} -EXPORT_SYMBOL_GPL(greybus_free_gbuf); - int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask) { gbuf->status = -EINPROGRESS; @@ -77,16 +45,3 @@ void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, gb_connection_operation_recv(connection, data, length); } EXPORT_SYMBOL_GPL(greybus_cport_in); - -int gb_gbuf_init(void) -{ - gbuf_head_cache = kmem_cache_create("gbuf_head_cache", - sizeof(struct gbuf), 0, 0, NULL); - return 0; -} - -void gb_gbuf_exit(void) -{ - kmem_cache_destroy(gbuf_head_cache); - gbuf_head_cache = NULL; -} diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index e1f918d50df2..30d5625ea4ff 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -105,12 +105,6 @@ void greybus_remove_hd(struct greybus_host_device *hd); void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, u8 *data, size_t length); -struct gbuf *greybus_alloc_gbuf(struct greybus_host_device *hd, - u16 dest_cport_id, - unsigned int size, - gfp_t gfp_mask); -void greybus_free_gbuf(struct gbuf *gbuf); - int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t mem_flags); void greybus_kill_gbuf(struct gbuf *gbuf); From 374e6a269cc3b1f044be78215c3e96021796de7d Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 18:08:37 -0600 Subject: [PATCH 10/10] greybus: kill off the last of gbuf.c Only three functions remain in "gbuf.c". Move one of them into "connection.c" and the other two into "operation.c". Some more cleanup is coming that will further straighten out gbufs but for now there's no sense in drawing this out any longer. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/Makefile | 1 - drivers/staging/greybus/connection.c | 15 +++++++++ drivers/staging/greybus/connection.h | 2 ++ drivers/staging/greybus/gbuf.c | 47 ---------------------------- drivers/staging/greybus/greybus.h | 6 ---- drivers/staging/greybus/operation.c | 14 +++++++++ 6 files changed, 31 insertions(+), 54 deletions(-) delete mode 100644 drivers/staging/greybus/gbuf.c diff --git a/drivers/staging/greybus/Makefile b/drivers/staging/greybus/Makefile index 7ec70fe6bd5d..c19fc84a724b 100644 --- a/drivers/staging/greybus/Makefile +++ b/drivers/staging/greybus/Makefile @@ -1,5 +1,4 @@ greybus-y := core.o \ - gbuf.o \ sysfs.o \ debugfs.o \ ap.o \ diff --git a/drivers/staging/greybus/connection.c b/drivers/staging/greybus/connection.c index cb6e2e1c085c..258d96cdba67 100644 --- a/drivers/staging/greybus/connection.c +++ b/drivers/staging/greybus/connection.c @@ -29,6 +29,21 @@ struct gb_connection *gb_hd_connection_find(struct greybus_host_device *hd, return connection; } +void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, + u8 *data, size_t length) +{ + struct gb_connection *connection; + + connection = gb_hd_connection_find(hd, cport_id); + if (!connection) { + dev_err(hd->parent, + "nonexistent connection (%zu bytes dropped)\n", length); + return; + } + gb_connection_operation_recv(connection, data, length); +} +EXPORT_SYMBOL_GPL(greybus_cport_in); + /* * Allocate an available CPort Id for use for the host side of the * given connection. The lowest-available id is returned, so the diff --git a/drivers/staging/greybus/connection.h b/drivers/staging/greybus/connection.h index 5e969672b793..bcaad47aaa03 100644 --- a/drivers/staging/greybus/connection.h +++ b/drivers/staging/greybus/connection.h @@ -53,6 +53,8 @@ void gb_connection_exit(struct gb_connection *connection); struct gb_connection *gb_hd_connection_find(struct greybus_host_device *hd, u16 cport_id); +void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, + u8 *data, size_t length); __printf(2, 3) void gb_connection_err(struct gb_connection *connection, const char *fmt, ...); diff --git a/drivers/staging/greybus/gbuf.c b/drivers/staging/greybus/gbuf.c deleted file mode 100644 index d47cf367e412..000000000000 --- a/drivers/staging/greybus/gbuf.c +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Greybus gbuf handling - * - * Copyright 2014 Google Inc. - * - * Released under the GPLv2 only. - */ - -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - -#include -#include -#include -#include -#include - -#include "greybus.h" - -int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t gfp_mask) -{ - gbuf->status = -EINPROGRESS; - - return gbuf->hd->driver->submit_gbuf(gbuf, gfp_mask); -} - -void greybus_kill_gbuf(struct gbuf *gbuf) -{ - if (gbuf->status != -EINPROGRESS) - return; - - gbuf->hd->driver->kill_gbuf(gbuf); -} - -void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, - u8 *data, size_t length) -{ - struct gb_connection *connection; - - connection = gb_hd_connection_find(hd, cport_id); - if (!connection) { - dev_err(hd->parent, - "nonexistent connection (%zu bytes dropped)\n", length); - return; - } - gb_connection_operation_recv(connection, data, length); -} -EXPORT_SYMBOL_GPL(greybus_cport_in); diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index 30d5625ea4ff..301bd4598c11 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -102,12 +102,6 @@ struct greybus_host_device { struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *hd, struct device *parent); void greybus_remove_hd(struct greybus_host_device *hd); -void greybus_cport_in(struct greybus_host_device *hd, u16 cport_id, - u8 *data, size_t length); - -int greybus_submit_gbuf(struct gbuf *gbuf, gfp_t mem_flags); -void greybus_kill_gbuf(struct gbuf *gbuf); - struct greybus_driver { const char *name; diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index 7eaea71f8604..223988327795 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -103,6 +103,20 @@ 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) +{ + gbuf->status = -EINPROGRESS; + + return gbuf->hd->driver->submit_gbuf(gbuf, gfp_mask); +} + +static void greybus_kill_gbuf(struct gbuf *gbuf) +{ + if (gbuf->status != -EINPROGRESS) + return; + + gbuf->hd->driver->kill_gbuf(gbuf); +} /* * An operations's response message has arrived. If no callback was * supplied it was submitted for asynchronous completion, so we notify