* [PATCH 1/2] input: adp5588-keys - get value from data out when dir out
@ 2012-04-20 15:32 Jean-François Dagenais
2012-04-20 15:32 ` [PATCH 2/2] gpio: adp5588 " Jean-François Dagenais
2012-04-21 6:47 ` [PATCH 1/2] input: adp5588-keys " Dmitry Torokhov
0 siblings, 2 replies; 5+ messages in thread
From: Jean-François Dagenais @ 2012-04-20 15:32 UTC (permalink / raw)
To: grant.likely, dtor, dmitry.torokhov, linus.walleij, linux-kernel
Cc: linux-input, michael.hennerich, Jean-François Dagenais
As discussed here: http://ez.analog.com/message/35852,
the 5587 revC and 5588 revB spec sheets contain a mistake
in the GPIO_DAT_STATx register description.
According to R.Shnell at ADI, as well as my own
observations, it should read:
"GPIO data status (shows GPIO state when read for inputs)".
This commit changes the get value function accordingly.
A similar patch for gpio-adp5588 follows.
Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
---
drivers/input/keyboard/adp5588-keys.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 6412ced..b7a0f1a 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -78,6 +78,13 @@ static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned off)
unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
+ mutex_lock(&kpad->gpio_lock);
+ if (kpad->dir[bank] & bit) {
+ int result = !!(kpad->dat_out[bank] & bit);
+ mutex_unlock(&kpad->gpio_lock);
+ return result;
+ }
+ mutex_unlock(&kpad->gpio_lock);
return !!(adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank) & bit);
}
--
1.7.9
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] gpio: adp5588 - get value from data out when dir out
2012-04-20 15:32 [PATCH 1/2] input: adp5588-keys - get value from data out when dir out Jean-François Dagenais
@ 2012-04-20 15:32 ` Jean-François Dagenais
2012-04-21 6:47 ` [PATCH 1/2] input: adp5588-keys " Dmitry Torokhov
1 sibling, 0 replies; 5+ messages in thread
From: Jean-François Dagenais @ 2012-04-20 15:32 UTC (permalink / raw)
To: grant.likely, dtor, dmitry.torokhov, linus.walleij, linux-kernel
Cc: linux-input, michael.hennerich, Jean-François Dagenais
As discussed here: http://ez.analog.com/message/35852,
the 5587 revC and 5588 revB spec sheets contain a mistake
in the GPIO_DAT_STATx register description.
According to R.Shnell at ADI, as well as my own
observations, it should read:
"GPIO data status (shows GPIO state when read for inputs)".
This commit changes the get value function accordingly.
Follows a similar patch for adp5588-keys.
Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
---
drivers/gpio/gpio-adp5588.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 3122713..293ddb6 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -65,11 +65,23 @@ static int adp5588_gpio_write(struct i2c_client *client, u8 reg, u8 val)
static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned off)
{
+ unsigned bank, bit;
struct adp5588_gpio *dev =
container_of(chip, struct adp5588_gpio, gpio_chip);
+ bank = ADP5588_BANK(off);
+ bit = ADP5588_BIT(off);
+
+ mutex_lock(&dev->lock);
+ if (dev->dir[bank] & bit) {
+ int result = !!(dev->dat_out[bank] & bit);
+ mutex_unlock(&dev->lock);
+ return result;
+ }
+ mutex_unlock(&dev->lock);
+
return !!(adp5588_gpio_read(dev->client,
- GPIO_DAT_STAT1 + ADP5588_BANK(off)) & ADP5588_BIT(off));
+ GPIO_DAT_STAT1 + bank) & bit);
}
static void adp5588_gpio_set_value(struct gpio_chip *chip,
--
1.7.9
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] input: adp5588-keys - get value from data out when dir out
2012-04-20 15:32 [PATCH 1/2] input: adp5588-keys - get value from data out when dir out Jean-François Dagenais
2012-04-20 15:32 ` [PATCH 2/2] gpio: adp5588 " Jean-François Dagenais
@ 2012-04-21 6:47 ` Dmitry Torokhov
2012-04-24 13:33 ` Jean-Francois Dagenais
1 sibling, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2012-04-21 6:47 UTC (permalink / raw)
To: Jean-François Dagenais
Cc: grant.likely, linus.walleij, linux-kernel, linux-input,
michael.hennerich
Hi Jean-François,
On Fri, Apr 20, 2012 at 11:32:00AM -0400, Jean-François Dagenais wrote:
> As discussed here: http://ez.analog.com/message/35852,
> the 5587 revC and 5588 revB spec sheets contain a mistake
> in the GPIO_DAT_STATx register description.
>
> According to R.Shnell at ADI, as well as my own
> observations, it should read:
> "GPIO data status (shows GPIO state when read for inputs)".
>
> This commit changes the get value function accordingly.
>
> A similar patch for gpio-adp5588 follows.
>
> Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
> drivers/input/keyboard/adp5588-keys.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index 6412ced..b7a0f1a 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -78,6 +78,13 @@ static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned off)
> unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
> unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
>
> + mutex_lock(&kpad->gpio_lock);
> + if (kpad->dir[bank] & bit) {
> + int result = !!(kpad->dat_out[bank] & bit);
> + mutex_unlock(&kpad->gpio_lock);
> + return result;
> + }
> + mutex_unlock(&kpad->gpio_lock);
> return !!(adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank) & bit);
This locking looks wrong as it is possible for adp5588_gpio_get_value()
to get scheduled out after checking kpad->dir[bank] and releasing
kpad->gpio_lock while another thread executes
adp5588_gpio_direction_output() and modifies kpad->dir[bank].
You should keep mutex for the entire duration; something like this:
mutex_lock(&kpad->gpio_lock);
if (kpad->dir[bank] & bit)
val = kpad->dat_out[bank];
else
val = adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank);
mutex_unlock(&kpad->gpio_lock);
return !!(val & bit);
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] input: adp5588-keys - get value from data out when dir out
2012-04-21 6:47 ` [PATCH 1/2] input: adp5588-keys " Dmitry Torokhov
@ 2012-04-24 13:33 ` Jean-Francois Dagenais
0 siblings, 0 replies; 5+ messages in thread
From: Jean-Francois Dagenais @ 2012-04-24 13:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: grant.likely, linus.walleij, linux-kernel, linux-input,
michael.hennerich
On Apr 21, 2012, at 2:47, Dmitry Torokhov wrote:
> Hi Jean-François,
>
> On Fri, Apr 20, 2012 at 11:32:00AM -0400, Jean-François Dagenais wrote:
>> As discussed here: http://ez.analog.com/message/35852,
>> the 5587 revC and 5588 revB spec sheets contain a mistake
>> in the GPIO_DAT_STATx register description.
>>
>> According to R.Shnell at ADI, as well as my own
>> observations, it should read:
>> "GPIO data status (shows GPIO state when read for inputs)".
>>
>> This commit changes the get value function accordingly.
>>
>> A similar patch for gpio-adp5588 follows.
>>
>> Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
>> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
>> ---
>> drivers/input/keyboard/adp5588-keys.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
>> index 6412ced..b7a0f1a 100644
>> --- a/drivers/input/keyboard/adp5588-keys.c
>> +++ b/drivers/input/keyboard/adp5588-keys.c
>> @@ -78,6 +78,13 @@ static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned off)
>> unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
>> unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
>>
>> + mutex_lock(&kpad->gpio_lock);
>> + if (kpad->dir[bank] & bit) {
>> + int result = !!(kpad->dat_out[bank] & bit);
>> + mutex_unlock(&kpad->gpio_lock);
>> + return result;
>> + }
>> + mutex_unlock(&kpad->gpio_lock);
>> return !!(adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank) & bit);
>
> This locking looks wrong as it is possible for adp5588_gpio_get_value()
> to get scheduled out after checking kpad->dir[bank] and releasing
> kpad->gpio_lock while another thread executes
> adp5588_gpio_direction_output() and modifies kpad->dir[bank].
>
> You should keep mutex for the entire duration; something like this:
>
> mutex_lock(&kpad->gpio_lock);
> if (kpad->dir[bank] & bit)
> val = kpad->dat_out[bank];
> else
> val = adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank);
> mutex_unlock(&kpad->gpio_lock);
>
> return !!(val & bit);
>
Agreed, I think I wanted to minimize refactoring a bit too much and got distracted. I will send V2 shortly for both drivers.
> Thanks.
Thank you!
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] input: adp5588-keys - get value from data out when dir out
@ 2012-01-09 15:05 Jean-François Dagenais
0 siblings, 0 replies; 5+ messages in thread
From: Jean-François Dagenais @ 2012-01-09 15:05 UTC (permalink / raw)
To: linux-kernel; +Cc: michael.hennerich, grant.likely, Jean-François Dagenais
As discussed here: http://ez.analog.com/message/35852,
the 5587 revC and 5588 revB spec sheets contain a mistake
in the GPIO_DAT_STATx register description.
According to R.Shnell at ADI, as well as my own
observations, it should read:
"GPIO data status (shows GPIO state when read for inputs)".
This commit changes the get value function accordingly.
A similar patch for gpio-adp5588 follows.
Signed-off-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
---
drivers/input/keyboard/adp5588-keys.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 6412ced..b7a0f1a 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -78,6 +78,13 @@ static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned off)
unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
+ mutex_lock(&kpad->gpio_lock);
+ if (kpad->dir[bank] & bit) {
+ int result = !!(kpad->dat_out[bank] & bit);
+ mutex_unlock(&kpad->gpio_lock);
+ return result;
+ }
+ mutex_unlock(&kpad->gpio_lock);
return !!(adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank) & bit);
}
--
1.7.8.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-04-24 13:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-20 15:32 [PATCH 1/2] input: adp5588-keys - get value from data out when dir out Jean-François Dagenais
2012-04-20 15:32 ` [PATCH 2/2] gpio: adp5588 " Jean-François Dagenais
2012-04-21 6:47 ` [PATCH 1/2] input: adp5588-keys " Dmitry Torokhov
2012-04-24 13:33 ` Jean-Francois Dagenais
-- strict thread matches above, loose matches on Subject: below --
2012-01-09 15:05 Jean-François Dagenais
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox