[PATCH v1] gpio: mlxbf2: Disable interrupts around the chip lock in direction ops

[PATCH v1] gpio: mlxbf2: Disable interrupts around the chip lock in direction ops

[David Thompson]

mlxbf2_gpio_lock_acquire() takes the generic GPIO chip lock with
gpio_generic_chip_lock(), a plain raw_spin_lock() that leaves
interrupts enabled. The same lock is also taken with the irqsave
variant by the IRQ callbacks (irq_set_type, irq_enable, irq_disable),
which run with the IRQ descriptor lock held.

This creates a possible deadlock that lockdep reports as an IRQ lock
inversion:

  CPU0                              CPU1
  ----                              ----
  lock(&chip->lock)                 lock(irq_desc->lock)
  <GPIO interrupt>                  lock(&chip->lock)
    lock(irq_desc->lock)

If a GPIO interrupt arrives on CPU0 while it holds the chip lock during
a direction change, the interrupt handler spins on the descriptor lock
that CPU1 holds, while CPU1 spins on the chip lock held by CPU0.

This was seen during probe of pwr-mlxbf. acpi_dev_gpio_irq_get() sets
the pin to input through mlxbf2_gpio_direction_input(), and then
irq_set_irq_type() runs with the descriptor lock held.

Use the irqsave variant for the chip lock in mlxbf2_gpio_lock_acquire()
and its release counterpart, and pass the saved flags from
mlxbf2_gpio_direction_input() and mlxbf2_gpio_direction_output(). This
matches the IRQ callbacks, so interrupts stay disabled for the whole
ModeX update.

Fixes: 2b725265cb08 ("gpio: mlxbf2: Introduce IRQ support")
Cc: stable@vger.kernel.org
Signed-off-by: David Thompson <davthompson@nvidia.com>
---
 drivers/gpio/gpio-mlxbf2.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
index 70c19258a51d..c9e08a7c5eba 100644
--- a/drivers/gpio/gpio-mlxbf2.c
+++ b/drivers/gpio/gpio-mlxbf2.c
@@ -128,12 +128,20 @@ static int mlxbf2_gpio_get_lock_res(struct platform_device *pdev)
  * Acquire the YU arm_gpio_lock to be able to change the direction
  * mode. If the lock_active bit is already set, return an error.
  */
-static int mlxbf2_gpio_lock_acquire(struct mlxbf2_gpio_context *gs)
+static int mlxbf2_gpio_lock_acquire(struct mlxbf2_gpio_context *gs,
+				    unsigned long *flags)
 {
 	u32 arm_gpio_lock_val;
 
 	mutex_lock(yu_arm_gpio_lock_param.lock);
-	gpio_generic_chip_lock(&gs->chip);
+	/*
+	 * Disable interrupts before taking the chip lock. The GPIO irq
+	 * callbacks take this lock while holding the irq descriptor lock,
+	 * and the irq handler takes the descriptor lock in hardirq context.
+	 * A plain spin_lock() here lets lockdep report an inversion, and
+	 * can deadlock if a GPIO interrupt arrives on this CPU.
+	 */
+	gpio_generic_chip_lock_irqsave(&gs->chip, *flags);
 
 	arm_gpio_lock_val = readl(yu_arm_gpio_lock_param.io);
 
@@ -141,7 +149,7 @@ static int mlxbf2_gpio_lock_acquire(struct mlxbf2_gpio_context *gs)
 	 * When lock active bit[31] is set, ModeX is write enabled
 	 */
 	if (YU_LOCK_ACTIVE_BIT(arm_gpio_lock_val)) {
-		gpio_generic_chip_unlock(&gs->chip);
+		gpio_generic_chip_unlock_irqrestore(&gs->chip, *flags);
 		mutex_unlock(yu_arm_gpio_lock_param.lock);
 		return -EINVAL;
 	}
@@ -154,12 +162,13 @@ static int mlxbf2_gpio_lock_acquire(struct mlxbf2_gpio_context *gs)
 /*
  * Release the YU arm_gpio_lock after changing the direction mode.
  */
-static void mlxbf2_gpio_lock_release(struct mlxbf2_gpio_context *gs)
+static void mlxbf2_gpio_lock_release(struct mlxbf2_gpio_context *gs,
+				     unsigned long flags)
 	__releases(&gs->chip.lock)
 	__releases(yu_arm_gpio_lock_param.lock)
 {
 	writel(YU_ARM_GPIO_LOCK_RELEASE, yu_arm_gpio_lock_param.io);
-	gpio_generic_chip_unlock(&gs->chip);
+	gpio_generic_chip_unlock_irqrestore(&gs->chip, flags);
 	mutex_unlock(yu_arm_gpio_lock_param.lock);
 }
 
@@ -185,20 +194,21 @@ static int mlxbf2_gpio_direction_input(struct gpio_chip *chip,
 				       unsigned int offset)
 {
 	struct mlxbf2_gpio_context *gs = gpiochip_get_data(chip);
+	unsigned long flags;
 	int ret;
 
 	/*
 	 * Although the arm_gpio_lock was set in the probe function, check again
 	 * if it is still enabled to be able to write to the ModeX registers.
 	 */
-	ret = mlxbf2_gpio_lock_acquire(gs);
+	ret = mlxbf2_gpio_lock_acquire(gs, &flags);
 	if (ret < 0)
 		return ret;
 
 	writel(BIT(offset), gs->gpio_io + YU_GPIO_MODE0_CLEAR);
 	writel(BIT(offset), gs->gpio_io + YU_GPIO_MODE1_CLEAR);
 
-	mlxbf2_gpio_lock_release(gs);
+	mlxbf2_gpio_lock_release(gs, flags);
 
 	return ret;
 }
@@ -212,6 +222,7 @@ static int mlxbf2_gpio_direction_output(struct gpio_chip *chip,
 					int value)
 {
 	struct mlxbf2_gpio_context *gs = gpiochip_get_data(chip);
+	unsigned long flags;
 	int ret = 0;
 
 	/*
@@ -219,14 +230,14 @@ static int mlxbf2_gpio_direction_output(struct gpio_chip *chip,
 	 * check again it is still enabled to be able to write to the
 	 * ModeX registers.
 	 */
-	ret = mlxbf2_gpio_lock_acquire(gs);
+	ret = mlxbf2_gpio_lock_acquire(gs, &flags);
 	if (ret < 0)
 		return ret;
 
 	writel(BIT(offset), gs->gpio_io + YU_GPIO_MODE1_CLEAR);
 	writel(BIT(offset), gs->gpio_io + YU_GPIO_MODE0_SET);
 
-	mlxbf2_gpio_lock_release(gs);
+	mlxbf2_gpio_lock_release(gs, flags);
 
 	return ret;
 }
-- 
2.43.2