[U-Boot] [PATCH] mx6: gpio: read data register if direction is out

[U-Boot] [PATCH] mx6: gpio: read data register if direction is out

[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.

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(&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;
 }
-- 
1.9.0

[U-Boot] [PATCH] mx6: gpio: read data register if direction is out

[Stefano Babic]

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(&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


-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH] mx6: gpio: read data register if direction is out

[Fabio Estevam]

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>

[U-Boot] [PATCH] mx6: gpio: read data register if direction is out

[Klaus Goger]

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