| Message ID | 1374748876-23461-1-git-send-email-maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Maxime Ripard, On Thu, 25 Jul 2013 12:41:16 +0200, Maxime Ripard wrote: > The current gpio_set function is ignoring the previous value set in the > GPIO value register, which leads in erasing the values already set for > the other GPIOs in the same bank when setting the value of a given GPIO. > > Add the usual read/mask/write pattern to fix this brown paper bag bug. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/pinctrl-sunxi.c b/drivers/pinctrl/pinctrl-sunxi.c > index fc058b6..4f8bb18 100644 > --- a/drivers/pinctrl/pinctrl-sunxi.c > +++ b/drivers/pinctrl/pinctrl-sunxi.c > @@ -464,8 +464,14 @@ static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, > struct sunxi_pinctrl *pctl = dev_get_drvdata(chip->dev); > u32 reg = sunxi_data_reg(offset); > u8 index = sunxi_data_offset(offset); > + u32 regval = readl(pctl->membase + reg); > > - writel((value & DATA_PINS_MASK) << index, pctl->membase + reg); > + if (value) > + regval |= BIT(index); > + else > + regval &= ~(BIT(index)); > + > + writel(regval, pctl->membase + reg); Hum, what about locking? Thomas
Hi Thomas, On Fri, Jul 26, 2013 at 12:54:45PM +0200, Thomas Petazzoni wrote: > On Thu, 25 Jul 2013 12:41:16 +0200, Maxime Ripard wrote: > > The current gpio_set function is ignoring the previous value set in the > > GPIO value register, which leads in erasing the values already set for > > the other GPIOs in the same bank when setting the value of a given GPIO. > > > > Add the usual read/mask/write pattern to fix this brown paper bag bug. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pinctrl/pinctrl-sunxi.c b/drivers/pinctrl/pinctrl-sunxi.c > > index fc058b6..4f8bb18 100644 > > --- a/drivers/pinctrl/pinctrl-sunxi.c > > +++ b/drivers/pinctrl/pinctrl-sunxi.c > > @@ -464,8 +464,14 @@ static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, > > struct sunxi_pinctrl *pctl = dev_get_drvdata(chip->dev); > > u32 reg = sunxi_data_reg(offset); > > u8 index = sunxi_data_offset(offset); > > + u32 regval = readl(pctl->membase + reg); > > > > - writel((value & DATA_PINS_MASK) << index, pctl->membase + reg); > > + if (value) > > + regval |= BIT(index); > > + else > > + regval &= ~(BIT(index)); > > + > > + writel(regval, pctl->membase + reg); > > Hum, what about locking? Right, I should probably use some spinlocks. I'll do a followup patch, because this chunk won't be the only area impacted obviously. Maxime
Hi Linus, On Thu, Jul 25, 2013 at 12:41:16PM +0200, Maxime Ripard wrote: > The current gpio_set function is ignoring the previous value set in the > GPIO value register, which leads in erasing the values already set for > the other GPIOs in the same bank when setting the value of a given GPIO. > > Add the usual read/mask/write pattern to fix this brown paper bag bug. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> This should probably go in 3.11. I'll send a patch adding locking like Thomas suggested, could you queue up this patch for 3.11? Thanks, Maxime
On Thu, Jul 25, 2013 at 12:41 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > The current gpio_set function is ignoring the previous value set in the > GPIO value register, which leads in erasing the values already set for > the other GPIOs in the same bank when setting the value of a given GPIO. > > Add the usual read/mask/write pattern to fix this brown paper bag bug. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Patch applied for fixes. Yours, Linus Walleij
diff --git a/drivers/pinctrl/pinctrl-sunxi.c b/drivers/pinctrl/pinctrl-sunxi.c index fc058b6..4f8bb18 100644 --- a/drivers/pinctrl/pinctrl-sunxi.c +++ b/drivers/pinctrl/pinctrl-sunxi.c @@ -464,8 +464,14 @@ static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, struct sunxi_pinctrl *pctl = dev_get_drvdata(chip->dev); u32 reg = sunxi_data_reg(offset); u8 index = sunxi_data_offset(offset); + u32 regval = readl(pctl->membase + reg); - writel((value & DATA_PINS_MASK) << index, pctl->membase + reg); + if (value) + regval |= BIT(index); + else + regval &= ~(BIT(index)); + + writel(regval, pctl->membase + reg); } static int sunxi_pinctrl_gpio_of_xlate(struct gpio_chip *gc,
The current gpio_set function is ignoring the previous value set in the GPIO value register, which leads in erasing the values already set for the other GPIOs in the same bank when setting the value of a given GPIO. Add the usual read/mask/write pattern to fix this brown paper bag bug. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)