diff mbox

gpio: twl4030: Correct status reporting when the GPIO is used as output

Message ID 1354700985-4453-1-git-send-email-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi Dec. 5, 2012, 9:49 a.m. UTC
When the GPIO is configured as output we need to read the GPIODATAOUT*
register to get correct information. When the GPIO is output the GPIODATAIN*
registers report 0 all the time (no feedback from output path).

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/gpio/gpio-twl4030.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Grant Likely Dec. 5, 2012, 10:48 p.m. UTC | #1
On Wed, 5 Dec 2012 10:49:45 +0100, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> When the GPIO is configured as output we need to read the GPIODATAOUT*
> register to get correct information. When the GPIO is output the GPIODATAIN*
> registers report 0 all the time (no feedback from output path).
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/gpio/gpio-twl4030.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
> index 55b4fe4..e7aa620 100644
> --- a/drivers/gpio/gpio-twl4030.c
> +++ b/drivers/gpio/gpio-twl4030.c
> @@ -191,13 +191,19 @@ static int twl4030_get_gpio_datain(int gpio)
>  	u8 d_bnk = gpio >> 3;
>  	u8 d_off = gpio & 0x7;
>  	u8 base = 0;
> +	int direction;
>  	int ret = 0;
>  
>  	if (unlikely((gpio >= TWL4030_GPIO_MAX)
>  		|| !(gpio_usage_count & BIT(gpio))))
>  		return -EPERM;
>  
> -	base = REG_GPIODATAIN1 + d_bnk;
> +	direction = gpio_twl4030_read(REG_GPIODATADIR1 + d_bnk);
> +	if (direction > 0 && (direction >> d_off) & 0x1)
> +		base = REG_SETGPIODATAOUT1 + d_bnk;
> +	else
> +		base = REG_GPIODATAIN1 + d_bnk;
> +

This is probably quite expensive considering that reads need to go out
the i2c bus. Things like the output state and the pin direction should
be cached by the driver in its private data structure so that you don't
add an additional i2c round trip.

g.

--
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
Peter Ujfalusi Dec. 6, 2012, 10:55 a.m. UTC | #2
On 12/05/2012 11:48 PM, Grant Likely wrote:
>> -	base = REG_GPIODATAIN1 + d_bnk;
>> +	direction = gpio_twl4030_read(REG_GPIODATADIR1 + d_bnk);
>> +	if (direction > 0 && (direction >> d_off) & 0x1)
>> +		base = REG_SETGPIODATAOUT1 + d_bnk;
>> +	else
>> +		base = REG_GPIODATAIN1 + d_bnk;
>> +
> 
> This is probably quite expensive considering that reads need to go out
> the i2c bus. Things like the output state and the pin direction should
> be cached by the driver in its private data structure so that you don't
> add an additional i2c round trip.

True. I have sent the v2, which grown a bit since I did not wanted to add more
global variable to this driver.
diff mbox

Patch

diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index 55b4fe4..e7aa620 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -191,13 +191,19 @@  static int twl4030_get_gpio_datain(int gpio)
 	u8 d_bnk = gpio >> 3;
 	u8 d_off = gpio & 0x7;
 	u8 base = 0;
+	int direction;
 	int ret = 0;
 
 	if (unlikely((gpio >= TWL4030_GPIO_MAX)
 		|| !(gpio_usage_count & BIT(gpio))))
 		return -EPERM;
 
-	base = REG_GPIODATAIN1 + d_bnk;
+	direction = gpio_twl4030_read(REG_GPIODATADIR1 + d_bnk);
+	if (direction > 0 && (direction >> d_off) & 0x1)
+		base = REG_SETGPIODATAOUT1 + d_bnk;
+	else
+		base = REG_GPIODATAIN1 + d_bnk;
+
 	ret = gpio_twl4030_read(base);
 	if (ret > 0)
 		ret = (ret >> d_off) & 0x1;