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

[Holmberg, Hans]

> > 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
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå