diff mbox

twl4030-gpio: Fix getting the value of the TWL4030 GPIO output pin

Message ID 4e1455be0902062213o769a4d15ha0c892508839dbad@mail.gmail.com
State Rejected, archived
Headers show

Commit Message

Joonyoung Shim Feb. 7, 2009, 6:13 a.m. UTC
If TWL4030 GPIO pin is output, must read the value from REG_GPIODATAOUTx
register in twl4030_get_gpio_datainout().

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

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

Comments

David Brownell Feb. 7, 2009, 8 a.m. UTC | #1
On Friday 06 February 2009, Joonyoung Shim wrote:
> If TWL4030 GPIO pin is output, must read the value from REG_GPIODATAOUTx
> register in twl4030_get_gpio_datainout().

Why "must"?  The goal is to return the *actual* value of the pin,
not the intended one.  When an external signal overwhelms the drive
capability of that GPIO -- just a couple mA -- the correct value
to return can't be the value written in the output register.

Plus, this patch won't even link ... you changed the name of the
function at its definition, but not at its use points.

NAK, unless you can show there's a real bug this addresses...

- Dave
--
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
Joonyoung Shim Feb. 9, 2009, 1:02 p.m. UTC | #2
2009/2/7 David Brownell <david-b@pacbell.net>:
> On Friday 06 February 2009, Joonyoung Shim wrote:
>> If TWL4030 GPIO pin is output, must read the value from REG_GPIODATAOUTx
>> register in twl4030_get_gpio_datainout().
>
> Why "must"?  The goal is to return the *actual* value of the pin,
> not the intended one.  When an external signal overwhelms the drive
> capability of that GPIO -- just a couple mA -- the correct value
> to return can't be the value written in the output register.

but, it doesn't return actual value of the pin if twl4030 GPIO pin is output.

For example, the GPIO13 pin of twl4030 is configured as an output
using gpio_direction_output() and assigns the GPIO's value to 1
using gpio_set_value_cansleep(), the result of "# cat /sys/kernel/debug/gpio"
reports to me as follows.

GPIOs 192-209, platform/twl4030_gpio, twl4030, can sleep:
 gpio-205 (keyled              ) out lo

but it should have reported "hi" instead of "lo".


>
> Plus, this patch won't even link ... you changed the name of the
> function at its definition, but not at its use points.

Sorry, i missed some code.

>
> NAK, unless you can show there's a real bug this addresses...
>
> - Dave
>
--
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
David Brownell Feb. 12, 2009, 2:16 a.m. UTC | #3
On Monday 09 February 2009, Joonyoung Shim wrote:
> but, it doesn't return actual value of the pin if twl4030 GPIO pin is output.

So it would seem that these GPIOs don't support
bidirectional usage.  Documentation/gpio.txt includes:

>>>		However, note that not all platforms can
>>>	read the value of output pins; those that can't
>>>	should always return zero.  

Which is exactly what's happening here, it seems...




> For example, the GPIO13 pin of twl4030 is configured as an output
> using gpio_direction_output() and assigns the GPIO's value to 1
> using gpio_set_value_cansleep(), the result of "# cat /sys/kernel/debug/gpio"
> reports to me as follows.
> 
> GPIOs 192-209, platform/twl4030_gpio, twl4030, can sleep:
>  gpio-205 (keyled              ) out lo
> 
> but it should have reported "hi" instead of "lo".
> 


--
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
diff mbox

Patch

diff --git a/drivers/gpio/twl4030-gpio.c b/drivers/gpio/twl4030-gpio.c
index afad147..890518a 100644
--- a/drivers/gpio/twl4030-gpio.c
+++ b/drivers/gpio/twl4030-gpio.c
@@ -183,7 +183,7 @@  static int twl4030_set_gpio_dataout(int gpio, int enable)
 	return gpio_twl4030_write(base, d_msk);
 }

-static int twl4030_get_gpio_datain(int gpio)
+static int twl4030_get_gpio_datainout(int gpio)
 {
 	u8 d_bnk = gpio >> 3;
 	u8 d_off = gpio & 0x7;
@@ -194,7 +194,17 @@  static int twl4030_get_gpio_datain(int gpio)
 		|| !(gpio_usage_count & BIT(gpio))))
 		return -EPERM;

-	base = REG_GPIODATAIN1 + d_bnk;
+	base = REG_GPIODATADIR1 + d_bnk;
+	ret = gpio_twl4030_read(base);
+	if (ret > 0)
+		ret = (ret >> d_off) & 0x1;
+	else
+		return ret;
+
+	if (ret)
+		base = REG_GPIODATAOUT1 + d_bnk;
+	else
+		base = REG_GPIODATAIN1 + d_bnk;
 	ret = gpio_twl4030_read(base);
 	if (ret > 0)
 		ret = (ret >> d_off) & 0x1;