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 <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

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;

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; */

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;

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 */

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 */

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;

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;

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;