Skip to contentSkip to conversation

mx6: gpio: read data register if direction is out

PATCH New Klaus Goger 3 participants 4 messages Jun 23, 2014 – Jun 24, 2014

Subsystems (2)
Arm i.MX and Layerscape GPIO
lore
Share & tools
lore

Klaus Goger

author
#0 lore raw

PATCH 1/1 mx6: gpio: read data register if direction is out

On i.MX6 GPIOx_PSR does not reflect the current output value if the direction is set to output. Instead we should read GPIOx_DR.

Signed-off-by Klaus Goger <[email protected]>
---
 drivers/gpio/mxc_gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/gpio/mxc_gpio.c +9 -1
index 6a572d5..5838fc2 100644
@@ -101,8 +101,16 @@ int gpio_get_value(unsigned gpio)
 	gpio &= 0x1f;
 
 	regs = (struct gpio_regs *)gpio_ports[port];
-
+#if defined(CONFIG_MX6)
+	/* if the direction is set to output we will always read 0 as pad status.
+	 * so we have to read the data register to get the current output state */
+	if (readl(&regs->gpio_dir) >> gpio & 0x01)
+		val = (readl(&regs->gpio_dr) >> gpio & 0x01);
+	else
+		val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
+#else
 	val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
+#endif
 
 	return val;
 }
Signature · 1 line
1.9.0
#1 lore raw

PATCH 1/1 mx6: gpio: read data register if direction is out

Hi Klaus,

Quoted 3 lines · Klaus Goger

On i.MX6 GPIOx_PSR does not reflect the current output value if the direction is set to output. Instead we should read GPIOx_DR.

This conflicts with commit :

commit 5dafa4543c399d329c7b01df1afa98437861cac0

Author: Beno?t Th?baudeau <[email protected]>
Date:   Mon Aug 20 10:55:41 2012 +0000

mxc: Make gpio_get_value() use PSR

Quoted 14 lines
Signed-off-by Klaus Goger <[email protected]>
---
 drivers/gpio/mxc_gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/gpio/mxc_gpio.c +0 -1
index 6a572d5..5838fc2 100644
@@ -101,8 +101,16 @@ int gpio_get_value(unsigned gpio)
 	gpio &= 0x1f;
 
 	regs = (struct gpio_regs *)gpio_ports[port];
-

drivers/gpio/mxc_gpio.c int gpio_get_value(unsigned gpio) · line 113

Quoted 10 lines
drivers/gpio/mxc_gpio.c +9 -0
+#if defined(CONFIG_MX6)
+	/* if the direction is set to output we will always read 0 as pad status.
+	 * so we have to read the data register to get the current output state */
+	if (readl(&regs->gpio_dir) >> gpio & 0x01)
+		val = (readl(&regs->gpio_dr) >> gpio & 0x01);
+	else
+		val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
+#else
 	val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
+#endif

According to the documentation, I disagree with you. What I read from the MX6 manual is:

28.4.2.1 GPIO Read Mode

The programming sequence for reading input signals should be as follows:
1. Configure IOMUX to select GPIO mode (Via IOMUX Controller (IOMUXC) ).
2. Configure GPIO direction register to input (GPIO_GDIR[GDIR] set to 0b).
3. Read value from data register/pad status register.

What you are describing looks an issue in your pinmux configuration because you do not set the SION bit, as requested to read back the value of the GPIO from the Pad Register.

I assume that if you read the value from the DR register, you do not read the current value of the pin, but simply you get what you have programmed, that could be different if you have a conflict on the pin itself.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
reviewer
#2 lore raw

PATCH 1/1 mx6: gpio: read data register if direction is out

Quoted 3 lines · Stefano Babic

What you are describing looks an issue in your pinmux configuration because you do not set the SION bit, as requested to read back the value of the GPIO from the Pad Register.

Yes, correct.

Klaus, please check this commit:

commit 7773fd196918826ebaab769e63a4775607f5256c

Author: Otavio Salvador <[email protected]>
Date:   Mon Dec 16 20:44:00 2013 -0200

imx: Easy enabling of SION per-pin using MUX_MODE_SION helper macro

The macro allows easy setting in per-pin, as for example:

,----
| imx_iomux_v3_setup_pad(MX6_PAD_NANDF_D1__GPIO_2_1 | MUX_MODE_SION); `----

The IOMUX_CONFIG_SION allows for reading PAD value from PSR register.

The following quote from the datasheet:

,----
| ...
| 28.4.2.2 GPIO Write Mode
| The programming sequence for driving output signals should be as follows:
| 1. Configure IOMUX to select GPIO mode (Via IOMUXC), also enable
SION if need
| to read loopback pad value through PSR
| 2. Configure GPIO direction register to output (GPIO_GDIR[GDIR]
set to 1b).
| 3. Write value to data register (GPIO_DR).
| ...
`----

This fixes the gpio_get_value to properly work when a GPIO is set for output and has no conflicts.

Thanks for Beno?t Th?baudeau <[email protected]>, Fabio Estevam <[email protected]> and Eric B?nard <[email protected]> for helping to properly trace this down.

Signed-off-by Otavio Salvador <[email protected]>
Acked-by Stefano Babic <[email protected]>
author
#3 lore raw

PATCH 1/1 mx6: gpio: read data register if direction is out

Quoted 8 lines · Fabio Estevam
Quoted 3 lines · Stefano Babic

What you are describing looks an issue in your pinmux configuration because you do not set the SION bit, as requested to read back the value of the GPIO from the Pad Register.

Yes, correct.

Quoted 7 lines

Klaus, please check this commit:

commit 7773fd196918826ebaab769e63a4775607f5256c

Author: Otavio Salvador <[email protected]>
Date:   Mon Dec 16 20:44:00 2013 -0200

imx: Easy enabling of SION per-pin using MUX_MODE_SION helper macro

Using the MUX_MODE_SION macro helped indeed.
Thank you Stefano and Fabio.

Best Regards,
Klaus