Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-12-27 13:22:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

gpio: pca953x: Synchronize interrupt handler properly

Browse Source 
Since the commit aa58a21ae3 ("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
6e20fb1805 ("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: 6e20fb1805 ("drivers/gpio/pca953x.c: add a mutex to fix race condition")
Depends-on: 35d13d9489 ("gpio: pca953x: convert to use bitmap API")
Depends-on: 4942723276 ("gpio: pca953x: Perform basic regmap conversion")
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Roland Stigge <stigge@antcom.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This commit is contained in:
Andy Shevchenko 2020-06-05 16:40:33 +03:00 committed by Bartosz Golaszewski
parent b3a9e3b962
commit 064c73afe7
1 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
drivers/gpio/gpio-pca953x.c
View File

@ -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)