From: Alex Elder Date: Sat, 22 Nov 2014 01:29:12 +0000 (-0600) Subject: greybus: use errno for operation result X-Git-Tag: v4.9-rc1~119^2~378^2~21^2~1836 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=23383defa8395eb34d02dd4d4fc1d95e95c5603d;p=karo-tx-linux.git greybus: use errno for operation result An in-core operation structure tracks the progress of an operation. Currently it holds a result field that was intended to take the status value that arrives in an operation response message header. But operations can fail for reasons other than that, and it's inconvenient to try to represent those using the operation status codes. So change the operation->result field to be an int, and switch to storing negative errno values in it. Rename it "errno" to make it obvious how to interpret the value. This patch makes another change, which simplifies the protocol drivers a lot. It's being done as part of this patch because it affects all the same code as the above change does. If desired I can split this into two separate patches. If a caller makes a synchronous gb_operation_request_send() request (i.e., no callback function is supplied), and the operation request and response messages were transferred successfully, have gb_operation_request_send() return the result of the request (i.e., operation->errno). This allows the caller (or more generally, any caller of gb_request_wait() to avoid having to look at this field for every successful send. Any caller that does an asynchronous request will of course need to look at request->errno in the callback function to see the result of the operation. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/staging/greybus/battery-gb.c b/drivers/staging/greybus/battery-gb.c index 13ade437d63c..ae03869373d8 100644 --- a/drivers/staging/greybus/battery-gb.c +++ b/drivers/staging/greybus/battery-gb.c @@ -88,7 +88,7 @@ struct gb_battery_voltage_response { * None of the battery operation requests have any payload. This * function implements all of the requests by allowing the caller to * supply a buffer into which the operation response should be - * copied. + * copied. If there is an error, the response buffer is left alone. */ static int battery_operation(struct gb_battery *gb, int type, void *response, int response_size) @@ -103,25 +103,10 @@ static int battery_operation(struct gb_battery *gb, int type, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("version operation failed (%d)\n", ret); - goto out; - } - - /* - * We only want to look at the status, and all requests have the same - * layout for where the status is, so cast this to a random request so - * we can see the status easier. - */ - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "operation result %hhu", - operation->result); - } else { - /* Good response, so copy to the caller's buffer */ + else /* Good response, so copy to the caller's buffer */ memcpy(response, operation->response->payload, response_size); - } -out: gb_operation_destroy(operation); return ret; diff --git a/drivers/staging/greybus/gpio-gb.c b/drivers/staging/greybus/gpio-gb.c index 170f8aa289ca..c6c85d8c28d2 100644 --- a/drivers/staging/greybus/gpio-gb.c +++ b/drivers/staging/greybus/gpio-gb.c @@ -116,7 +116,8 @@ struct gb_gpio_set_debounce_request { * This request only uses the connection field, and if successful, * fills in the major and minor protocol version of the target. */ -static int gb_gpio_proto_version_operation(struct gb_gpio_controller *gb_gpio_controller) +static int +gb_gpio_proto_version_operation(struct gb_gpio_controller *gb_gpio_controller) { struct gb_connection *connection = gb_gpio_controller->connection; struct gb_operation *operation; @@ -137,24 +138,14 @@ static int gb_gpio_proto_version_operation(struct gb_gpio_controller *gb_gpio_co goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "version result %hhu", - operation->result); + 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); + ret = -ENOTSUPP; } else { - 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); - ret = -ENOTSUPP; - goto out; - } gb_gpio_controller->version_major = response->major; gb_gpio_controller->version_minor = response->minor; - - pr_debug("%s: version_major = %u version_minor = %u\n", __func__, - gb_gpio_controller->version_major, - gb_gpio_controller->version_minor); } out: gb_operation_destroy(operation); @@ -180,21 +171,10 @@ static int gb_gpio_line_count_operation(struct gb_gpio_controller *gb_gpio_contr ret = gb_operation_request_send(operation, NULL); if (ret) { pr_err("line count operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "line count result %hhu", - operation->result); } else { response = operation->response->payload; gb_gpio_controller->line_max = response->count; - - pr_debug("%s: count = %u\n", __func__, - gb_gpio_controller->line_max + 1); } -out: gb_operation_destroy(operation); return ret; @@ -222,21 +202,10 @@ static int gb_gpio_activate_operation(struct gb_gpio_controller *gb_gpio_control /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("activate operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "activate result %hhu", - operation->result); - } else { + else gb_gpio_controller->lines[which].active = true; - - pr_debug("%s: %u is now active\n", __func__, which); - } -out: gb_operation_destroy(operation); return ret; @@ -264,20 +233,10 @@ static int gb_gpio_deactivate_operation(struct gb_gpio_controller *gb_gpio_contr /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("deactivate operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "deactivate result %hhu", - operation->result); - } else { + else gb_gpio_controller->lines[which].active = false; - pr_debug("%s: %u is now inactive\n", __func__, which); - } -out: gb_operation_destroy(operation); return ret; @@ -291,6 +250,7 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *gb_gpio_co struct gb_gpio_get_direction_request *request; struct gb_gpio_get_direction_response *response; int ret; + u8 direction; if (which > gb_gpio_controller->line_max) return -EINVAL; @@ -310,22 +270,12 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *gb_gpio_co goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "get direction result %hhu", - operation->result); - } else { - u8 direction; - - response = operation->response->payload; - direction = response->direction; - if (direction && direction != 1) - pr_warn("gpio %u direction was %u (should be 0 or 1)\n", - which, direction); - gb_gpio_controller->lines[which].direction = direction ? 1 : 0; - pr_debug("%s: direction of %u is %s\n", __func__, which, - direction ? "in" : "out"); - } + response = operation->response->payload; + direction = response->direction; + if (direction && direction != 1) + pr_warn("gpio %u direction was %u (should be 0 or 1)\n", + which, direction); + gb_gpio_controller->lines[which].direction = direction ? 1 : 0; out: gb_operation_destroy(operation); @@ -354,20 +304,10 @@ static int gb_gpio_direction_in_operation(struct gb_gpio_controller *gb_gpio_con /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("direction in operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "direction in result %hhu", - operation->result); - } else { + else gb_gpio_controller->lines[which].direction = 1; - pr_debug("%s: direction of %u is now in\n", __func__, which); - } -out: gb_operation_destroy(operation); return ret; @@ -396,21 +336,10 @@ static int gb_gpio_direction_out_operation(struct gb_gpio_controller *gb_gpio_co /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("direction out operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "direction out result %hhu", - operation->result); - } else { + else gb_gpio_controller->lines[which].direction = 0; - pr_debug("%s: direction of %u is now out, value %s\n", __func__, - which, value_high ? "high" : "low"); - } -out: gb_operation_destroy(operation); return ret; @@ -424,6 +353,7 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *gb_gpio_contro struct gb_gpio_get_value_request *request; struct gb_gpio_get_value_response *response; int ret; + u8 value; if (which > gb_gpio_controller->line_max) return -EINVAL; @@ -443,24 +373,12 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *gb_gpio_contro goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "get value result %hhu", - operation->result); - } else { - u8 value; - - response = operation->response->payload; - value = response->value; - if (value && value != 1) - pr_warn("gpio %u value was %u (should be 0 or 1)\n", - which, value); - gb_gpio_controller->lines[which].value = value ? 1 : 0; - /* XXX should this set direction to out? */ - pr_debug("%s: value of %u is %s\n", __func__, which, - gb_gpio_controller->lines[which].value ? "high" : - "low"); - } + response = operation->response->payload; + value = response->value; + if (value && value != 1) + pr_warn("gpio %u value was %u (should be 0 or 1)\n", + which, value); + gb_gpio_controller->lines[which].value = value ? 1 : 0; out: gb_operation_destroy(operation); @@ -490,23 +408,10 @@ static int gb_gpio_set_value_operation(struct gb_gpio_controller *gb_gpio_contro /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("set value operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "set value result %hhu", - operation->result); - } else { - /* XXX should this set direction to out? */ + else /* XXX should this set direction to out? */ gb_gpio_controller->lines[which].value = request->value; - pr_debug("%s: out value of %u is now %s\n", __func__, which, - gb_gpio_controller->lines[which].value ? "high" : - "low"); - } -out: gb_operation_destroy(operation); return ret; @@ -535,21 +440,11 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *gb_gpio_con /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("set debounce operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "set debounce result %hhu", - operation->result); - } else { - gb_gpio_controller->lines[which].debounce_usec = le16_to_cpu(request->usec); - pr_debug("%s: debounce of %u is now %hu usec\n", __func__, which, - gb_gpio_controller->lines[which].debounce_usec); - } -out: + else + gb_gpio_controller->lines[which].debounce_usec = + le16_to_cpu(request->usec); gb_operation_destroy(operation); return ret; @@ -562,7 +457,6 @@ static int gb_gpio_request(struct gpio_chip *chip, unsigned offset) if (offset < 0 || offset >= chip->ngpio) return -EINVAL; - pr_debug("%s: passed check\n", __func__); ret = gb_gpio_activate_operation(gb_gpio_controller, (u8)offset); if (ret) ; /* return ret; */ diff --git a/drivers/staging/greybus/i2c-gb.c b/drivers/staging/greybus/i2c-gb.c index 2a5fb822535e..6b2fd7e934e3 100644 --- a/drivers/staging/greybus/i2c-gb.c +++ b/drivers/staging/greybus/i2c-gb.c @@ -111,23 +111,16 @@ static int gb_i2c_proto_version_operation(struct gb_i2c_device *gb_i2c_dev) goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "version result %hhu", - operation->result); - } else { - 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); - ret = -ENOTSUPP; - goto out; - } - gb_i2c_dev->version_major = response->major; - gb_i2c_dev->version_minor = response->minor; + 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); + ret = -ENOTSUPP; + goto out; } + gb_i2c_dev->version_major = response->major; + gb_i2c_dev->version_minor = response->minor; out: - gb_operation_destroy(operation); return ret; @@ -163,16 +156,9 @@ static int gb_i2c_functionality_operation(struct gb_i2c_device *gb_i2c_dev) goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "functionality result %hhu", - operation->result); - } else { - response = operation->response->payload; - functionality = le32_to_cpu(response->functionality); - gb_i2c_dev->functionality = - gb_i2c_functionality_map(functionality); - } + response = operation->response->payload; + functionality = le32_to_cpu(response->functionality); + gb_i2c_dev->functionality = gb_i2c_functionality_map(functionality); out: gb_operation_destroy(operation); @@ -195,19 +181,10 @@ static int gb_i2c_timeout_operation(struct gb_i2c_device *gb_i2c_dev, u16 msec) /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("timeout operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "timeout result %hhu", - operation->result); - } else { + else gb_i2c_dev->timeout_msec = msec; - } -out: gb_operation_destroy(operation); return ret; @@ -230,19 +207,10 @@ static int gb_i2c_retries_operation(struct gb_i2c_device *gb_i2c_dev, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("retries operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "retries result %hhu", - operation->result); - } else { + else gb_i2c_dev->retries = retries; - } -out: gb_operation_destroy(operation); return ret; @@ -365,22 +333,13 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); if (ret) { - pr_err("transfer operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - if (ret != -EAGAIN) { - gb_connection_err(connection, "transfer result %hhu", - operation->result); - } + if (ret != -EAGAIN) + pr_err("transfer operation failed (%d)\n", ret); } else { response = operation->response->payload; gb_i2c_transfer_response(msgs, msg_count, response->data); ret = msg_count; } -out: gb_operation_destroy(operation); return ret; diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index 442046562bfb..5ab1c47f5521 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -159,7 +159,11 @@ static void gb_operation_complete(struct gb_operation *operation) complete_all(&operation->completion); } -/* Wait for a submitted operation to complete */ +/* + * Wait for a submitted operation to complete. Returns -RESTARTSYS + * if the wait was interrupted. Otherwise returns the result of the + * operation. + */ int gb_operation_wait(struct gb_operation *operation) { int ret; @@ -168,6 +172,8 @@ int gb_operation_wait(struct gb_operation *operation) /* If interrupted, cancel the in-flight buffer */ if (ret < 0) gb_message_cancel(operation->request); + else + ret = operation->errno; return ret; } @@ -189,7 +195,7 @@ static void gb_operation_request_handle(struct gb_operation *operation) gb_connection_err(operation->connection, "unexpected incoming request type 0x%02hhx\n", header->type); - operation->result = GB_OP_PROTOCOL_BAD; + operation->errno = -EPROTONOSUPPORT; } /* @@ -224,7 +230,7 @@ static void operation_timeout(struct work_struct *work) operation = container_of(work, struct gb_operation, timeout_work.work); pr_debug("%s: timeout!\n", __func__); - operation->result = GB_OP_TIMEOUT; + operation->errno = -ETIMEDOUT; gb_operation_complete(operation); } @@ -470,13 +476,10 @@ int gb_operation_request_send(struct gb_operation *operation, /* All set, send the request */ ret = gb_message_send(operation->request, GFP_KERNEL); - if (ret) + if (ret || callback) return ret; - if (!callback) - ret = gb_operation_wait(operation); - - return ret; + return gb_operation_wait(operation); } /* @@ -493,8 +496,6 @@ int gb_operation_response_send(struct gb_operation *operation) * This function is called when a buffer send request has completed. * The "header" is the message header--the beginning of what we * asked to have sent. - * - * XXX Mismatch between errno here and operation result code */ void greybus_data_sent(struct greybus_host_device *hd, void *header, int status) @@ -510,7 +511,7 @@ greybus_data_sent(struct greybus_host_device *hd, void *header, int status) message = gb_hd_message_find(hd, header); operation = message->operation; gb_connection_err(operation->connection, "send error %d\n", status); - operation->result = status; /* XXX */ + operation->errno = status; gb_operation_complete(operation); } EXPORT_SYMBOL_GPL(greybus_data_sent); @@ -567,14 +568,14 @@ static void gb_connection_recv_response(struct gb_connection *connection, if (size <= message->size) { /* Transfer the operation result from the response header */ header = message->header; - operation->result = header->result; + operation->errno = gb_operation_status_map(header->result); } else { gb_connection_err(connection, "recv buffer too small"); - operation->result = GB_OP_OVERFLOW; + operation->errno = -E2BIG; } /* We must ignore the payload if a bad status is returned */ - if (operation->result == GB_OP_SUCCESS) + if (!operation->errno) memcpy(message->header, data, size); /* The rest will be handled in work queue context */ diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index 3e5e1f570f3e..33d50039327b 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -13,7 +13,7 @@ struct gb_operation; -enum gb_operation_status { +enum gb_operation_result { GB_OP_SUCCESS = 0, GB_OP_INVALID = 1, GB_OP_NO_MEMORY = 2, @@ -71,7 +71,8 @@ struct gb_operation { u16 id; bool canceled; - u8 result; + int errno; /* Operation result */ + struct work_struct recv_work; gb_operation_callback callback; /* If asynchronous */ struct completion completion; /* Used if no callback */ diff --git a/drivers/staging/greybus/pwm-gb.c b/drivers/staging/greybus/pwm-gb.c index d3d39be59201..e146b240f89e 100644 --- a/drivers/staging/greybus/pwm-gb.c +++ b/drivers/staging/greybus/pwm-gb.c @@ -104,21 +104,15 @@ static int gb_pwm_proto_version_operation(struct gb_pwm_chip *pwmc) goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "version result %hhu", - operation->result); - } else { - 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); - ret = -ENOTSUPP; - goto out; - } - pwmc->version_major = response->major; - pwmc->version_minor = response->minor; + 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); + ret = -ENOTSUPP; + goto out; } + pwmc->version_major = response->major; + pwmc->version_minor = response->minor; out: gb_operation_destroy(operation); @@ -142,18 +136,10 @@ static int gb_pwm_count_operation(struct gb_pwm_chip *pwmc) ret = gb_operation_request_send(operation, NULL); if (ret) { pr_err("line count operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "pwm count result %hhu", - operation->result); } else { response = operation->response->payload; pwmc->pwm_max = response->count; } -out: gb_operation_destroy(operation); return ret; @@ -180,17 +166,8 @@ static int gb_pwm_activate_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("activate operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "activate result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; @@ -217,17 +194,8 @@ static int gb_pwm_deactivate_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("deactivate operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "deactivate result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; @@ -255,17 +223,8 @@ static int gb_pwm_config_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("config operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "config result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; @@ -293,17 +252,8 @@ static int gb_pwm_set_polarity_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("set polarity operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "set polarity result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; @@ -330,17 +280,8 @@ static int gb_pwm_enable_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("enable operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "enable result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; @@ -367,17 +308,8 @@ static int gb_pwm_disable_operation(struct gb_pwm_chip *pwmc, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("disable operation failed (%d)\n", ret); - goto out; - } - - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "disable result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return ret; diff --git a/drivers/staging/greybus/uart-gb.c b/drivers/staging/greybus/uart-gb.c index 48fad7b4c886..e9faf310e39c 100644 --- a/drivers/staging/greybus/uart-gb.c +++ b/drivers/staging/greybus/uart-gb.c @@ -154,24 +154,18 @@ static int get_version(struct gb_tty *tty) goto out; } - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(tty->connection, "result %hhu", - operation->result); - } else { - 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); - ret = -ENOTSUPP; - goto out; - } - tty->version_major = response->major; - tty->version_minor = response->minor; - - pr_debug("%s: version_major = %u version_minor = %u\n", - __func__, tty->version_major, tty->version_minor); + 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); + ret = -ENOTSUPP; + goto out; } + tty->version_major = response->major; + tty->version_minor = response->minor; + + pr_debug("%s: version_major = %u version_minor = %u\n", + __func__, tty->version_major, tty->version_minor); out: gb_operation_destroy(operation); @@ -198,18 +192,9 @@ static int send_data(struct gb_tty *tty, u16 size, const u8 *data) /* Synchronous operation--no callback */ retval = gb_operation_request_send(operation, NULL); - if (retval) { + if (retval) dev_err(&connection->dev, "send data operation failed (%d)\n", retval); - goto out; - } - - if (operation->result) { - retval = gb_operation_status_map(operation->result); - gb_connection_err(connection, "send data result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return retval; @@ -232,18 +217,9 @@ static int send_line_coding(struct gb_tty *tty, /* Synchronous operation--no callback */ retval = gb_operation_request_send(operation, NULL); - if (retval) { + if (retval) dev_err(&connection->dev, "send line coding operation failed (%d)\n", retval); - goto out; - } - - if (operation->result) { - retval = gb_operation_status_map(operation->result); - gb_connection_err(connection, "send line coding result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return retval; @@ -266,18 +242,9 @@ static int send_control(struct gb_tty *tty, u16 control) /* Synchronous operation--no callback */ retval = gb_operation_request_send(operation, NULL); - if (retval) { + if (retval) dev_err(&connection->dev, "send control operation failed (%d)\n", retval); - goto out; - } - - if (operation->result) { - retval = gb_operation_status_map(operation->result); - gb_connection_err(connection, "send control result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return retval; @@ -304,18 +271,9 @@ static int send_break(struct gb_tty *tty, u8 state) /* Synchronous operation--no callback */ retval = gb_operation_request_send(operation, NULL); - if (retval) { + if (retval) dev_err(&connection->dev, "send break operation failed (%d)\n", retval); - goto out; - } - - if (operation->result) { - retval = gb_operation_status_map(operation->result); - gb_connection_err(connection, "send break result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return retval; diff --git a/drivers/staging/greybus/vibrator-gb.c b/drivers/staging/greybus/vibrator-gb.c index b974973fc067..b65a586b87c9 100644 --- a/drivers/staging/greybus/vibrator-gb.c +++ b/drivers/staging/greybus/vibrator-gb.c @@ -42,6 +42,13 @@ struct gb_vibrator_on_request { __le16 timeout_ms; }; +/* + * The get_version and turn_off vibrator operations have no payload. + * This function implements these requests by allowing the caller to + * supply a buffer into which the operation response should be + * copied. The turn_off operation, there is no response either. + * If there is an error, the response buffer is left alone. + */ static int request_operation(struct gb_connection *connection, int type, void *response, int response_size) { @@ -54,27 +61,11 @@ static int request_operation(struct gb_connection *connection, int type, /* Synchronous operation--no callback */ ret = gb_operation_request_send(operation, NULL); - if (ret) { + if (ret) pr_err("version operation failed (%d)\n", ret); - goto out; - } - - /* - * We only want to look at the status, and all requests have the same - * layout for where the status is, so cast this to a random request so - * we can see the status easier. - */ - if (operation->result) { - ret = gb_operation_status_map(operation->result); - gb_connection_err(connection, "operation result %hhu", - operation->result); - } else { - /* Good request, so copy to the caller's buffer */ - if (response_size && response) - memcpy(response, operation->response->payload, - response_size); - } -out: + else if (response_size && response) + memcpy(response, operation->response->payload, + response_size); gb_operation_destroy(operation); return ret; @@ -124,18 +115,9 @@ static int turn_on(struct gb_vibrator_device *vib, u16 timeout_ms) /* Synchronous operation--no callback */ retval = gb_operation_request_send(operation, NULL); - if (retval) { + if (retval) dev_err(&connection->dev, "send data operation failed (%d)\n", retval); - goto out; - } - - if (operation->result) { - retval = gb_operation_status_map(operation->result); - gb_connection_err(connection, "send data result %hhu", - operation->result); - } -out: gb_operation_destroy(operation); return retval;