linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: make sure gpiod_to_irq() returns negative on NULL desc
@ 2016-06-15 21:03 Linus Walleij
  2016-06-17 16:09 ` Hans de Goede
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Walleij @ 2016-06-15 21:03 UTC (permalink / raw)
  To: linux-gpio, Alexandre Courbot, Hans de Goede; +Cc: Linus Walleij, Maxime Ripard

commit 54d77198fdfbc4f0fe11b4252c1d9c97d51a3264
("gpio: bail out silently on NULL descriptors")
doesn't work for gpiod_to_irq(): drivers assume that NULL
descriptors will give negative IRQ numbers in return.

It has been pointed out that returning 0 is NO_IRQ and that
drivers should be amended to treat this as an error, but that
is for the longer term: now let us repair the semantics.

Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Reported-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 58d822d7e8da..f39bf05993e7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2056,7 +2056,14 @@ int gpiod_to_irq(const struct gpio_desc *desc)
 	struct gpio_chip *chip;
 	int offset;
 
-	VALIDATE_DESC(desc);
+	/*
+	 * Cannot VALIDATE_DESC() here as gpiod_to_irq() consumer semantics
+	 * requires this function to not return zero on an invalid descriptor
+	 * but rather a negative error number.
+	 */
+	if (!desc || !desc->gdev || !desc->gdev->chip)
+		return -EINVAL;
+
 	chip = desc->gdev->chip;
 	offset = gpio_chip_hwgpio(desc);
 	if (chip->to_irq) {
-- 
2.4.11


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

* Re: [PATCH] gpio: make sure gpiod_to_irq() returns negative on NULL desc
  2016-06-15 21:03 [PATCH] gpio: make sure gpiod_to_irq() returns negative on NULL desc Linus Walleij
@ 2016-06-17 16:09 ` Hans de Goede
  2016-06-17 16:13   ` Linus Walleij
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2016-06-17 16:09 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio, Alexandre Courbot; +Cc: Maxime Ripard

Hi,

On 15-06-16 23:03, Linus Walleij wrote:
> commit 54d77198fdfbc4f0fe11b4252c1d9c97d51a3264
> ("gpio: bail out silently on NULL descriptors")
> doesn't work for gpiod_to_irq(): drivers assume that NULL
> descriptors will give negative IRQ numbers in return.
>
> It has been pointed out that returning 0 is NO_IRQ and that
> drivers should be amended to treat this as an error, but that
> is for the longer term: now let us repair the semantics.
>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I can confirm that this fixes the problem for me:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Thanks & Regards,

Hans

> ---
>  drivers/gpio/gpiolib.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 58d822d7e8da..f39bf05993e7 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2056,7 +2056,14 @@ int gpiod_to_irq(const struct gpio_desc *desc)
>  	struct gpio_chip *chip;
>  	int offset;
>
> -	VALIDATE_DESC(desc);
> +	/*
> +	 * Cannot VALIDATE_DESC() here as gpiod_to_irq() consumer semantics
> +	 * requires this function to not return zero on an invalid descriptor
> +	 * but rather a negative error number.
> +	 */
> +	if (!desc || !desc->gdev || !desc->gdev->chip)
> +		return -EINVAL;
> +
>  	chip = desc->gdev->chip;
>  	offset = gpio_chip_hwgpio(desc);
>  	if (chip->to_irq) {
>

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

* Re: [PATCH] gpio: make sure gpiod_to_irq() returns negative on NULL desc
  2016-06-17 16:09 ` Hans de Goede
@ 2016-06-17 16:13   ` Linus Walleij
  0 siblings, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2016-06-17 16:13 UTC (permalink / raw)
  To: Hans de Goede
  Cc: linux-gpio@vger.kernel.org, Alexandre Courbot, Maxime Ripard

On Fri, Jun 17, 2016 at 6:09 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On 15-06-16 23:03, Linus Walleij wrote:
>>
>> commit 54d77198fdfbc4f0fe11b4252c1d9c97d51a3264
>> ("gpio: bail out silently on NULL descriptors")
>> doesn't work for gpiod_to_irq(): drivers assume that NULL
>> descriptors will give negative IRQ numbers in return.
>>
>> It has been pointed out that returning 0 is NO_IRQ and that
>> drivers should be amended to treat this as an error, but that
>> is for the longer term: now let us repair the semantics.
>>
>> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
>> Reported-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
>
> I can confirm that this fixes the problem for me:
>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Thanks, applied for fixes and pushed for linux-next.

Yours,
Linus Walleij

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

end of thread, other threads:[~2016-06-17 16:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-15 21:03 [PATCH] gpio: make sure gpiod_to_irq() returns negative on NULL desc Linus Walleij
2016-06-17 16:09 ` Hans de Goede
2016-06-17 16:13   ` 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).