• Andy Shevchenko's avatar
    gpio: pca953x: Synchronize interrupt handler properly · 064c73af
    Andy Shevchenko authored
    Since the commit aa58a21a ("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
    6e20fb18 ("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: 6e20fb18 ("drivers/gpio/pca953x.c: add a mutex to fix race condition")
    Depends-on: 35d13d94 ("gpio: pca953x: convert to use bitmap API")
    Depends-on: 49427232 ("gpio: pca953x: Perform basic regmap conversion")
    Cc: Marek Vasut <marek.vasut@gmail.com>
    Cc: Roland Stigge <stigge@antcom.de>
    Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
    Signed-off-by: default avatarBartosz Golaszewski <bgolaszewski@baylibre.com>
    064c73af
gpio-pca953x.c 32.5 KB