* [PATCH] gpio: improve error path in gpiolib
@ 2013-08-30 7:46 Linus Walleij
2013-08-30 9:17 ` Alex Courbot
[not found] ` <5220E6F6.5080006@gmail.com>
0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2013-08-30 7:46 UTC (permalink / raw)
To: linux-gpio; +Cc: Alexandre Courbot, Linus Walleij, Frank Rowand, Tim Bird
At several places the gpiolib will proceed to handle a GPIO
descriptor even if it's ->chip member is NULL and no gpiochip
is associated.
Fix this by checking that both the descriptor cookie *and*
the chip pointer are valid.
Also bail out earlier with more specific diagnostic messages
on missing operations for setting as input/output or debounce.
Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Cc: Frank Rowand <frank.rowand@sonymobile.com>
Cc: Tim Bird <tim.bird@sonymobile.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d6413b2..f041f9e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1398,7 +1398,7 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
int status = -EPROBE_DEFER;
unsigned long flags;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1406,8 +1406,6 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
spin_lock_irqsave(&gpio_lock, flags);
chip = desc->chip;
- if (chip == NULL)
- goto done;
if (!try_module_get(chip->owner))
goto done;
@@ -1630,16 +1628,20 @@ static int gpiod_direction_input(struct gpio_desc *desc)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
+ chip = desc->chip;
+ if (!chip->get || !chip->direction_input) {
+ pr_warn("%s: missing get() or direction_input() operations\n",
+ __func__);
+ return -EIO;
+ }
+
spin_lock_irqsave(&gpio_lock, flags);
- chip = desc->chip;
- if (!chip || !chip->get || !chip->direction_input)
- goto fail;
status = gpio_ensure_requested(desc);
if (status < 0)
goto fail;
@@ -1691,7 +1693,7 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1704,11 +1706,15 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags))
return gpiod_direction_input(desc);
+ chip = desc->chip;
+ if (!chip->set || !chip->direction_output) {
+ pr_warn("%s: missing set() or direction_output() operations\n",
+ __func__);
+ return -EIO;
+ }
+
spin_lock_irqsave(&gpio_lock, flags);
- chip = desc->chip;
- if (!chip || !chip->set || !chip->direction_output)
- goto fail;
status = gpio_ensure_requested(desc);
if (status < 0)
goto fail;
@@ -1765,7 +1771,7 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1773,8 +1779,10 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
spin_lock_irqsave(&gpio_lock, flags);
chip = desc->chip;
- if (!chip || !chip->set || !chip->set_debounce)
- goto fail;
+ if (!chip->set || !chip->set_debounce) {
+ pr_warn("%s: missing set() or set_debounce() operations\n",
+ __func__);
+ }
status = gpio_ensure_requested(desc);
if (status < 0)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] gpio: improve error path in gpiolib
2013-08-30 7:46 [PATCH] gpio: improve error path in gpiolib Linus Walleij
@ 2013-08-30 9:17 ` Alex Courbot
[not found] ` <5220E6F6.5080006@gmail.com>
1 sibling, 0 replies; 4+ messages in thread
From: Alex Courbot @ 2013-08-30 9:17 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio@vger.kernel.org, Frank Rowand, Tim Bird
On 08/30/2013 04:46 PM, Linus Walleij wrote:
> At several places the gpiolib will proceed to handle a GPIO
> descriptor even if it's ->chip member is NULL and no gpiochip
> is associated.
>
> Fix this by checking that both the descriptor cookie *and*
> the chip pointer are valid.
>
> Also bail out earlier with more specific diagnostic messages
> on missing operations for setting as input/output or debounce.
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Frank Rowand <frank.rowand@sonymobile.com>
> Cc: Tim Bird <tim.bird@sonymobile.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpiolib.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d6413b2..f041f9e 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1398,7 +1398,7 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
> int status = -EPROBE_DEFER;
> unsigned long flags;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1406,8 +1406,6 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
> spin_lock_irqsave(&gpio_lock, flags);
>
> chip = desc->chip;
> - if (chip == NULL)
> - goto done;
>
> if (!try_module_get(chip->owner))
> goto done;
> @@ -1630,16 +1628,20 @@ static int gpiod_direction_input(struct gpio_desc *desc)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
>
> + chip = desc->chip;
> + if (!chip->get || !chip->direction_input) {
> + pr_warn("%s: missing get() or direction_input() operations\n",
> + __func__);
> + return -EIO;
> + }
> +
> spin_lock_irqsave(&gpio_lock, flags);
>
> - chip = desc->chip;
> - if (!chip || !chip->get || !chip->direction_input)
> - goto fail;
> status = gpio_ensure_requested(desc);
> if (status < 0)
> goto fail;
> @@ -1691,7 +1693,7 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1704,11 +1706,15 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
> if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags))
> return gpiod_direction_input(desc);
>
> + chip = desc->chip;
> + if (!chip->set || !chip->direction_output) {
> + pr_warn("%s: missing set() or direction_output() operations\n",
> + __func__);
> + return -EIO;
> + }
> +
> spin_lock_irqsave(&gpio_lock, flags);
>
> - chip = desc->chip;
> - if (!chip || !chip->set || !chip->direction_output)
> - goto fail;
> status = gpio_ensure_requested(desc);
> if (status < 0)
> goto fail;
> @@ -1765,7 +1771,7 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1773,8 +1779,10 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
> spin_lock_irqsave(&gpio_lock, flags);
>
> chip = desc->chip;
> - if (!chip || !chip->set || !chip->set_debounce)
> - goto fail;
> + if (!chip->set || !chip->set_debounce) {
> + pr_warn("%s: missing set() or set_debounce() operations\n",
> + __func__);
> + }
>
> status = gpio_ensure_requested(desc);
> if (status < 0)
>
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 4+ messages in thread[parent not found: <5220E6F6.5080006@gmail.com>]
* Re: [PATCH] gpio: improve error path in gpiolib
[not found] ` <5220E6F6.5080006@gmail.com>
@ 2013-08-30 20:28 ` Frank Rowand
0 siblings, 0 replies; 4+ messages in thread
From: Frank Rowand @ 2013-08-30 20:28 UTC (permalink / raw)
To: linus.walleij; +Cc: Tim Bird, linux-gpio, acourbot
Linus,
The patch looks good, except for one issue (noted inline).
Reviewed-by: Frank Rowand <frank.rowand@sonymobile.com>
-Frank
> -------- Original Message --------
> Subject: [PATCH] gpio: improve error path in gpiolib
> Date: Fri, 30 Aug 2013 09:46:35 +0200
> From: Linus Walleij <linus.walleij@linaro.org>
> To: linux-gpio@vger.kernel.org
> CC: Alexandre Courbot <acourbot@nvidia.com>, Linus Walleij
> <linus.walleij@linaro.org>, Frank Rowand <frank.rowand@sonymobile.com>,
> Tim Bird <tim.bird@sonymobile.com>
>
> At several places the gpiolib will proceed to handle a GPIO
> descriptor even if it's ->chip member is NULL and no gpiochip
> is associated.
>
> Fix this by checking that both the descriptor cookie *and*
> the chip pointer are valid.
>
> Also bail out earlier with more specific diagnostic messages
> on missing operations for setting as input/output or debounce.
>
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Frank Rowand <frank.rowand@sonymobile.com>
> Cc: Tim Bird <tim.bird@sonymobile.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpiolib.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d6413b2..f041f9e 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1398,7 +1398,7 @@ static int gpiod_request(struct gpio_desc *desc,
> const char *label)
> int status = -EPROBE_DEFER;
> unsigned long flags;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1406,8 +1406,6 @@ static int gpiod_request(struct gpio_desc *desc,
> const char *label)
> spin_lock_irqsave(&gpio_lock, flags);
>
> chip = desc->chip;
> - if (chip == NULL)
> - goto done;
>
> if (!try_module_get(chip->owner))
> goto done;
> @@ -1630,16 +1628,20 @@ static int gpiod_direction_input(struct
> gpio_desc *desc)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
>
> + chip = desc->chip;
> + if (!chip->get || !chip->direction_input) {
> + pr_warn("%s: missing get() or direction_input() operations\n",
> + __func__);
> + return -EIO;
> + }
> +
> spin_lock_irqsave(&gpio_lock, flags);
>
> - chip = desc->chip;
> - if (!chip || !chip->get || !chip->direction_input)
> - goto fail;
> status = gpio_ensure_requested(desc);
> if (status < 0)
> goto fail;
> @@ -1691,7 +1693,7 @@ static int gpiod_direction_output(struct gpio_desc
> *desc, int value)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1704,11 +1706,15 @@ static int gpiod_direction_output(struct
> gpio_desc *desc, int value)
> if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags))
> return gpiod_direction_input(desc);
>
> + chip = desc->chip;
> + if (!chip->set || !chip->direction_output) {
> + pr_warn("%s: missing set() or direction_output() operations\n",
> + __func__);
> + return -EIO;
> + }
> +
> spin_lock_irqsave(&gpio_lock, flags);
>
> - chip = desc->chip;
> - if (!chip || !chip->set || !chip->direction_output)
> - goto fail;
> status = gpio_ensure_requested(desc);
> if (status < 0)
> goto fail;
> @@ -1765,7 +1771,7 @@ static int gpiod_set_debounce(struct gpio_desc
> *desc, unsigned debounce)
> int status = -EINVAL;
> int offset;
>
> - if (!desc) {
> + if (!desc || !desc->chip) {
> pr_warn("%s: invalid GPIO\n", __func__);
> return -EINVAL;
> }
> @@ -1773,8 +1779,10 @@ static int gpiod_set_debounce(struct gpio_desc
> *desc, unsigned debounce)
> spin_lock_irqsave(&gpio_lock, flags);
>
> chip = desc->chip;
> - if (!chip || !chip->set || !chip->set_debounce)
> - goto fail;
> + if (!chip->set || !chip->set_debounce) {
> + pr_warn("%s: missing set() or set_debounce() operations\n",
> + __func__);
There should be a "return -EIO" here also. Otherwise the
NULL chip->set_debounce() gets called a few lines below.
> + }
>
> status = gpio_ensure_requested(desc);
> if (status < 0)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] gpio: improve error path in gpiolib
@ 2013-09-02 8:39 Linus Walleij
0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2013-09-02 8:39 UTC (permalink / raw)
To: linux-gpio; +Cc: Alexandre Courbot, Linus Walleij, Tim Bird
At several places the gpiolib will proceed to handle a GPIO
descriptor even if it's ->chip member is NULL and no gpiochip
is associated.
Fix this by checking that both the descriptor cookie *and*
the chip pointer are valid.
Also bail out earlier with more specific diagnostic messages
on missing operations for setting as input/output or debounce.
ChangeLog v1->v2:
- Also return -EIO on gpiod_set_debounce() with missing
operations in the vtable
- Fix indentations.
Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Frank Rowand <frank.rowand@sonymobile.com>
Cc: Tim Bird <tim.bird@sonymobile.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d6413b2..a7587db 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1398,7 +1398,7 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
int status = -EPROBE_DEFER;
unsigned long flags;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1406,8 +1406,6 @@ static int gpiod_request(struct gpio_desc *desc, const char *label)
spin_lock_irqsave(&gpio_lock, flags);
chip = desc->chip;
- if (chip == NULL)
- goto done;
if (!try_module_get(chip->owner))
goto done;
@@ -1630,16 +1628,20 @@ static int gpiod_direction_input(struct gpio_desc *desc)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
+ chip = desc->chip;
+ if (!chip->get || !chip->direction_input) {
+ pr_warn("%s: missing get() or direction_input() operations\n",
+ __func__);
+ return -EIO;
+ }
+
spin_lock_irqsave(&gpio_lock, flags);
- chip = desc->chip;
- if (!chip || !chip->get || !chip->direction_input)
- goto fail;
status = gpio_ensure_requested(desc);
if (status < 0)
goto fail;
@@ -1691,7 +1693,7 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1704,11 +1706,15 @@ static int gpiod_direction_output(struct gpio_desc *desc, int value)
if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags))
return gpiod_direction_input(desc);
+ chip = desc->chip;
+ if (!chip->set || !chip->direction_output) {
+ pr_warn("%s: missing set() or direction_output() operations\n",
+ __func__);
+ return -EIO;
+ }
+
spin_lock_irqsave(&gpio_lock, flags);
- chip = desc->chip;
- if (!chip || !chip->set || !chip->direction_output)
- goto fail;
status = gpio_ensure_requested(desc);
if (status < 0)
goto fail;
@@ -1765,7 +1771,7 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
int status = -EINVAL;
int offset;
- if (!desc) {
+ if (!desc || !desc->chip) {
pr_warn("%s: invalid GPIO\n", __func__);
return -EINVAL;
}
@@ -1773,8 +1779,11 @@ static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
spin_lock_irqsave(&gpio_lock, flags);
chip = desc->chip;
- if (!chip || !chip->set || !chip->set_debounce)
- goto fail;
+ if (!chip->set || !chip->set_debounce) {
+ pr_warn("%s: missing set() or set_debounce() operations\n",
+ __func__);
+ return -EIO;
+ }
status = gpio_ensure_requested(desc);
if (status < 0)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-02 8:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-30 7:46 [PATCH] gpio: improve error path in gpiolib Linus Walleij
2013-08-30 9:17 ` Alex Courbot
[not found] ` <5220E6F6.5080006@gmail.com>
2013-08-30 20:28 ` Frank Rowand
-- strict thread matches above, loose matches on Subject: below --
2013-09-02 8:39 Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).