linux-kernel - RE: gpio-pxa: getting GPIOs by devicetree phandle broken

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B6D6D87A8FB62428BE0A3E38461A1AB18D6B200@irsmsx105.ger.corp.intel.com>
Date:	Mon, 9 Feb 2015 09:42:02 +0000
From:	"Holmberg, Hans" <hans.holmberg@...el.com>
To:	Alexandre Courbot <gnurou@...il.com>,
	Tyler Hall <tylerwhall@...il.com>
CC:	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Daniel Mack <zonque@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	"Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
	Howard Cochran <cochran@...mark.com>,
	Robert Jarzmik <robert.jarzmik@...e.fr>
Subject: RE: gpio-pxa: getting GPIOs by devicetree phandle broken


> > 1. Require child nodes in DT for each bank
> 
> This would break DT compatibility.
> 
> > 2. Refactor gpio-pxa to only register one gpiochip
> 
> Sounds better, especially since this would reflect the hardware more
> accurately. One DT node should translate into one GPIO chip. The problem is
> that I'm afraid several other drivers are doing the same thing and thus are
> now similarly broken.

This sounds reasonable to me.

> The following is also likely to work as a workaround, but I would not go as
> far as saying this should be taken as a fix. Hans, since you wrote 7b8792b,
> could we have your input on this?

> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index
> 08261f2b3a82..a09095531b00 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -45,14 +45,16 @@ static int of_gpiochip_find_and_xlate(struct
> gpio_chip *gc, void *data)
>                 return false;
> 
>         ret = gc->of_xlate(gc, &gg_data->gpiospec, gg_data->flags);
> -       if (ret < 0) {
> +       if (ret == -EPROBE_DEFER) {
>                 /* We've found the gpio chip, but the translation failed.
>                  * Return true to stop looking and return the translation
>                  * error via out_gpio
>                  */
>                 gg_data->out_gpio = ERR_PTR(ret);
>                 return true;
> -        }
> +       } else if (ret < 0) {
> +               return false;
> +       }
> 
>         gg_data->out_gpio = gpiochip_get_desc(gc, ret);
>         return true;

The problem my patch solved was that -EPROBE_DEFER was returned regardless of lookup-error in of_get_named_gpiod_flags, not that -EPROBE_DEFER error was not passed on.

The issue with multiple gpiochips per of-node could be worked around as followed I believe, comments?

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 08261f2..43984ab 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -47,11 +47,12 @@ static int of_gpiochip_find_and_xlate(struct gpio_chip *gc, void *data)
        ret = gc->of_xlate(gc, &gg_data->gpiospec, gg_data->flags);
        if (ret < 0) {
                /* We've found the gpio chip, but the translation failed.
-                * Return true to stop looking and return the translation
-                * error via out_gpio
+                * Store translation error in out_gpio.
+                * Return false to keep looking, as more than one GPIO chip
+                * could be registered per of-node.
                 */
                gg_data->out_gpio = ERR_PTR(ret);
-               return true;
+               return false;
         }
 
        gg_data->out_gpio = gpiochip_get_desc(gc, ret);
--

Best regards,
Hans