From e6f390a834b56583e6fc0949822644ce92fbb107 Mon Sep 17 00:00:00 2001 From: Navid Emamdoost Date: Thu, 4 Jun 2020 21:52:07 -0500 Subject: gpio: arizona: handle pm_runtime_get_sync failure case Calling pm_runtime_get_sync increments the counter even in case of failure, causing incorrect ref count. Call pm_runtime_put if pm_runtime_get_sync fails. Signed-off-by: Navid Emamdoost Acked-by: Charles Keepax Link: https://lore.kernel.org/r/20200605025207.65719-1-navid.emamdoost@gmail.com Signed-off-by: Linus Walleij --- drivers/gpio/gpio-arizona.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c index 5640efe5e750..7520a13b4c7c 100644 --- a/drivers/gpio/gpio-arizona.c +++ b/drivers/gpio/gpio-arizona.c @@ -106,6 +106,7 @@ static int arizona_gpio_direction_out(struct gpio_chip *chip, ret = pm_runtime_get_sync(chip->parent); if (ret < 0) { dev_err(chip->parent, "Failed to resume: %d\n", ret); + pm_runtime_put(chip->parent); return ret; } } -- cgit v1.2.3 From 861254d826499944cb4d9b5a15f5a794a6b99a69 Mon Sep 17 00:00:00 2001 From: Navid Emamdoost Date: Thu, 4 Jun 2020 22:00:52 -0500 Subject: gpio: arizona: put pm_runtime in case of failure Calling pm_runtime_get_sync increments the counter even in case of failure, causing incorrect ref count if pm_runtime_put is not called in error handling paths. Call pm_runtime_put if pm_runtime_get_sync fails. Signed-off-by: Navid Emamdoost Acked-by: Charles Keepax Link: https://lore.kernel.org/r/20200605030052.78235-1-navid.emamdoost@gmail.com Signed-off-by: Linus Walleij --- drivers/gpio/gpio-arizona.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c index 7520a13b4c7c..5bda38e0780f 100644 --- a/drivers/gpio/gpio-arizona.c +++ b/drivers/gpio/gpio-arizona.c @@ -64,6 +64,7 @@ static int arizona_gpio_get(struct gpio_chip *chip, unsigned offset) ret = pm_runtime_get_sync(chip->parent); if (ret < 0) { dev_err(chip->parent, "Failed to resume: %d\n", ret); + pm_runtime_put_autosuspend(chip->parent); return ret; } @@ -72,12 +73,15 @@ static int arizona_gpio_get(struct gpio_chip *chip, unsigned offset) if (ret < 0) { dev_err(chip->parent, "Failed to drop cache: %d\n", ret); + pm_runtime_put_autosuspend(chip->parent); return ret; } ret = regmap_read(arizona->regmap, reg, &val); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_autosuspend(chip->parent); return ret; + } pm_runtime_mark_last_busy(chip->parent); pm_runtime_put_autosuspend(chip->parent); -- cgit v1.2.3 From 064c73afe7385de99e5b2785b88c83dc5d84403b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 5 Jun 2020 16:40:33 +0300 Subject: gpio: pca953x: Synchronize interrupt handler properly Since the commit aa58a21ae378 ("gpio: pca953x: disable regmap locking") the locking of regmap is disabled and that immediately introduces a synchronization issue. It's easy to see when we try to monitor more than one interrupt from the same chip. It seems that the problem exists from the day one and even commit 6e20fb18054c ("drivers/gpio/pca953x.c: add a mutex to fix race condition") missed this. Below are the traces and shell reproducers before and after proposed change. Note duplicates in the IRQ events. /proc/interrupts also shows a deviation, i.e. sum of children interrupts higher than parent's one. When locking is disabled for regmap and no protection in IRQ handler ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... gpioset-194 regmap_hw_write_start: i2c-INT3491:02 reg=2 count=1 irq/31-i2c-INT3-139 regmap_hw_read_start: i2c-INT3491:02 reg=4c count=2 gpioset-194 regmap_hw_write_done: i2c-INT3491:02 reg=2 count=1 gpioset-194 regmap_reg_read_cache: i2c-INT3491:02 reg=6 val=f5 gpioset-194 regmap_reg_write: i2c-INT3491:02 reg=6 val=f5 gpioset-194 regmap_hw_write_start: i2c-INT3491:02 reg=6 count=1 irq/31-i2c-INT3-139 regmap_hw_read_done: i2c-INT3491:02 reg=4c count=2 ... % gpiomon gpiochip3 0 & % gpioset gpiochip3 1=0 % gpioset gpiochip3 1=1 event: RISING EDGE offset: 0 timestamp: [ 302.782583765] % gpiomon gpiochip3 2 & % gpioset gpiochip3 1=0 event: RISING EDGE offset: 2 timestamp: [ 312.033148829] event: FALLING EDGE offset: 0 timestamp: [ 312.022757525] % gpioset gpiochip3 1=1 event: RISING EDGE offset: 2 timestamp: [ 316.201148473] event: RISING EDGE offset: 0 timestamp: [ 316.191759599] When locking is disabled for regmap and protection in IRQ handler ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... gpioset-202 regmap_hw_write_start: i2c-INT3491:02 reg=2 count=1 gpioset-202 regmap_hw_write_done: i2c-INT3491:02 reg=2 count=1 gpioset-202 regmap_reg_read_cache: i2c-INT3491:02 reg=6 val=fd gpioset-202 regmap_reg_write: i2c-INT3491:02 reg=6 val=fd gpioset-202 regmap_hw_write_start: i2c-INT3491:02 reg=6 count=1 gpioset-202 regmap_hw_write_done: i2c-INT3491:02 reg=6 count=1 irq/31-i2c-INT3-139 regmap_hw_read_start: i2c-INT3491:02 reg=4c count=2 irq/31-i2c-INT3-139 regmap_hw_read_done: i2c-INT3491:02 reg=4c count=2 ... % gpiomon gpiochip3 0 & % gpioset gpiochip3 1=0 event: FALLING EDGE offset: 0 timestamp: [ 531.330078107] % gpioset gpiochip3 1=1 event: RISING EDGE offset: 0 timestamp: [ 532.912239128] % gpiomon gpiochip3 2 & % gpioset gpiochip3 1=0 event: FALLING EDGE offset: 0 timestamp: [ 539.633669484] % gpioset gpiochip3 1=1 event: RISING EDGE offset: 0 timestamp: [ 542.256978461] Fixes: 6e20fb18054c ("drivers/gpio/pca953x.c: add a mutex to fix race condition") Depends-on: 35d13d94893f ("gpio: pca953x: convert to use bitmap API") Depends-on: 49427232764d ("gpio: pca953x: Perform basic regmap conversion") Cc: Marek Vasut Cc: Roland Stigge Signed-off-by: Andy Shevchenko Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 1fca8dd7824f..afe78639ec58 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -734,14 +734,16 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid) struct gpio_chip *gc = &chip->gpio_chip; DECLARE_BITMAP(pending, MAX_LINE); int level; + bool ret; - if (!pca953x_irq_pending(chip, pending)) - return IRQ_NONE; + mutex_lock(&chip->i2c_lock); + ret = pca953x_irq_pending(chip, pending); + mutex_unlock(&chip->i2c_lock); for_each_set_bit(level, pending, gc->ngpio) handle_nested_irq(irq_find_mapping(gc->irq.domain, level)); - return IRQ_HANDLED; + return IRQ_RETVAL(ret); } static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base) -- cgit v1.2.3 From ba8c90c6184784b397807b72403656085ac2f8c1 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 5 Jun 2020 16:40:34 +0300 Subject: gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACPI table on Intel Galileo Gen 2 has wrong pin number for IRQ resource of one of the I²C GPIO expanders. Since we know what that number is and luckily have GPIO bases fixed for SoC's controllers, we may use a simple DMI quirk to match the platform and retrieve GpioInt() pin on it for the expander in question. Mika suggested the way to avoid a quirk in the GPIO ACPI library and here is the second, almost rewritten version of it. Fixes: f32517bf1ae0 ("gpio: pca953x: support ACPI devices found on Galileo Gen2") Depends-on: 25e3ef894eef ("gpio: acpi: Split out acpi_gpio_get_irq_resource() helper") Suggested-by: Mika Westerberg Reviewed-by: Mika Westerberg Signed-off-by: Andy Shevchenko Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 79 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index afe78639ec58..4d3157d8b5cd 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -107,6 +107,79 @@ static const struct i2c_device_id pca953x_id[] = { }; MODULE_DEVICE_TABLE(i2c, pca953x_id); +#ifdef CONFIG_GPIO_PCA953X_IRQ + +#include +#include +#include + +static const struct dmi_system_id pca953x_dmi_acpi_irq_info[] = { + { + /* + * On Intel Galileo Gen 2 board the IRQ pin of one of + * the I²C GPIO expanders, which has GpioInt() resource, + * is provided as an absolute number instead of being + * relative. Since first controller (gpio-sch.c) and + * second (gpio-dwapb.c) are at the fixed bases, we may + * safely refer to the number in the global space to get + * an IRQ out of it. + */ + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"), + }, + }, + {} +}; + +#ifdef CONFIG_ACPI +static int pca953x_acpi_get_pin(struct acpi_resource *ares, void *data) +{ + struct acpi_resource_gpio *agpio; + int *pin = data; + + if (acpi_gpio_get_irq_resource(ares, &agpio)) + *pin = agpio->pin_table[0]; + return 1; +} + +static int pca953x_acpi_find_pin(struct device *dev) +{ + struct acpi_device *adev = ACPI_COMPANION(dev); + int pin = -ENOENT, ret; + LIST_HEAD(r); + + ret = acpi_dev_get_resources(adev, &r, pca953x_acpi_get_pin, &pin); + acpi_dev_free_resource_list(&r); + if (ret < 0) + return ret; + + return pin; +} +#else +static inline int pca953x_acpi_find_pin(struct device *dev) { return -ENXIO; } +#endif + +static int pca953x_acpi_get_irq(struct device *dev) +{ + int pin, ret; + + pin = pca953x_acpi_find_pin(dev); + if (pin < 0) + return pin; + + dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin); + + if (!gpio_is_valid(pin)) + return -EINVAL; + + ret = gpio_request(pin, "pca953x interrupt"); + if (ret) + return ret; + + return gpio_to_irq(pin); +} +#endif + static const struct acpi_device_id pca953x_acpi_ids[] = { { "INT3491", 16 | PCA953X_TYPE | PCA_LATCH_INT, }, { } @@ -754,6 +827,12 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base) DECLARE_BITMAP(irq_stat, MAX_LINE); int ret; + if (dmi_first_match(pca953x_dmi_acpi_irq_info)) { + ret = pca953x_acpi_get_irq(&client->dev); + if (ret > 0) + client->irq = ret; + } + if (!client->irq) return 0; -- cgit v1.2.3 From 0b22c25e1b81c5f718e89c4d759e6a359be24417 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 5 Jun 2020 16:40:35 +0300 Subject: gpio: pca953x: Fix direction setting when configure an IRQ The commit 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache") seems inadvertently made a typo in pca953x_irq_bus_sync_unlock(). When the direction bit is 1 it means input, and the piece of code in question was looking for output ones that should be turned to inputs. Fix direction setting when configure an IRQ by injecting a bitmap complement operation. Fixes: 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache") Depends-on: 35d13d94893f ("gpio: pca953x: convert to use bitmap API") Cc: Marek Vasut Signed-off-by: Andy Shevchenko Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 4d3157d8b5cd..97c9ac31ecb5 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -696,8 +696,6 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) DECLARE_BITMAP(reg_direction, MAX_LINE); int level; - pca953x_read_regs(chip, chip->regs->direction, reg_direction); - if (chip->driver_data & PCA_PCAL) { /* Enable latch on interrupt-enabled inputs */ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask); @@ -708,7 +706,11 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) pca953x_write_regs(chip, PCAL953X_INT_MASK, irq_mask); } + /* Switch direction to input if needed */ + pca953x_read_regs(chip, chip->regs->direction, reg_direction); + bitmap_or(irq_mask, chip->irq_trig_fall, chip->irq_trig_raise, gc->ngpio); + bitmap_complement(reg_direction, reg_direction, gc->ngpio); bitmap_and(irq_mask, irq_mask, reg_direction, gc->ngpio); /* Look for any newly setup interrupt */ -- cgit v1.2.3 From ec3decd21380081e3b5de4ba8d85d90a95f201a0 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 5 Jun 2020 16:40:36 +0300 Subject: gpio: pca953x: disable regmap locking for automatic address incrementing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's a repetition of the commit aa58a21ae378 ("gpio: pca953x: disable regmap locking") which states the following: This driver uses its own locking but regmap silently uses a mutex for all operations too. Add the option to disable locking to the regmap config struct. Fixes: bcf41dc480b1 ("gpio: pca953x: fix handling of automatic address incrementing") Cc: Uwe Kleine-König Signed-off-by: Andy Shevchenko Reviewed-by: Uwe Kleine-König Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 97c9ac31ecb5..6f409ee0b033 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -395,6 +395,7 @@ static const struct regmap_config pca953x_ai_i2c_regmap = { .writeable_reg = pca953x_writeable_register, .volatile_reg = pca953x_volatile_register, + .disable_locking = true, .cache_type = REGCACHE_RBTREE, .max_register = 0x7f, }; -- cgit v1.2.3 From 5d8913504ccfeea6120df5ae1c6f4479ff09b931 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 18 Jun 2020 14:49:06 +0300 Subject: gpio: pca953x: Fix GPIO resource leak on Intel Galileo Gen 2 When adding a quirk for IRQ on Intel Galileo Gen 2 the commit ba8c90c61847 ("gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2") missed GPIO resource release. We can safely do this in the same quirk, since IRQ will be locked by GPIO framework when requested and unlocked on freeing. Fixes: ba8c90c61847 ("gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2") Signed-off-by: Andy Shevchenko Cc: Mika Westerberg Reviewed-by: Mika Westerberg Reviewed-by: Linus Walleij Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pca953x.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 6f409ee0b033..a3b9bdedbe44 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -176,7 +176,12 @@ static int pca953x_acpi_get_irq(struct device *dev) if (ret) return ret; - return gpio_to_irq(pin); + ret = gpio_to_irq(pin); + + /* When pin is used as an IRQ, no need to keep it requested */ + gpio_free(pin); + + return ret; } #endif -- cgit v1.2.3