From: Alex Courbot <acourbot@nvidia.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
Frank Rowand <frank.rowand@sonymobile.com>,
Tim Bird <tim.bird@sonymobile.com>
Subject: Re: [PATCH] gpio: improve error path in gpiolib
Date: Fri, 30 Aug 2013 18:17:16 +0900 [thread overview]
Message-ID: <5220631C.8060107@nvidia.com> (raw)
In-Reply-To: <1377848795-28070-1-git-send-email-linus.walleij@linaro.org>
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.
-----------------------------------------------------------------------------------
next prev parent reply other threads:[~2013-08-30 9:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-30 7:46 [PATCH] gpio: improve error path in gpiolib Linus Walleij
2013-08-30 9:17 ` Alex Courbot [this message]
[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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5220631C.8060107@nvidia.com \
--to=acourbot@nvidia.com \
--cc=frank.rowand@sonymobile.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=tim.bird@sonymobile.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).