Skip to contentSkip to conversation

pinctrl: sunxi: Fix gpio_set behaviour

PATCH New Maxime Ripard 3 participants 5 messages Jul 25, 2013 – Aug 7, 2013

Subsystems (2)
Arm/Allwinner sunXi SoC support Pin control
lore
Share & tools
lore

Maxime Ripard

author
#0 lore raw

PATCH 1/1 pinctrl: sunxi: Fix gpio_set behaviour

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 <[email protected]>
---
 drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/pinctrl/pinctrl-sunxi.c +7 -1
index fc058b6..4f8bb18 100644
@@ -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,
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/1 pinctrl: sunxi: Fix gpio_set behaviour

Dear Maxime Ripard,

Quoted 18 lines · Maxime Ripard

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 <[email protected]>
---
 drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/pinctrl/pinctrl-sunxi.c +0 -0
index fc058b6..4f8bb18 100644
@@ -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);

drivers/pinctrl/pinctrl-sunxi.c static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, · line 474

Quoted 10 lines
drivers/pinctrl/pinctrl-sunxi.c +7 -1
 	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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#2 lore raw

PATCH 1/1 pinctrl: sunxi: Fix gpio_set behaviour

Hi Thomas,

Quoted 22 lines · Thomas Petazzoni
Quoted 21 lines · Maxime Ripard

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 <[email protected]>
---
 drivers/pinctrl/pinctrl-sunxi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/pinctrl/pinctrl-sunxi.c +1 -0
index fc058b6..4f8bb18 100644
@@ -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);
Quoted 9 lines

drivers/pinctrl/pinctrl-sunxi.c static void sunxi_pinctrl_gpio_set(struct gpio_chip *chip, · line 474

Quoted 7 lines
drivers/pinctrl/pinctrl-sunxi.c +6 -1
-	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

Signature · 10 lines
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>
author
#3 lore raw

PATCH 1/1 pinctrl: sunxi: Fix gpio_set behaviour

In reply to Maxime Ripard

Hi Linus,

Quoted 7 lines · Maxime Ripard

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 <[email protected]>

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

Signature · 10 lines
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>
#4 lore raw

PATCH 1/1 pinctrl: sunxi: Fix gpio_set behaviour

In reply to Maxime Ripard
Quoted 7 lines · Maxime Ripard

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 <[email protected]>

Patch applied for fixes.

Yours,
Linus Walleij