* [PATCH] pinctrl: sunxi: Fix gpio_set behaviour
@ 2013-07-25 10:41 Maxime Ripard
2013-07-26 10:54 ` Thomas Petazzoni
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Maxime Ripard @ 2013-07-25 10:41 UTC (permalink / raw)
To: linux-arm-kernel
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);
}
static int sunxi_pinctrl_gpio_of_xlate(struct gpio_chip *gc,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: sunxi: Fix gpio_set behaviour
2013-07-25 10:41 [PATCH] pinctrl: sunxi: Fix gpio_set behaviour Maxime Ripard
@ 2013-07-26 10:54 ` Thomas Petazzoni
2013-07-26 13:26 ` Maxime Ripard
2013-08-02 10:12 ` Maxime Ripard
2013-08-07 19:57 ` Linus Walleij
2 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2013-07-26 10:54 UTC (permalink / raw)
To: linux-arm-kernel
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
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: sunxi: Fix gpio_set behaviour
2013-07-26 10:54 ` Thomas Petazzoni
@ 2013-07-26 13:26 ` Maxime Ripard
0 siblings, 0 replies; 5+ messages in thread
From: Maxime Ripard @ 2013-07-26 13:26 UTC (permalink / raw)
To: linux-arm-kernel
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
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130726/6e3654a2/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: sunxi: Fix gpio_set behaviour
2013-07-25 10:41 [PATCH] pinctrl: sunxi: Fix gpio_set behaviour Maxime Ripard
2013-07-26 10:54 ` Thomas Petazzoni
@ 2013-08-02 10:12 ` Maxime Ripard
2013-08-07 19:57 ` Linus Walleij
2 siblings, 0 replies; 5+ messages in thread
From: Maxime Ripard @ 2013-08-02 10:12 UTC (permalink / raw)
To: linux-arm-kernel
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
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130802/1279d13b/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] pinctrl: sunxi: Fix gpio_set behaviour
2013-07-25 10:41 [PATCH] pinctrl: sunxi: Fix gpio_set behaviour Maxime Ripard
2013-07-26 10:54 ` Thomas Petazzoni
2013-08-02 10:12 ` Maxime Ripard
@ 2013-08-07 19:57 ` Linus Walleij
2 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2013-08-07 19:57 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-08-07 19:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-25 10:41 [PATCH] pinctrl: sunxi: Fix gpio_set behaviour Maxime Ripard
2013-07-26 10:54 ` Thomas Petazzoni
2013-07-26 13:26 ` Maxime Ripard
2013-08-02 10:12 ` Maxime Ripard
2013-08-07 19:57 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.