All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: Propagate errors from chip->get()
@ 2015-08-28 16:44 ` Bjorn Andersson
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2015-08-28 16:44 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot; +Cc: linux-gpio, linux-kernel

It's possible to have gpio chips hanging off unreliable remote buses
where the get() operation will fail to acquire a readout of the current
gpio state. Propagate these errors to the consumer so that they can
act on, retry or ignore these failing reads, instead of treating them as
the line being held high.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/gpio/gpiolib.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3b5e516298e0..dc17dbf8c234 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1173,15 +1173,16 @@ EXPORT_SYMBOL_GPL(gpiod_is_active_low);
  * that the GPIO was actually requested.
  */
 
-static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
+static int _gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	struct gpio_chip	*chip;
-	bool value;
 	int offset;
+	int value;
 
 	chip = desc->chip;
 	offset = gpio_chip_hwgpio(desc);
-	value = chip->get ? chip->get(chip, offset) : false;
+	value = chip->get ? chip->get(chip, offset) : -EIO;
+	value = value < 0 ? value : !!value;
 	trace_gpio_value(desc_to_gpio(desc), 1, value);
 	return value;
 }
@@ -1191,7 +1192,7 @@ static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's raw value, i.e. the value of the physical line disregarding
- * its ACTIVE_LOW status.
+ * its ACTIVE_LOW status, or negative errno on failure.
  *
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
@@ -1211,7 +1212,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into
- * account.
+ * account, or negative errno on failure.
  *
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
@@ -1225,6 +1226,9 @@ int gpiod_get_value(const struct gpio_desc *desc)
 	WARN_ON(desc->chip->can_sleep);
 
 	value = _gpiod_get_raw_value(desc);
+	if (value < 0)
+		return value;
+
 	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 		value = !value;
 
@@ -1547,7 +1551,7 @@ EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's raw value, i.e. the value of the physical line disregarding
- * its ACTIVE_LOW status.
+ * its ACTIVE_LOW status, or negative errno on failure.
  *
  * This function is to be called from contexts that can sleep.
  */
@@ -1565,7 +1569,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value_cansleep);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into
- * account.
+ * account, or negative errno on failure.
  *
  * This function is to be called from contexts that can sleep.
  */
@@ -1578,6 +1582,9 @@ int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 		return 0;
 
 	value = _gpiod_get_raw_value(desc);
+	if (value < 0)
+		return value;
+
 	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 		value = !value;
 
-- 
1.8.2.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] gpio: Propagate errors from chip->get()
@ 2015-08-28 16:44 ` Bjorn Andersson
  0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2015-08-28 16:44 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot; +Cc: linux-gpio, linux-kernel

It's possible to have gpio chips hanging off unreliable remote buses
where the get() operation will fail to acquire a readout of the current
gpio state. Propagate these errors to the consumer so that they can
act on, retry or ignore these failing reads, instead of treating them as
the line being held high.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/gpio/gpiolib.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3b5e516298e0..dc17dbf8c234 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1173,15 +1173,16 @@ EXPORT_SYMBOL_GPL(gpiod_is_active_low);
  * that the GPIO was actually requested.
  */
 
-static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
+static int _gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	struct gpio_chip	*chip;
-	bool value;
 	int offset;
+	int value;
 
 	chip = desc->chip;
 	offset = gpio_chip_hwgpio(desc);
-	value = chip->get ? chip->get(chip, offset) : false;
+	value = chip->get ? chip->get(chip, offset) : -EIO;
+	value = value < 0 ? value : !!value;
 	trace_gpio_value(desc_to_gpio(desc), 1, value);
 	return value;
 }
@@ -1191,7 +1192,7 @@ static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's raw value, i.e. the value of the physical line disregarding
- * its ACTIVE_LOW status.
+ * its ACTIVE_LOW status, or negative errno on failure.
  *
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
@@ -1211,7 +1212,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into
- * account.
+ * account, or negative errno on failure.
  *
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
@@ -1225,6 +1226,9 @@ int gpiod_get_value(const struct gpio_desc *desc)
 	WARN_ON(desc->chip->can_sleep);
 
 	value = _gpiod_get_raw_value(desc);
+	if (value < 0)
+		return value;
+
 	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 		value = !value;
 
@@ -1547,7 +1551,7 @@ EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's raw value, i.e. the value of the physical line disregarding
- * its ACTIVE_LOW status.
+ * its ACTIVE_LOW status, or negative errno on failure.
  *
  * This function is to be called from contexts that can sleep.
  */
@@ -1565,7 +1569,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value_cansleep);
  * @desc: gpio whose value will be returned
  *
  * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into
- * account.
+ * account, or negative errno on failure.
  *
  * This function is to be called from contexts that can sleep.
  */
@@ -1578,6 +1582,9 @@ int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 		return 0;
 
 	value = _gpiod_get_raw_value(desc);
+	if (value < 0)
+		return value;
+
 	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 		value = !value;
 
-- 
1.8.2.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Propagate errors from chip->get()
  2015-08-28 16:44 ` Bjorn Andersson
  (?)
@ 2015-09-01  5:16 ` Bjorn Andersson
  2015-09-02 12:37   ` Alexandre Courbot
  2015-09-08 13:04   ` Linus Walleij
  -1 siblings, 2 replies; 6+ messages in thread
From: Bjorn Andersson @ 2015-09-01  5:16 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot; +Cc: linux-gpio, linux-kernel

On Fri 28 Aug 09:44 PDT 2015, Bjorn Andersson wrote:

> It's possible to have gpio chips hanging off unreliable remote buses
> where the get() operation will fail to acquire a readout of the current
> gpio state. Propagate these errors to the consumer so that they can
> act on, retry or ignore these failing reads, instead of treating them as
> the line being held high.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>  drivers/gpio/gpiolib.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 3b5e516298e0..dc17dbf8c234 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1173,15 +1173,16 @@ EXPORT_SYMBOL_GPL(gpiod_is_active_low);
>   * that the GPIO was actually requested.
>   */
>  
> -static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
> +static int _gpiod_get_raw_value(const struct gpio_desc *desc)
>  {
>  	struct gpio_chip	*chip;
> -	bool value;
>  	int offset;
> +	int value;
>  
>  	chip = desc->chip;
>  	offset = gpio_chip_hwgpio(desc);
> -	value = chip->get ? chip->get(chip, offset) : false;
> +	value = chip->get ? chip->get(chip, offset) : -EIO;

Linus, Alexandre, please feel free to apply this with -ENOTSUPP in
accordance to Alexandre's comment in [1], if you prefer that. I picked
-EIO as that's what's used in most other places when the get() op is
missing.

Maybe we should follow up with separate patch to make that consistent?

[1] https://lkml.org/lkml/2015/8/31/8

> +	value = value < 0 ? value : !!value;
>  	trace_gpio_value(desc_to_gpio(desc), 1, value);
>  	return value;
>  }

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Propagate errors from chip->get()
  2015-09-01  5:16 ` Bjorn Andersson
@ 2015-09-02 12:37   ` Alexandre Courbot
  2015-09-08 13:04   ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-09-02 12:37 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Linus Walleij, linux-gpio, linux-kernel

On Tue, Sep 1, 2015 at 2:16 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> On Fri 28 Aug 09:44 PDT 2015, Bjorn Andersson wrote:
>
>> It's possible to have gpio chips hanging off unreliable remote buses
>> where the get() operation will fail to acquire a readout of the current
>> gpio state. Propagate these errors to the consumer so that they can
>> act on, retry or ignore these failing reads, instead of treating them as
>> the line being held high.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>> ---
>>  drivers/gpio/gpiolib.c | 21 ++++++++++++++-------
>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 3b5e516298e0..dc17dbf8c234 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -1173,15 +1173,16 @@ EXPORT_SYMBOL_GPL(gpiod_is_active_low);
>>   * that the GPIO was actually requested.
>>   */
>>
>> -static bool _gpiod_get_raw_value(const struct gpio_desc *desc)
>> +static int _gpiod_get_raw_value(const struct gpio_desc *desc)
>>  {
>>       struct gpio_chip        *chip;
>> -     bool value;
>>       int offset;
>> +     int value;
>>
>>       chip = desc->chip;
>>       offset = gpio_chip_hwgpio(desc);
>> -     value = chip->get ? chip->get(chip, offset) : false;
>> +     value = chip->get ? chip->get(chip, offset) : -EIO;
>
> Linus, Alexandre, please feel free to apply this with -ENOTSUPP in
> accordance to Alexandre's comment in [1], if you prefer that. I picked
> -EIO as that's what's used in most other places when the get() op is
> missing.
>
> Maybe we should follow up with separate patch to make that consistent?

I will let Linus decide which value is the more appropriate, the patch
looks solid to me!

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Propagate errors from chip->get()
  2015-08-28 16:44 ` Bjorn Andersson
  (?)
  (?)
@ 2015-09-08 13:02 ` Linus Walleij
  -1 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2015-09-08 13:02 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Fri, Aug 28, 2015 at 6:44 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:

> It's possible to have gpio chips hanging off unreliable remote buses
> where the get() operation will fail to acquire a readout of the current
> gpio state. Propagate these errors to the consumer so that they can
> act on, retry or ignore these failing reads, instead of treating them as
> the line being held high.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>

This patch applied for fixes with Alex' Review-tag.
Thanks for fixing this!

Should I even tag it for stable?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Propagate errors from chip->get()
  2015-09-01  5:16 ` Bjorn Andersson
  2015-09-02 12:37   ` Alexandre Courbot
@ 2015-09-08 13:04   ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2015-09-08 13:04 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Tue, Sep 1, 2015 at 7:16 AM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:

> Linus, Alexandre, please feel free to apply this with -ENOTSUPP in
> accordance to Alexandre's comment in [1], if you prefer that. I picked
> -EIO as that's what's used in most other places when the get() op is
> missing.

I applied as-is for fixes.

> Maybe we should follow up with separate patch to make that consistent?

A patch doing this would be appropriate for devel as it makes
deeper semantic changes.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-09-08 13:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-28 16:44 [PATCH] gpio: Propagate errors from chip->get() Bjorn Andersson
2015-08-28 16:44 ` Bjorn Andersson
2015-09-01  5:16 ` Bjorn Andersson
2015-09-02 12:37   ` Alexandre Courbot
2015-09-08 13:04   ` Linus Walleij
2015-09-08 13:02 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.