greybus: operation: refactor response handling

Send response to incoming requests from the operation request handler
rather than in every protocol request_recv callback.

This simplifies request_recv error handling and allows for further code
reuse.

Note that if we ever get protocols that need to hold off sending
responses we could implement this by letting them return a special
value (after acquiring the necessary operation references) to suppress
the response from being sent by greybus core.

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/staging/greybus/gpio.c

@@ -394,7 +394,7 @@ static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	return ret;
 }
 
-static void gb_gpio_request_recv(u8 type, struct gb_operation *op)
+static int gb_gpio_request_recv(u8 type, struct gb_operation *op)
 {
 	struct gb_connection *connection = op->connection;
 	struct gb_gpio_controller *ggc = connection->private;
@@ -402,41 +402,35 @@ static void gb_gpio_request_recv(u8 type, struct gb_operation *op)
 	struct gb_gpio_irq_event_request *event;
 	int irq;
 	struct irq_desc *desc;
-	int ret;
-	int status;
 
 	if (type != GB_GPIO_TYPE_IRQ_EVENT) {
 		dev_err(&connection->dev,
 			"unsupported unsolicited request: %u\n", type);
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	request = op->request;
 
 	if (request->payload_size < sizeof(*event)) {
 		dev_err(ggc->chip.dev, "short event received\n");
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	event = request->payload;
 	if (event->which > ggc->line_max) {
 		dev_err(ggc->chip.dev, "invalid hw irq: %d\n", event->which);
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
+
 	irq = gpio_to_irq(ggc->chip.base + event->which);
 	if (irq < 0) {
 		dev_err(ggc->chip.dev, "failed to map irq\n");
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 	desc = irq_to_desc(irq);
 	if (!desc) {
 		dev_err(ggc->chip.dev, "failed to look up irq\n");
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	/* Dispatch interrupt */
@@ -444,14 +438,7 @@ static void gb_gpio_request_recv(u8 type, struct gb_operation *op)
 	handle_simple_irq(irq, desc);
 	local_irq_enable();
 
-	status = 0;
-send_response:
-	ret = gb_operation_response_send(op, status);
-	if (ret) {
-		dev_err(ggc->chip.dev,
-			"failed to send response status %d: %d\n",
-			status, ret);
-	}
+	return 0;
 }
 
 static int gb_gpio_request(struct gpio_chip *chip, unsigned offset)

drivers/staging/greybus/hid.c

@@ -150,25 +150,22 @@ static int gb_hid_set_report(struct gb_hid *ghid, u8 report_type, u8 report_id,
 	return ret;
 }
 
-static void gb_hid_irq_handler(u8 type, struct gb_operation *op)
+static int gb_hid_irq_handler(u8 type, struct gb_operation *op)
 {
 	struct gb_connection *connection = op->connection;
 	struct gb_hid *ghid = connection->private;
 	struct gb_hid_input_report_request *request = op->request->payload;
-	int status;
-	int ret, size;
+	int size;
 
 	if (type != GB_HID_TYPE_IRQ_EVENT) {
 		dev_err(&connection->dev,
 			"unsupported unsolicited request\n");
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	if (op->request->payload_size < 2) {
 		dev_err(&connection->dev, "short report received\n");
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	/*
@@ -178,22 +175,14 @@ static void gb_hid_irq_handler(u8 type, struct gb_operation *op)
 	size = request->report[0] | request->report[1] << 8;
 	if (size < 2 || size > op->request->payload_size - 2) {
 		dev_err(&connection->dev, "bad report size: %d\n", size);
-		status = -EINVAL;
-		goto send_response;
+		return -EINVAL;
 	}
 
 	if (test_bit(GB_HID_STARTED, &ghid->flags))
 		hid_input_report(ghid->hid, HID_INPUT_REPORT,
 				 request->report + 2, size - 2, 1);
 
-	status = 0;
-send_response:
-	ret = gb_operation_response_send(op, status);
-	if (ret) {
-		dev_err(&connection->dev,
-			"failed to send response status %d: %d\n",
-			status, ret);
-	}
+	return 0;
 }
 
 

drivers/staging/greybus/operation.c

@@ -208,24 +208,27 @@ static void gb_message_cancel(struct gb_message *message)
 static void gb_operation_request_handle(struct gb_operation *operation)
 {
 	struct gb_protocol *protocol = operation->connection->protocol;
+	int status;
 	int ret;
 
 	if (!protocol)
 		return;
 
 	if (protocol->request_recv) {
-		protocol->request_recv(operation->type, operation);
-		return;
+		status = protocol->request_recv(operation->type, operation);
+	} else {
+		dev_err(&operation->connection->dev,
+			"unexpected incoming request type 0x%02hhx\n",
+			operation->type);
+
+		status = -EPROTONOSUPPORT;
 	}
 
-	dev_err(&operation->connection->dev,
-		"unexpected incoming request type 0x%02hhx\n", operation->type);
-
-	ret = gb_operation_response_send(operation, -EPROTONOSUPPORT);
+	ret = gb_operation_response_send(operation, status);
 	if (ret) {
 		dev_err(&operation->connection->dev,
 			"failed to send response %d: %d\n",
-			-EPROTONOSUPPORT, ret);
+			status, ret);
 			return;
 	}
 }

drivers/staging/greybus/protocol.h

@@ -22,7 +22,7 @@ struct gb_protocol_version_response {
 
 typedef int (*gb_connection_init_t)(struct gb_connection *);
 typedef void (*gb_connection_exit_t)(struct gb_connection *);
-typedef void (*gb_request_recv_t)(u8, struct gb_operation *);
+typedef int (*gb_request_recv_t)(u8, struct gb_operation *);
 
 /*
  * Protocols having the same id but different major and/or minor