[U-Boot] [PATCH] 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 klaus.goger@theobroma-systems.com --- drivers/gpio/mxc_gpio.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 6a572d5..5838fc2 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -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(®s->gpio_dir) >> gpio & 0x01) + val = (readl(®s->gpio_dr) >> gpio & 0x01); + else + val = (readl(®s->gpio_psr) >> gpio) & 0x01; +#else val = (readl(®s->gpio_psr) >> gpio) & 0x01; +#endif
return val; }

Hi Klaus,
On 23/06/2014 08:35, Klaus Goger wrote:
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 benoit.thebaudeau@advansee.com Date: Mon Aug 20 10:55:41 2012 +0000
mxc: Make gpio_get_value() use PSR
Signed-off-by: Klaus Goger klaus.goger@theobroma-systems.com
drivers/gpio/mxc_gpio.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 6a572d5..5838fc2 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -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(®s->gpio_dir) >> gpio & 0x01)
val = (readl(®s->gpio_dr) >> gpio & 0x01);
- else
val = (readl(®s->gpio_psr) >> gpio) & 0x01;
+#else val = (readl(®s->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

On Tue, Jun 24, 2014 at 10:07 AM, Stefano Babic sbabic@denx.de wrote:
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 otavio@ossystems.com.br 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 benoit.thebaudeau@advansee.com, Fabio Estevam fabio.estevam@freescale.com and Eric Bénard eric@eukrea.com for helping to properly trace this down.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br Acked-by: Stefano Babic sbabic@denx.de

On Jun 24, 2014, at 3:13 PM, Fabio Estevam festevam@gmail.com wrote:
On Tue, Jun 24, 2014 at 10:07 AM, Stefano Babic sbabic@denx.de wrote:
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 otavio@ossystems.com.br 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
participants (3)
-
Fabio Estevam
-
Klaus Goger
-
Stefano Babic