From 973ccfd62686a2331f43b0053de052d958f50d31 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Fri, 27 Mar 2015 12:45:49 +0100 Subject: [PATCH] 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 Reviewed-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 29 ++++++++--------------------- drivers/staging/greybus/hid.c | 23 ++++++----------------- drivers/staging/greybus/operation.c | 17 ++++++++++------- drivers/staging/greybus/protocol.h | 2 +- 4 files changed, 25 insertions(+), 46 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 9a34fc5a52b8..20917bf6479f 100644 --- 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 cc5708ddf068..5935aa6c6334 100644 --- 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 2b2b0d6332c8..260774e414f3 100644 --- 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 a74afef92a01..82d9e81386e3 100644 --- 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 -- 2.20.1