From c08b1ddaeb6e8f3c22b15f80e7475c809490a716 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 20 Nov 2014 16:09:15 -0600 Subject: [PATCH] greybus: dynamically allocate requests and responses Have an operation's request and response messages be dynamically allocated rather than embedded in an operation. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/battery-gb.c | 2 +- drivers/staging/greybus/gpio-gb.c | 24 +++++----- drivers/staging/greybus/i2c-gb.c | 12 ++--- drivers/staging/greybus/operation.c | 89 +++++++++++++++++------------------ drivers/staging/greybus/operation.h | 4 +- drivers/staging/greybus/pwm-gb.c | 16 +++---- drivers/staging/greybus/uart-gb.c | 10 ++-- drivers/staging/greybus/vibrator-gb.c | 4 +- 8 files changed, 78 insertions(+), 83 deletions(-) diff --git a/drivers/staging/greybus/battery-gb.c b/drivers/staging/greybus/battery-gb.c index 1c85341..02178f5 100644 --- a/drivers/staging/greybus/battery-gb.c +++ b/drivers/staging/greybus/battery-gb.c @@ -119,7 +119,7 @@ static int battery_operation(struct gb_battery *gb, int type, operation->result); } else { /* Good response, so copy to the caller's buffer */ - memcpy(response, operation->response.payload, response_size); + memcpy(response, operation->response->payload, response_size); } out: gb_operation_destroy(operation); diff --git a/drivers/staging/greybus/gpio-gb.c b/drivers/staging/greybus/gpio-gb.c index f2e2eef..170f8aa 100644 --- a/drivers/staging/greybus/gpio-gb.c +++ b/drivers/staging/greybus/gpio-gb.c @@ -142,7 +142,7 @@ static int gb_gpio_proto_version_operation(struct gb_gpio_controller *gb_gpio_co gb_connection_err(connection, "version result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; if (response->major > GB_GPIO_VERSION_MAJOR) { pr_err("unsupported major version (%hhu > %hhu)\n", response->major, GB_GPIO_VERSION_MAJOR); @@ -188,7 +188,7 @@ static int gb_gpio_line_count_operation(struct gb_gpio_controller *gb_gpio_contr gb_connection_err(connection, "line count result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; gb_gpio_controller->line_max = response->count; pr_debug("%s: count = %u\n", __func__, @@ -217,7 +217,7 @@ static int gb_gpio_activate_operation(struct gb_gpio_controller *gb_gpio_control sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -259,7 +259,7 @@ static int gb_gpio_deactivate_operation(struct gb_gpio_controller *gb_gpio_contr sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -300,7 +300,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 */ @@ -317,7 +317,7 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *gb_gpio_co } else { u8 direction; - response = operation->response.payload; + response = operation->response->payload; direction = response->direction; if (direction && direction != 1) pr_warn("gpio %u direction was %u (should be 0 or 1)\n", @@ -349,7 +349,7 @@ static int gb_gpio_direction_in_operation(struct gb_gpio_controller *gb_gpio_con sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -390,7 +390,7 @@ static int gb_gpio_direction_out_operation(struct gb_gpio_controller *gb_gpio_co sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; request->value = value_high ? 1 : 0; @@ -433,7 +433,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 */ @@ -450,7 +450,7 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *gb_gpio_contro } else { u8 value; - response = operation->response.payload; + response = operation->response->payload; value = response->value; if (value && value != 1) pr_warn("gpio %u value was %u (should be 0 or 1)\n", @@ -484,7 +484,7 @@ static int gb_gpio_set_value_operation(struct gb_gpio_controller *gb_gpio_contro sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; request->value = value_high ? 1 : 0; @@ -529,7 +529,7 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *gb_gpio_con sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; request->usec = cpu_to_le16(debounce_usec); diff --git a/drivers/staging/greybus/i2c-gb.c b/drivers/staging/greybus/i2c-gb.c index 9a090f4..2a5fb82 100644 --- a/drivers/staging/greybus/i2c-gb.c +++ b/drivers/staging/greybus/i2c-gb.c @@ -116,7 +116,7 @@ static int gb_i2c_proto_version_operation(struct gb_i2c_device *gb_i2c_dev) gb_connection_err(connection, "version result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; if (response->major > GB_I2C_VERSION_MAJOR) { pr_err("unsupported major version (%hhu > %hhu)\n", response->major, GB_I2C_VERSION_MAJOR); @@ -168,7 +168,7 @@ static int gb_i2c_functionality_operation(struct gb_i2c_device *gb_i2c_dev) gb_connection_err(connection, "functionality result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; functionality = le32_to_cpu(response->functionality); gb_i2c_dev->functionality = gb_i2c_functionality_map(functionality); @@ -190,7 +190,7 @@ static int gb_i2c_timeout_operation(struct gb_i2c_device *gb_i2c_dev, u16 msec) sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->msec = cpu_to_le16(msec); /* Synchronous operation--no callback */ @@ -225,7 +225,7 @@ static int gb_i2c_retries_operation(struct gb_i2c_device *gb_i2c_dev, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->retries = retries; /* Synchronous operation--no callback */ @@ -310,7 +310,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]; @@ -376,7 +376,7 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev, operation->result); } } else { - response = operation->response.payload; + response = operation->response->payload; gb_i2c_transfer_response(msgs, msg_count, response->data); ret = msg_count; } diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index f6940a3..b02c5314 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -81,7 +81,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.header; + header = operation->request->header; header->operation_id = cpu_to_le16(operation->id); } @@ -167,7 +167,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) - gb_message_cancel(&operation->request); + gb_message_cancel(operation->request); return ret; } @@ -177,7 +177,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.header; + header = operation->request->header; /* * If the protocol has no incoming request handler, report @@ -205,7 +205,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.header == NULL; + incoming_request = operation->response->header == NULL; if (incoming_request) gb_operation_request_handle(operation); gb_operation_complete(operation); @@ -251,61 +251,53 @@ gb_buffer_free(struct greybus_host_device *hd, void *buffer) } /* - * Allocate a buffer to be used for an operation request or response - * message. For outgoing messages, both types of message contain a + * Allocate a message to be used for an operation request or + * response. For outgoing messages, both types of message contain a * common header, which is filled in here. Incoming requests or * responses also contain the same header, but there's no need to * initialize it here (it'll be overwritten by the incoming * message). */ -static int gb_operation_message_init(struct gb_operation *operation, - u8 type, size_t size, - bool request, gfp_t gfp_flags) +static struct gb_message * +gb_operation_message_alloc(struct greybus_host_device *hd, u8 type, + size_t size, gfp_t gfp_flags) { - struct gb_connection *connection = operation->connection; - struct greybus_host_device *hd = connection->hd; struct gb_message *message; struct gb_operation_msg_hdr *header; size += sizeof(*header); if (size > hd->buffer_size_max) - return -E2BIG; + return NULL; - if (request) { - message = &operation->request; - } else { - message = &operation->response; - type |= GB_OPERATION_TYPE_RESPONSE; - } + message = kzalloc(sizeof(*message), gfp_flags); + if (!message) + return NULL; - message->header = gb_buffer_alloc(hd, size, gfp_flags); - if (!message->header) - return -ENOMEM; - message->size = size; + header = gb_buffer_alloc(hd, size, gfp_flags); + if (!header) { + kfree(message); + return NULL; + } /* Fill in the header structure */ - header = message->header; header->size = cpu_to_le16(size); header->operation_id = 0; /* Filled in when submitted */ header->type = type; + message->header = header; message->payload = header + 1; - message->operation = operation; + message->size = size; - return 0; + return message; } -static void gb_operation_message_exit(struct gb_message *message) +static void gb_operation_message_free(struct gb_message *message) { struct greybus_host_device *hd; hd = message->operation->connection->hd; gb_buffer_free(hd, message->header); - - message->operation = NULL; - message->payload = NULL; - message->header = NULL; - message->size = 0; + kfree(message); } /* @@ -358,25 +350,28 @@ gb_operation_create_common(struct gb_connection *connection, bool outgoing, u8 type, size_t request_size, size_t response_size) { + struct greybus_host_device *hd = connection->hd; struct gb_operation *operation; gfp_t gfp_flags = response_size ? GFP_KERNEL : GFP_ATOMIC; - int ret; operation = kmem_cache_zalloc(gb_operation_cache, gfp_flags); if (!operation) return NULL; operation->connection = connection; - ret = gb_operation_message_init(operation, type, request_size, - true, gfp_flags); - if (ret) + operation->request = gb_operation_message_alloc(hd, type, request_size, + gfp_flags); + if (!operation->request) goto err_cache; + operation->request->operation = operation; if (outgoing) { - ret = gb_operation_message_init(operation, type, response_size, - false, GFP_KERNEL); - if (ret) + type |= GB_OPERATION_TYPE_RESPONSE; + operation->response = gb_operation_message_alloc(hd, type, + response_size, GFP_KERNEL); + if (!operation->response) goto err_request; + operation->response->operation = operation; } INIT_WORK(&operation->recv_work, gb_operation_recv_work); @@ -392,7 +387,7 @@ gb_operation_create_common(struct gb_connection *connection, bool outgoing, return operation; err_request: - gb_operation_message_exit(&operation->request); + gb_operation_message_free(operation->request); err_cache: kmem_cache_free(gb_operation_cache, operation); @@ -430,8 +425,8 @@ static void _gb_operation_destroy(struct kref *kref) list_del(&operation->links); spin_unlock_irq(&gb_operations_lock); - gb_operation_message_exit(&operation->response); - gb_operation_message_exit(&operation->request); + gb_operation_message_free(operation->response); + gb_operation_message_free(operation->request); kmem_cache_free(gb_operation_cache, operation); } @@ -478,7 +473,7 @@ int gb_operation_request_send(struct gb_operation *operation, schedule_delayed_work(&operation->timeout_work, timeout); /* All set, send the request */ - ret = gb_message_send(&operation->request, GFP_KERNEL); + ret = gb_message_send(operation->request, GFP_KERNEL); if (ret) return ret; @@ -516,7 +511,7 @@ void gb_connection_recv_request(struct gb_connection *connection, return; /* XXX Respond with pre-allocated ENOMEM */ } operation->id = operation_id; - memcpy(operation->request.header, data, size); + memcpy(operation->request->header, data, size); /* The rest will be handled in work queue context */ queue_work(gb_operation_recv_workqueue, &operation->recv_work); @@ -546,7 +541,7 @@ static void gb_connection_recv_response(struct gb_connection *connection, cancel_delayed_work(&operation->timeout_work); gb_pending_operation_remove(operation); - message = &operation->response; + message = operation->response; if (size <= message->size) { /* Transfer the operation result from the response header */ header = message->header; @@ -609,9 +604,9 @@ void gb_connection_recv(struct gb_connection *connection, void gb_operation_cancel(struct gb_operation *operation) { operation->canceled = true; - gb_message_cancel(&operation->request); - if (operation->response.header) - gb_message_cancel(&operation->response); + gb_message_cancel(operation->request); + if (operation->response->header) + gb_message_cancel(operation->response); } int gb_operation_init(void) diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index 38b2833..567bb70 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -64,8 +64,8 @@ struct gb_message { typedef void (*gb_operation_callback)(struct gb_operation *); struct gb_operation { struct gb_connection *connection; - struct gb_message request; - struct gb_message response; + struct gb_message *request; + struct gb_message *response; u16 id; bool canceled; diff --git a/drivers/staging/greybus/pwm-gb.c b/drivers/staging/greybus/pwm-gb.c index c92d8e2..d3d39be 100644 --- a/drivers/staging/greybus/pwm-gb.c +++ b/drivers/staging/greybus/pwm-gb.c @@ -109,7 +109,7 @@ static int gb_pwm_proto_version_operation(struct gb_pwm_chip *pwmc) gb_connection_err(connection, "version result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; if (response->major > GB_PWM_VERSION_MAJOR) { pr_err("unsupported major version (%hhu > %hhu)\n", response->major, GB_PWM_VERSION_MAJOR); @@ -150,7 +150,7 @@ static int gb_pwm_count_operation(struct gb_pwm_chip *pwmc) gb_connection_err(connection, "pwm count result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; pwmc->pwm_max = response->count; } out: @@ -175,7 +175,7 @@ static int gb_pwm_activate_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -212,7 +212,7 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -248,7 +248,7 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; request->duty = duty; request->period = period; @@ -287,7 +287,7 @@ static int gb_pwm_set_polarity_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; request->polarity = polarity; @@ -325,7 +325,7 @@ static int gb_pwm_enable_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ @@ -362,7 +362,7 @@ static int gb_pwm_disable_operation(struct gb_pwm_chip *pwmc, sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->which = which; /* Synchronous operation--no callback */ diff --git a/drivers/staging/greybus/uart-gb.c b/drivers/staging/greybus/uart-gb.c index 8df3bfb..7d7e223 100644 --- a/drivers/staging/greybus/uart-gb.c +++ b/drivers/staging/greybus/uart-gb.c @@ -159,7 +159,7 @@ static int get_version(struct gb_tty *tty) gb_connection_err(tty->connection, "result %hhu", operation->result); } else { - response = operation->response.payload; + response = operation->response->payload; if (response->major > GB_UART_VERSION_MAJOR) { pr_err("unsupported major version (%hhu > %hhu)\n", response->major, GB_UART_VERSION_MAJOR); @@ -192,7 +192,7 @@ static int send_data(struct gb_tty *tty, u16 size, const u8 *data) sizeof(*request) + size, 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->size = cpu_to_le16(size); memcpy(&request->data[0], data, size); @@ -227,7 +227,7 @@ static int send_line_coding(struct gb_tty *tty, sizeof(*request), 0); 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 */ @@ -261,7 +261,7 @@ static int send_control(struct gb_tty *tty, u16 control) sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->control = cpu_to_le16(control); /* Synchronous operation--no callback */ @@ -299,7 +299,7 @@ static int send_break(struct gb_tty *tty, u8 state) sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->state = state; /* Synchronous operation--no callback */ diff --git a/drivers/staging/greybus/vibrator-gb.c b/drivers/staging/greybus/vibrator-gb.c index 9ad3cb0..b974973 100644 --- a/drivers/staging/greybus/vibrator-gb.c +++ b/drivers/staging/greybus/vibrator-gb.c @@ -71,7 +71,7 @@ static int request_operation(struct gb_connection *connection, int type, } else { /* Good request, so copy to the caller's buffer */ if (response_size && response) - memcpy(response, operation->response.payload, + memcpy(response, operation->response->payload, response_size); } out: @@ -119,7 +119,7 @@ static int turn_on(struct gb_vibrator_device *vib, u16 timeout_ms) sizeof(*request), 0); if (!operation) return -ENOMEM; - request = operation->request.payload; + request = operation->request->payload; request->timeout_ms = cpu_to_le16(timeout_ms); /* Synchronous operation--no callback */ -- 2.7.4