From 1d771fe41e629fad9077a0a1a9cf2771c9a5287d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:18 +0200 Subject: [PATCH 1/8] greybus: operation: add support for incoming unidirectional operations Add support for incoming, unidirectional operations where the sender of a request does not care about a response. Unidirectional operations have an operation id of 0. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/operation.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index f595b97fa900..4f7a555e4b96 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -686,6 +686,10 @@ int gb_operation_response_send(struct gb_operation *operation, int errno) return -EIO; /* Shouldn't happen */ } + /* Sender of request does not care about response. */ + if (!operation->id) + return 0; + if (!operation->response) { if (!gb_operation_response_alloc(operation, 0)) { dev_err(&connection->dev, From c8d1ad8013681eeb2dc8dac405db3b95284adc1d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:19 +0200 Subject: [PATCH 2/8] greybus: gpio: fix debugfs output Fix debugfs output by removing the unimplemented, custom dbg_show callback. The default implementation is perfectly sufficient. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 6e5fe5b3db39..0191bb809968 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -414,11 +414,6 @@ static int gb_gpio_set_debounce(struct gpio_chip *chip, unsigned offset, return gb_gpio_set_debounce_operation(ggc, (u8)offset, usec); } -static void gb_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) -{ - return; /* XXX */ -} - static int gb_gpio_controller_setup(struct gb_gpio_controller *ggc) { int ret; @@ -616,7 +611,6 @@ static int gb_gpio_connection_init(struct gb_connection *connection) gpio->get = gb_gpio_get; gpio->set = gb_gpio_set; gpio->set_debounce = gb_gpio_set_debounce; - gpio->dbg_show = gb_gpio_dbg_show; gpio->to_irq = gb_gpio_to_irq; gpio->base = -1; /* Allocate base dynamically */ gpio->ngpio = ggc->line_max + 1; From b8e3ffebac09b29ad4cc0bdbcafbbd77b3278685 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:20 +0200 Subject: [PATCH 3/8] greybus: gpio: remove unused irq-ack operation Remove unused irq-ack operation, which has never been called and does not make sense for message-signalled interrupts over slow buses. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 16 ---------------- drivers/staging/greybus/greybus_protocols.h | 6 ------ 2 files changed, 22 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 0191bb809968..c570f62046f5 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -211,21 +211,6 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *ggc, return ret; } -static void gb_gpio_ack_irq(struct irq_data *d) -{ - struct gpio_chip *chip = irq_data_to_gpio_chip(d); - struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); - struct gb_gpio_irq_ack_request request; - int ret; - - request.which = d->hwirq; - ret = gb_operation_sync(ggc->connection, - GB_GPIO_TYPE_IRQ_ACK, - &request, sizeof(request), NULL, 0); - if (ret) - dev_err(chip->dev, "failed to ack irq: %d\n", ret); -} - static void gb_gpio_mask_irq(struct irq_data *d) { struct gpio_chip *chip = irq_data_to_gpio_chip(d); @@ -591,7 +576,6 @@ static int gb_gpio_connection_init(struct gb_connection *connection) goto err_free_controller; irqc = &ggc->irqc; - irqc->irq_ack = gb_gpio_ack_irq; irqc->irq_mask = gb_gpio_mask_irq; irqc->irq_unmask = gb_gpio_unmask_irq; irqc->irq_set_type = gb_gpio_irq_set_type; diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 28c40a09a05b..0fd42bc44161 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -128,7 +128,6 @@ struct gb_i2c_transfer_response { #define GB_GPIO_TYPE_SET_VALUE 0x09 #define GB_GPIO_TYPE_SET_DEBOUNCE 0x0a #define GB_GPIO_TYPE_IRQ_TYPE 0x0b -#define GB_GPIO_TYPE_IRQ_ACK 0x0c #define GB_GPIO_TYPE_IRQ_MASK 0x0d #define GB_GPIO_TYPE_IRQ_UNMASK 0x0e #define GB_GPIO_TYPE_IRQ_EVENT 0x0f @@ -203,11 +202,6 @@ struct gb_gpio_irq_unmask_request { }; /* irq unmask response has no payload */ -struct gb_gpio_irq_ack_request { - __u8 which; -}; -/* irq ack response has no payload */ - /* irq event requests originate on another module and are handled on the AP */ struct gb_gpio_irq_event_request { __u8 which; From 2611ebef8322fc12dc3c6b0ec869f1902aa25626 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:21 +0200 Subject: [PATCH 4/8] greybus: gpio: don't call irq-flow handler directly Use generic_handle_irq_desc rather than call a hardcoded irq-flow handler directly. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index c570f62046f5..526dd7e73a2f 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -314,9 +314,8 @@ static int gb_gpio_request_recv(u8 type, struct gb_operation *op) return -EINVAL; } - /* Dispatch interrupt */ local_irq_disable(); - handle_simple_irq(irq, desc); + generic_handle_irq_desc(irq, desc); local_irq_enable(); return 0; From ec762115a5006db8549b3582f7f19849f7cf4ab4 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:22 +0200 Subject: [PATCH 5/8] greybus: gpio: use irq-domain lookups Use irq_find_mapping directly rather than go through the legacy gpio interface. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 526dd7e73a2f..15cc0ea4e3a5 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -303,9 +303,9 @@ static int gb_gpio_request_recv(u8 type, struct gb_operation *op) return -EINVAL; } - irq = gpio_to_irq(ggc->chip.base + event->which); - if (irq < 0) { - dev_err(ggc->chip.dev, "failed to map irq\n"); + irq = irq_find_mapping(ggc->irqdomain, event->which); + if (!irq) { + dev_err(ggc->chip.dev, "failed to find IRQ\n"); return -EINVAL; } desc = irq_to_desc(irq); From 0cb918d72d8f76b1189a63cefedbc5cb4dad9d54 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:23 +0200 Subject: [PATCH 6/8] greybus: gpio: rename irq mask and unmask callbacks Rename irq mask and unmask functions to match the callback names. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 15cc0ea4e3a5..0220a9f1ff9c 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -211,7 +211,7 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *ggc, return ret; } -static void gb_gpio_mask_irq(struct irq_data *d) +static void gb_gpio_irq_mask(struct irq_data *d) { struct gpio_chip *chip = irq_data_to_gpio_chip(d); struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); @@ -226,7 +226,7 @@ static void gb_gpio_mask_irq(struct irq_data *d) dev_err(chip->dev, "failed to mask irq: %d\n", ret); } -static void gb_gpio_unmask_irq(struct irq_data *d) +static void gb_gpio_irq_unmask(struct irq_data *d) { struct gpio_chip *chip = irq_data_to_gpio_chip(d); struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); @@ -575,8 +575,8 @@ static int gb_gpio_connection_init(struct gb_connection *connection) goto err_free_controller; irqc = &ggc->irqc; - irqc->irq_mask = gb_gpio_mask_irq; - irqc->irq_unmask = gb_gpio_unmask_irq; + irqc->irq_mask = gb_gpio_irq_mask; + irqc->irq_unmask = gb_gpio_irq_unmask; irqc->irq_set_type = gb_gpio_irq_set_type; irqc->name = "greybus_gpio"; From 25f11ed965bb57b6e25d5464a8cd8b3657319056 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:24 +0200 Subject: [PATCH 7/8] greybus: gpio: fix atomic sleep when using interrupts The irq-chip callbacks are made in atomic context where we must not do any synchronous greybus operations. Fix the current gpio-interrupt implementation by using the bus-lock functionality provided by the irq subsystem. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 121 +++++++++++++++++++++++++-------- 1 file changed, 93 insertions(+), 28 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 0220a9f1ff9c..6cf3bb151f61 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -13,6 +13,8 @@ #include #include #include +#include + #include "greybus.h" struct gb_gpio_line { @@ -22,6 +24,11 @@ struct gb_gpio_line { direction: 1, /* 0 = output, 1 = input */ value: 1; /* 0 = low, 1 = high */ u16 debounce_usec; + + u8 irq_type; + bool irq_type_pending; + bool masked; + bool masked_pending; }; struct gb_gpio_controller { @@ -38,6 +45,7 @@ struct gb_gpio_controller { unsigned int irq_base; irq_flow_handler_t irq_handler; unsigned int irq_default_type; + struct mutex irq_lock; }; #define gpio_chip_to_gb_gpio_controller(chip) \ container_of(chip, struct gb_gpio_controller, chip) @@ -211,68 +219,121 @@ static int gb_gpio_set_debounce_operation(struct gb_gpio_controller *ggc, return ret; } -static void gb_gpio_irq_mask(struct irq_data *d) +static void _gb_gpio_irq_mask(struct gb_gpio_controller *ggc, u8 hwirq) { - struct gpio_chip *chip = irq_data_to_gpio_chip(d); - struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); struct gb_gpio_irq_mask_request request; int ret; - request.which = d->hwirq; + request.which = hwirq; ret = gb_operation_sync(ggc->connection, GB_GPIO_TYPE_IRQ_MASK, &request, sizeof(request), NULL, 0); if (ret) - dev_err(chip->dev, "failed to mask irq: %d\n", ret); + dev_err(ggc->chip.dev, "failed to mask irq: %d\n", ret); +} + +static void _gb_gpio_irq_unmask(struct gb_gpio_controller *ggc, u8 hwirq) +{ + struct gb_gpio_irq_unmask_request request; + int ret; + + request.which = hwirq; + ret = gb_operation_sync(ggc->connection, + GB_GPIO_TYPE_IRQ_UNMASK, + &request, sizeof(request), NULL, 0); + if (ret) + dev_err(ggc->chip.dev, "failed to unmask irq: %d\n", ret); +} + +static void _gb_gpio_irq_set_type(struct gb_gpio_controller *ggc, + u8 hwirq, u8 type) +{ + struct gb_gpio_irq_type_request request; + int ret; + + request.which = hwirq; + request.type = type; + + ret = gb_operation_sync(ggc->connection, + GB_GPIO_TYPE_IRQ_TYPE, + &request, sizeof(request), NULL, 0); + if (ret) + dev_err(ggc->chip.dev, "failed to set irq type: %d\n", ret); +} + +static void gb_gpio_irq_mask(struct irq_data *d) +{ + struct gpio_chip *chip = irq_data_to_gpio_chip(d); + struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); + struct gb_gpio_line *line = &ggc->lines[d->hwirq]; + + line->masked = true; + line->masked_pending = true; } static void gb_gpio_irq_unmask(struct irq_data *d) { struct gpio_chip *chip = irq_data_to_gpio_chip(d); struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); - struct gb_gpio_irq_unmask_request request; - int ret; + struct gb_gpio_line *line = &ggc->lines[d->hwirq]; - request.which = d->hwirq; - ret = gb_operation_sync(ggc->connection, - GB_GPIO_TYPE_IRQ_UNMASK, - &request, sizeof(request), NULL, 0); - if (ret) - dev_err(chip->dev, "failed to unmask irq: %d\n", ret); + line->masked = false; + line->masked_pending = true; } static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type) { struct gpio_chip *chip = irq_data_to_gpio_chip(d); struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); - struct gb_gpio_irq_type_request request; - int ret = 0; - - request.which = d->hwirq; - request.type = type; + struct gb_gpio_line *line = &ggc->lines[d->hwirq]; switch (type) { case IRQ_TYPE_NONE: - break; case IRQ_TYPE_EDGE_RISING: case IRQ_TYPE_EDGE_FALLING: case IRQ_TYPE_EDGE_BOTH: case IRQ_TYPE_LEVEL_LOW: case IRQ_TYPE_LEVEL_HIGH: - ret = gb_operation_sync(ggc->connection, - GB_GPIO_TYPE_IRQ_TYPE, - &request, sizeof(request), NULL, 0); - if (ret) { - dev_err(chip->dev, "failed to set irq type: %d\n", - ret); - } break; default: dev_err(chip->dev, "unsupported irq type: %u\n", type); - ret = -EINVAL; + return -EINVAL; } - return ret; + line->irq_type = type; + line->irq_type_pending = true; + + return 0; +} + +static void gb_gpio_irq_bus_lock(struct irq_data *d) +{ + struct gpio_chip *chip = irq_data_to_gpio_chip(d); + struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); + + mutex_lock(&ggc->irq_lock); +} + +static void gb_gpio_irq_bus_sync_unlock(struct irq_data *d) +{ + struct gpio_chip *chip = irq_data_to_gpio_chip(d); + struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip); + struct gb_gpio_line *line = &ggc->lines[d->hwirq]; + + if (line->irq_type_pending) { + _gb_gpio_irq_set_type(ggc, d->hwirq, line->irq_type); + line->irq_type_pending = false; + } + + if (line->masked_pending) { + if (line->masked) + _gb_gpio_irq_mask(ggc, d->hwirq); + else + _gb_gpio_irq_unmask(ggc, d->hwirq); + line->masked_pending = false; + } + + mutex_unlock(&ggc->irq_lock); } static int gb_gpio_request_recv(u8 type, struct gb_operation *op) @@ -578,8 +639,12 @@ static int gb_gpio_connection_init(struct gb_connection *connection) irqc->irq_mask = gb_gpio_irq_mask; irqc->irq_unmask = gb_gpio_irq_unmask; irqc->irq_set_type = gb_gpio_irq_set_type; + irqc->irq_bus_lock = gb_gpio_irq_bus_lock; + irqc->irq_bus_sync_unlock = gb_gpio_irq_bus_sync_unlock; irqc->name = "greybus_gpio"; + mutex_init(&ggc->irq_lock); + gpio = &ggc->chip; gpio->label = "greybus_gpio"; From 1409c4d6a841b00f3b4bdf010808a81eda6b7727 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 May 2015 15:29:25 +0200 Subject: [PATCH 8/8] greybus: gpio: fix interrupt protocol The current interrupt implementation uses the simple irq-flow handler, which means that the interrupt subsystem makes no irq-chip callbacks when handling an interrupt. Specifically, no end-of-interrupt message is sent when the threaded handler has run. This means that we may currently re-enable an interrupt before it has been serviced (i.e. the irq-event operation may complete before the threaded handler has run). The simple flow handler also silently drops a second interrupt arriving while a handler is running. This means that we may lose a second edge interrupt with the current firmware. Switch to a new one-shot interrupt protocol, where the primary handler (firmware) always masks and acks an interrupt before sending an event to the AP. The AP is responsible for unmasking the interrupt when it has been handled. By having the firmware ack an edge interrupt before sending the event, a second edge interrupt will no longer get lost. This one-shot protocol can be implemented in the kernel by using the level irq-flow handler, one-shot interrupts with threaded handlers and bus-lock synchronisation for slow buses. Note that the same flow handler is used for both edge and level interrupts. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/gpio.c | 2 +- drivers/staging/greybus/greybus_protocols.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 6cf3bb151f61..8dad9e579881 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -672,7 +672,7 @@ static int gb_gpio_connection_init(struct gb_connection *connection) } ret = gb_gpio_irqchip_add(gpio, irqc, 0, - handle_simple_irq, IRQ_TYPE_NONE); + handle_level_irq, IRQ_TYPE_NONE); if (ret) { dev_err(&connection->dev, "failed to add irq chip: %d\n", ret); goto irqchip_err; diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 0fd42bc44161..81ec3b246cc6 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -206,7 +206,7 @@ struct gb_gpio_irq_unmask_request { struct gb_gpio_irq_event_request { __u8 which; }; -/* irq event response has no payload */ +/* irq event has no response */ /* PWM */