[PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[Joonyoung Shim]

If the GPIO pin is output, must read the value from OMAP24XX_GPIO_DATAOUT
register in __omap_get_gpio_datain().

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index f856a90..296773a 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -428,7 +428,11 @@ static int __omap_get_gpio_datain(int gpio)
 #endif
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
 	case METHOD_GPIO_24XX:
-		reg += OMAP24XX_GPIO_DATAIN;
+		if (__raw_readl(reg + OMAP24XX_GPIO_OE)
+					& (1 << get_gpio_index(gpio)))
+			reg += OMAP24XX_GPIO_DATAIN;
+		else
+			reg += OMAP24XX_GPIO_DATAOUT;
 		break;
 #endif
 	default:

Re: [PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[David Brownell]

On Friday 06 February 2009, Joonyoung Shim wrote:
> If the GPIO pin is output, must read the value from OMAP24XX_GPIO_DATAOUT
> register in __omap_get_gpio_datain().

Same comment as with the similar twl4030 patch you sent:
the API specifies that the value reported for input should
be the actual value AT THE PIN, even for outputs ... this
patch would change the behavior to report the value the pin
is trying to drive, which isn't necessarily going to reflect
the signal value at the pin.

So, NAK.

In a few rare cases on OMAP3 chips -- not currently used
by Linux -- GPIOs can be output-only (input drivers not
enabled).  Such a change might be appropriate for GPIOs
configured that way ... were it not for the fact that in
those cases, a value of zero should be returned.

- Dave



> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
> 
> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> index f856a90..296773a 100644
> --- a/arch/arm/plat-omap/gpio.c
> +++ b/arch/arm/plat-omap/gpio.c
> @@ -428,7 +428,11 @@ static int __omap_get_gpio_datain(int gpio)
>  #endif
>  #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
>  	case METHOD_GPIO_24XX:
> -		reg += OMAP24XX_GPIO_DATAIN;
> +		if (__raw_readl(reg + OMAP24XX_GPIO_OE)
> +					& (1 << get_gpio_index(gpio)))
> +			reg += OMAP24XX_GPIO_DATAIN;
> +		else
> +			reg += OMAP24XX_GPIO_DATAOUT;
>  		break;
>  #endif
>  	default:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[Joonyoung Shim]

Sorry, I added CC.

2009/2/9 David Brownell <david-b@pacbell.net>:
> On Sunday 08 February 2009, Joonyoung Shim wrote:
>> I have knew if gpio pin is configured as an output, the value of the
>> corresponding bit
>> in the GPIO_DATAOUT register is driven on the corresponding gpio pin.
>> But the implemented API only reports the value of the corresponding bit in the
>> GPIO_DATAIN register. This can report a wrong value.
>>
>> For example, even though the gpio 141pin is configured as an output and assigns
>> the gpio's value to 1 through gpio_set_value(), the result of
>> "# cat /sys/kernel/debug/gpio" reports to me as follows.
>>
>> GPIOs 128-159, gpio:
>>  gpio-141 (motor enable        ) out lo
>>
>> but it should have reported "hi" instead of "lo".
>
> I happen to observe that mach-omap2/mux.c there are a bunch
> of pins that are wrongly multiplexed.  Like:
>
> MUX_CFG_34XX("AE6_34XX_GPIO141", 0x16e,
>                OMAP34XX_MUX_MODE4 | OMAP34XX_PIN_OUTPUT)

I think that it is right because i want to use gpio141 pin as output,

>
> Which should clearly be AE6_34XX_GPIO141_OUT since it does
> not enable the input driver.  Try making that ...PIN_OUTPUT
> read ...PIN_INPUT and see if things behave.

Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT.
but, the core of this problem is not it, the chip->get() called in
gpiolib_dbg_show()
of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c
can return the wrong value when gpio pin is configured as an output.

the reason as i say at former email, is because __omap_get_gpio_datain()
called by gpio_get() of arch/arm/plat-omap/gpio.c reports only value of
OMAP24XX_GPIO_DATAIN register.

- Joonyoung

>
> - Dave
>
>
>

Re: [PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[David Brownell]

On Sunday 08 February 2009, Joonyoung Shim wrote:
> > Which should clearly be AE6_34XX_GPIO141_OUT since it does
> > not enable the input driver.  Try making that ...PIN_OUTPUT
> > read ...PIN_INPUT and see if things behave.
> 
> Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT.
> but, the core of this problem is not it, the chip->get() called in
> gpiolib_dbg_show()
> of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c
> can return the wrong value when gpio pin is configured as an output.

No, the core of the problem is that you're using a GPIO
which is explicitly configured as output-only ... but are
treating it as if it were a normal bi-directional GPIO.

It's acting *exactly* right for an output-only GPIO.  But
that's not the behavior you expect, or want.

If you weren't disabling the input driver, then when you
ask for the input value it would work as you expect.  Which
is why I suggested you change how that ball is configured.
(That's actually a bugfix to those muxing tables ...)

- Dave

p.s. This specific confusion is new on OMAP3; previous
     OMAPs didn't have the extra byte of options to
     configure.  The PIN_INPUT flag was, on previous
     chips, effectively hard-wired on for GPIOs.

Re: [PATCH] OMAP3 GPIO: Fix getting the value of the GPIO output pin

[Joonyoung Shim]

2009/2/9 David Brownell <david-b@pacbell.net>:
> On Sunday 08 February 2009, Joonyoung Shim wrote:
>> > Which should clearly be AE6_34XX_GPIO141_OUT since it does
>> > not enable the input driver.  Try making that ...PIN_OUTPUT
>> > read ...PIN_INPUT and see if things behave.
>>
>> Of course, it will be better if the name is clear as AE6_34XX_GPIO141_OUT.
>> but, the core of this problem is not it, the chip->get() called in
>> gpiolib_dbg_show()
>> of drivers/gpio/gpiolib.c or dbg_gpio_show() of arch/arm/plat-omap/gpio.c
>> can return the wrong value when gpio pin is configured as an output.
>
> No, the core of the problem is that you're using a GPIO
> which is explicitly configured as output-only ... but are
> treating it as if it were a normal bi-directional GPIO.
>
> It's acting *exactly* right for an output-only GPIO.  But
> that's not the behavior you expect, or want.
>
> If you weren't disabling the input driver, then when you
> ask for the input value it would work as you expect.  Which
> is why I suggested you change how that ball is configured.
> (That's actually a bugfix to those muxing tables ...)

I understood that you say, ths PIN_OUTPUT flag must use
when GPIO pin is output-only, and i got the result i want
after GPIO pin is configured as bi-diretional GPIO.

But i wonder why don't control in gpio_get() of arch/arm/plat-omap/gpio.c
when the GPIO pin is output-only?

thank you.

- Joonyoung

>
> - Dave
>
> p.s. This specific confusion is new on OMAP3; previous
>     OMAPs didn't have the extra byte of options to
>     configure.  The PIN_INPUT flag was, on previous
>     chips, effectively hard-wired on for GPIOs.
>