Skip to content

Commit 064c73a

Browse files
andy-shevbrgl
authored andcommitted
gpio: pca953x: Synchronize interrupt handler properly
Since the commit aa58a21 ("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 6e20fb1 ("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: 6e20fb1 ("drivers/gpio/pca953x.c: add a mutex to fix race condition") Depends-on: 35d13d9 ("gpio: pca953x: convert to use bitmap API") Depends-on: 4942723 ("gpio: pca953x: Perform basic regmap conversion") Cc: Marek Vasut <[email protected]> Cc: Roland Stigge <[email protected]> Signed-off-by: Andy Shevchenko <[email protected]> Signed-off-by: Bartosz Golaszewski <[email protected]>
1 parent b3a9e3b commit 064c73a

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

drivers/gpio/gpio-pca953x.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -734,14 +734,16 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid)
734734
struct gpio_chip *gc = &chip->gpio_chip;
735735
DECLARE_BITMAP(pending, MAX_LINE);
736736
int level;
737+
bool ret;
737738

738-
if (!pca953x_irq_pending(chip, pending))
739-
return IRQ_NONE;
739+
mutex_lock(&chip->i2c_lock);
740+
ret = pca953x_irq_pending(chip, pending);
741+
mutex_unlock(&chip->i2c_lock);
740742

741743
for_each_set_bit(level, pending, gc->ngpio)
742744
handle_nested_irq(irq_find_mapping(gc->irq.domain, level));
743745

744-
return IRQ_HANDLED;
746+
return IRQ_RETVAL(ret);
745747
}
746748

747749
static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base)

0 commit comments

Comments
 (0)