pinctrl: samsung: fix SMP race condition
authorYoungmin Nam <ym0914@gmail.com>
Mon, 8 Feb 2016 15:49:28 +0000 (00:49 +0900)
committerLinus Walleij <linus.walleij@linaro.org>
Mon, 15 Feb 2016 19:45:50 +0000 (20:45 +0100)
Previously, samsung_gpio_drection_in/output function were not covered
with a spinlock.

For example, samsung_gpio_direction_output function consists of
two functions.
1. samsung_gpio_set
2. samsung_gpio_set_direction

When 2 CPUs try to control the same gpio pin heavily,
(situation like i2c control with gpio emulation)
This situation can cause below problem.

CPU 0                                   | CPU1
                                        |
samsung_gpio_direction_output           |
   samsung_gpio_set(pin A as 1)         | samsung_gpio_direction_output
                                        |    samsung_gpio_set(pin A as 0)
   samsung_gpio_set_direction           |
                                        |    samsung_gpio_set_direction

The initial value of pin A will be set as 0 while we wanted to set pin A as 1.

This patch modifies samsung_gpio_direction_in/output function
to be done in one spinlock to fix race condition.

Additionally, the new samsung_gpio_set_value was added to implement
gpio set callback(samsung_gpio_set) with spinlock using this function.

Cc: stable@vger.kernel.org
Signed-off-by: Youngmin Nam <ym0914@gmail.com>
Acked-by: Tomasz Figa <tomasz.figa@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
drivers/pinctrl/samsung/pinctrl-samsung.c

index f67b1e958589b9f9b4a891ce860bcfeb8909690d..5cc97f85db02d4986b3c9249a1116d952838fde0 100644 (file)
@@ -514,25 +514,35 @@ static const struct pinconf_ops samsung_pinconf_ops = {
        .pin_config_group_set   = samsung_pinconf_group_set,
 };
 
-/* gpiolib gpio_set callback function */
-static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
+/*
+ * The samsung_gpio_set_vlaue() should be called with "bank->slock" held
+ * to avoid race condition.
+ */
+static void samsung_gpio_set_value(struct gpio_chip *gc,
+                                         unsigned offset, int value)
 {
        struct samsung_pin_bank *bank = gpiochip_get_data(gc);
        const struct samsung_pin_bank_type *type = bank->type;
-       unsigned long flags;
        void __iomem *reg;
        u32 data;
 
        reg = bank->drvdata->virt_base + bank->pctl_offset;
 
-       spin_lock_irqsave(&bank->slock, flags);
-
        data = readl(reg + type->reg_offset[PINCFG_TYPE_DAT]);
        data &= ~(1 << offset);
        if (value)
                data |= 1 << offset;
        writel(data, reg + type->reg_offset[PINCFG_TYPE_DAT]);
+}
+
+/* gpiolib gpio_set callback function */
+static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
+{
+       struct samsung_pin_bank *bank = gpiochip_get_data(gc);
+       unsigned long flags;
 
+       spin_lock_irqsave(&bank->slock, flags);
+       samsung_gpio_set_value(gc, offset, value);
        spin_unlock_irqrestore(&bank->slock, flags);
 }
 
@@ -553,6 +563,8 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
 }
 
 /*
+ * The samsung_gpio_set_direction() should be called with "bank->slock" held
+ * to avoid race condition.
  * The calls to gpio_direction_output() and gpio_direction_input()
  * leads to this function call.
  */
@@ -564,7 +576,6 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc,
        struct samsung_pinctrl_drv_data *drvdata;
        void __iomem *reg;
        u32 data, mask, shift;
-       unsigned long flags;
 
        bank = gpiochip_get_data(gc);
        type = bank->type;
@@ -581,31 +592,42 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc,
                reg += 4;
        }
 
-       spin_lock_irqsave(&bank->slock, flags);
-
        data = readl(reg);
        data &= ~(mask << shift);
        if (!input)
                data |= FUNC_OUTPUT << shift;
        writel(data, reg);
 
-       spin_unlock_irqrestore(&bank->slock, flags);
-
        return 0;
 }
 
 /* gpiolib gpio_direction_input callback function. */
 static int samsung_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
 {
-       return samsung_gpio_set_direction(gc, offset, true);
+       struct samsung_pin_bank *bank = gpiochip_get_data(gc);
+       unsigned long flags;
+       int ret;
+
+       spin_lock_irqsave(&bank->slock, flags);
+       ret = samsung_gpio_set_direction(gc, offset, true);
+       spin_unlock_irqrestore(&bank->slock, flags);
+       return ret;
 }
 
 /* gpiolib gpio_direction_output callback function. */
 static int samsung_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
                                                        int value)
 {
-       samsung_gpio_set(gc, offset, value);
-       return samsung_gpio_set_direction(gc, offset, false);
+       struct samsung_pin_bank *bank = gpiochip_get_data(gc);
+       unsigned long flags;
+       int ret;
+
+       spin_lock_irqsave(&bank->slock, flags);
+       samsung_gpio_set_value(gc, offset, value);
+       ret = samsung_gpio_set_direction(gc, offset, false);
+       spin_unlock_irqrestore(&bank->slock, flags);
+
+       return ret;
 }
 
 /*