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>

diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
index 9a34fc5a52b87d7af0017741bde30722c7c3d41b..20917bf6479f12d079668059c62d82bd139e8156 100644 (file)
--- a/drivers/staging/greybus/gpio.c
+++ b/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)

diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c
index cc5708ddf06820f7f593dbffe7ac1b19725ac2c5..5935aa6c6334e078d8dd3cd009088e0e2c151dc7 100644 (file)
--- a/drivers/staging/greybus/hid.c
+++ b/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;
 }
 
 

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 2b2b0d6332c8e415e0a24b5e1787d840fbd7985a..260774e414f371dba6919272aeb67334a48192ca 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/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);
 
-       dev_err(&operation->connection->dev,
-               "unexpected incoming request type 0x%02hhx\n", operation->type);
+               status = -EPROTONOSUPPORT;
+       }
 
-       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;
        }
 }

diff --git a/drivers/staging/greybus/protocol.h b/drivers/staging/greybus/protocol.h
index a74afef92a01c84002bfccb34e784d7ec902add8..82d9e81386e3eae5609e6d1c78c58a3451e5cbe3 100644 (file)
--- a/drivers/staging/greybus/protocol.h
+++ b/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