Hello tomasz and linus,
I got to know that below my v3 patch has build error now.
I will fix it and resend.
Thanks.
On 2016년 02월 08일 23:54, Youngmin Nam wrote:
> Previously, samsung_gpio_drection_in/output function were not covered with
> one 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.
>
> Signed-off-by: Youngmin Nam <ym0914@xxxxxxxxx>
> Acked-by: Tomasz Figa <tomasz.figa@xxxxxxxxx>
> ---
> drivers/pinctrl/samsung/pinctrl-samsung.c | 48 ++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> index f67b1e9..38142c2 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -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 = gc_to_pin_bank(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 = gc_to_pin_bank(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 = gc_to_pin_bank(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;
> }
>
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
