[PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[Geert Uytterhoeven]

If a GPIO driver uses gpiochip_add_pin_range() (which is usually the
case for GPIO/PFC combos), the GPIO hogging mechanism configured from DT
doesn't work:

    requesting hog GPIO lcd0 (chip r8a7740_pfc, offset 176) failed

The actual error code is -517 == -EPROBE_DEFER.

The problem is that PFC+GPIO registration is handled in multiple steps:
  1. pinctrl_register(),
  2. gpiochip_add(),
  3. gpiochip_add_pin_range().

Configuration of the hogs is handled in gpiochip_add():

    gpiochip_add
        of_gpiochip_add
            of_gpiochip_scan_hogs
                gpiod_hog
                    gpiochip_request_own_desc
                        __gpiod_request
                            chip->request
                                pinctrl_request_gpio
                                    pinctrl_get_device_gpio_range

However, at this point the GPIO controller hasn't been added to
pinctrldev_list yet, so the range can't be found, and the operation fails
with -EPROBE_DEFER.

  - Exchanging the order of the calls to gpiochip_add() and
    gpiochip_add_pin_range() is not an option, as the latter depends on
    initialization done by the former.
  - Just moving the call of of_gpiochip_scan_hogs() from gpiochip_add()
    to gpiochip_add_pin_range() is also not an option, as the latter is
    optional, and thus not used by all drivers.

Hence if of_gpiochip_scan_hogs() fails with -EPROBE_DEFER, call it
again every time the pin range is changed, until it succeeded.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Questions:
  - Is there a better solution to handle this?

  - Should the pin ranges be configured by passing an array of data to
    gpiochip_add() instead of having calls to gpiochip_add_pin_range()?
    That would require changing all drivers.

  - What happens if you have multiple hogs in multiple ranges?
    The first hog(s) may be configured multiple times.  Is that a problem?

  - In one of the threads that discussed the GPIO hogging mechanism, Maxime
    Ripard said: "Our pinctrl driver is also our GPIO driver, so they both
    share the same node."
    Maxime: Did you try GPIO hogging? Did it work?
    If yes, which driver are you using? What's different compared to sh-pfc?
    If no, did you get it to work?

Thanks!
---
 drivers/gpio/gpiolib-of.c   | 21 +++++++++++++++++----
 drivers/gpio/gpiolib.c      |  1 +
 include/linux/gpio/driver.h |  1 +
 include/linux/of_gpio.h     |  2 ++
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 9a0ec48a47375d18..90dd02b19f75c27c 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -205,13 +205,14 @@ static struct gpio_desc *of_get_gpio_hog(struct device_node *np,
  * This is only used by of_gpiochip_add to request/set GPIO initial
  * configuration.
  */
-static void of_gpiochip_scan_hogs(struct gpio_chip *chip)
+static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 {
 	struct gpio_desc *desc = NULL;
 	struct device_node *np;
 	const char *name;
 	enum gpio_lookup_flags lflags;
 	enum gpiod_flags dflags;
+	int error;
 
 	for_each_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
@@ -221,9 +222,12 @@ static void of_gpiochip_scan_hogs(struct gpio_chip *chip)
 		if (IS_ERR(desc))
 			continue;
 
-		if (gpiod_hog(desc, name, lflags, dflags))
-			continue;
+		error = gpiod_hog(desc, name, lflags, dflags);
+		if (error == -EPROBE_DEFER)
+			return error;
 	}
+
+	return 0;
 }
 
 /**
@@ -416,8 +420,17 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 	}
 }
 
+void of_gpiochip_pin_range_changed(struct gpio_chip *chip)
+{
+	if (chip->hog_error) {
+		/* Retry */
+		chip->hog_error = of_gpiochip_scan_hogs(chip);
+	}
+}
+
 #else
 static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {}
+void of_gpiochip_pin_range_changed(struct gpio_chip *chip) {}
 #endif
 
 void of_gpiochip_add(struct gpio_chip *chip)
@@ -436,7 +449,7 @@ void of_gpiochip_add(struct gpio_chip *chip)
 	of_gpiochip_add_pin_range(chip);
 	of_node_get(chip->of_node);
 
-	of_gpiochip_scan_hogs(chip);
+	chip->hog_error = of_gpiochip_scan_hogs(chip);
 }
 
 void of_gpiochip_remove(struct gpio_chip *chip)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 957ede5664cfe168..b0fe7a459d8835bb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -759,6 +759,7 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
 
 	list_add_tail(&pin_range->node, &chip->pin_ranges);
 
+	of_gpiochip_pin_range_changed(chip);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add_pin_range);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index c8393cd4d44f2d87..9396b68dced2c5b1 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -146,6 +146,7 @@ struct gpio_chip {
 	 * corresponding pins for gpio usage.
 	 */
 	struct list_head pin_ranges;
+	int hog_error;
 #endif
 };
 
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 69dbe312b11b23f6..34421f17f4712d0b 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -54,6 +54,7 @@ extern int of_mm_gpiochip_add(struct device_node *np,
 			      struct of_mm_gpio_chip *mm_gc);
 extern void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc);
 
+extern void of_gpiochip_pin_range_changed(struct gpio_chip *chip);
 extern void of_gpiochip_add(struct gpio_chip *gc);
 extern void of_gpiochip_remove(struct gpio_chip *gc);
 extern int of_gpio_simple_xlate(struct gpio_chip *gc,
@@ -76,6 +77,7 @@ static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
 	return -ENOSYS;
 }
 
+static inline void of_gpiochip_pin_range_changed(struct gpio_chip *chip) { }
 static inline void of_gpiochip_add(struct gpio_chip *gc) { }
 static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
 
-- 
1.9.1

Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

Hi Geert,

On Tue, Jun 16, 2015 at 02:36:48PM +0200, Geert Uytterhoeven wrote:
> If a GPIO driver uses gpiochip_add_pin_range() (which is usually the
> case for GPIO/PFC combos), the GPIO hogging mechanism configured from DT
> doesn't work:
> 
>     requesting hog GPIO lcd0 (chip r8a7740_pfc, offset 176) failed
> 
> The actual error code is -517 == -EPROBE_DEFER.
> 
> The problem is that PFC+GPIO registration is handled in multiple steps:
>   1. pinctrl_register(),
>   2. gpiochip_add(),
>   3. gpiochip_add_pin_range().
> 
> Configuration of the hogs is handled in gpiochip_add():
> 
>     gpiochip_add
>         of_gpiochip_add
>             of_gpiochip_scan_hogs
>                 gpiod_hog
>                     gpiochip_request_own_desc
>                         __gpiod_request
>                             chip->request
>                                 pinctrl_request_gpio
>                                     pinctrl_get_device_gpio_range
> 
> However, at this point the GPIO controller hasn't been added to
> pinctrldev_list yet, so the range can't be found, and the operation fails
> with -EPROBE_DEFER.
> 
>   - Exchanging the order of the calls to gpiochip_add() and
>     gpiochip_add_pin_range() is not an option, as the latter depends on
>     initialization done by the former.
>   - Just moving the call of of_gpiochip_scan_hogs() from gpiochip_add()
>     to gpiochip_add_pin_range() is also not an option, as the latter is
>     optional, and thus not used by all drivers.
> 
> Hence if of_gpiochip_scan_hogs() fails with -EPROBE_DEFER, call it
> again every time the pin range is changed, until it succeeded.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Questions:
>   - Is there a better solution to handle this?
> 
>   - Should the pin ranges be configured by passing an array of data to
>     gpiochip_add() instead of having calls to gpiochip_add_pin_range()?
>     That would require changing all drivers.
> 
>   - What happens if you have multiple hogs in multiple ranges?
>     The first hog(s) may be configured multiple times.  Is that a problem?
> 
>   - In one of the threads that discussed the GPIO hogging mechanism, Maxime
>     Ripard said: "Our pinctrl driver is also our GPIO driver, so they both
>     share the same node."
>     Maxime: Did you try GPIO hogging? Did it work?
>     If yes, which driver are you using? What's different compared to sh-pfc?
>     If no, did you get it to work?

I'm using pinctrl-sunxi, and no, I haven't tried it yet, so it
probably have the issue you reported :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[Alexandre Courbot]

On Tue, Jun 16, 2015 at 9:36 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> If a GPIO driver uses gpiochip_add_pin_range() (which is usually the
> case for GPIO/PFC combos), the GPIO hogging mechanism configured from DT
> doesn't work:
>
>     requesting hog GPIO lcd0 (chip r8a7740_pfc, offset 176) failed
>
> The actual error code is -517 == -EPROBE_DEFER.
>
> The problem is that PFC+GPIO registration is handled in multiple steps:
>   1. pinctrl_register(),
>   2. gpiochip_add(),
>   3. gpiochip_add_pin_range().
>
> Configuration of the hogs is handled in gpiochip_add():
>
>     gpiochip_add
>         of_gpiochip_add
>             of_gpiochip_scan_hogs
>                 gpiod_hog
>                     gpiochip_request_own_desc
>                         __gpiod_request
>                             chip->request
>                                 pinctrl_request_gpio
>                                     pinctrl_get_device_gpio_range
>
> However, at this point the GPIO controller hasn't been added to
> pinctrldev_list yet, so the range can't be found, and the operation fails
> with -EPROBE_DEFER.
>
>   - Exchanging the order of the calls to gpiochip_add() and
>     gpiochip_add_pin_range() is not an option, as the latter depends on
>     initialization done by the former.
>   - Just moving the call of of_gpiochip_scan_hogs() from gpiochip_add()
>     to gpiochip_add_pin_range() is also not an option, as the latter is
>     optional, and thus not used by all drivers.
>
> Hence if of_gpiochip_scan_hogs() fails with -EPROBE_DEFER, call it
> again every time the pin range is changed, until it succeeded.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Questions:
>   - Is there a better solution to handle this?

I do not understand the issue well enough to propose a better
solution, but I really hope there is one. This turns GPIO into a
slightly bigger yarn mess that what it already is and does not help
understanding how probing is performed. Yielding in the middle of
adding hogs and re-trying later sounds like a recipe for issues like
hogs being added several times.

So I am not really fond of this, to be honest. The GPIO hogging
mechanism is still quite new, so there is certainly a way to fix such
issues by addressing the fundamental cause instead of duct taping it?

>
>   - Should the pin ranges be configured by passing an array of data to
>     gpiochip_add() instead of having calls to gpiochip_add_pin_range()?
>     That would require changing all drivers.
>
>   - What happens if you have multiple hogs in multiple ranges?
>     The first hog(s) may be configured multiple times.  Is that a problem?
>
>   - In one of the threads that discussed the GPIO hogging mechanism, Maxime
>     Ripard said: "Our pinctrl driver is also our GPIO driver, so they both
>     share the same node."
>     Maxime: Did you try GPIO hogging? Did it work?
>     If yes, which driver are you using? What's different compared to sh-pfc?
>     If no, did you get it to work?
>
> Thanks!
> ---
>  drivers/gpio/gpiolib-of.c   | 21 +++++++++++++++++----
>  drivers/gpio/gpiolib.c      |  1 +
>  include/linux/gpio/driver.h |  1 +
>  include/linux/of_gpio.h     |  2 ++
>  4 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 9a0ec48a47375d18..90dd02b19f75c27c 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -205,13 +205,14 @@ static struct gpio_desc *of_get_gpio_hog(struct device_node *np,
>   * This is only used by of_gpiochip_add to request/set GPIO initial
>   * configuration.
>   */
> -static void of_gpiochip_scan_hogs(struct gpio_chip *chip)
> +static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
>  {
>         struct gpio_desc *desc = NULL;
>         struct device_node *np;
>         const char *name;
>         enum gpio_lookup_flags lflags;
>         enum gpiod_flags dflags;
> +       int error;
>
>         for_each_child_of_node(chip->of_node, np) {
>                 if (!of_property_read_bool(np, "gpio-hog"))
> @@ -221,9 +222,12 @@ static void of_gpiochip_scan_hogs(struct gpio_chip *chip)
>                 if (IS_ERR(desc))
>                         continue;
>
> -               if (gpiod_hog(desc, name, lflags, dflags))
> -                       continue;
> +               error = gpiod_hog(desc, name, lflags, dflags);
> +               if (error == -EPROBE_DEFER)
> +                       return error;
>         }
> +
> +       return 0;
>  }
>
>  /**
> @@ -416,8 +420,17 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
>         }
>  }
>
> +void of_gpiochip_pin_range_changed(struct gpio_chip *chip)
> +{
> +       if (chip->hog_error) {
> +               /* Retry */
> +               chip->hog_error = of_gpiochip_scan_hogs(chip);
> +       }
> +}
> +
>  #else
>  static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {}
> +void of_gpiochip_pin_range_changed(struct gpio_chip *chip) {}
>  #endif
>
>  void of_gpiochip_add(struct gpio_chip *chip)
> @@ -436,7 +449,7 @@ void of_gpiochip_add(struct gpio_chip *chip)
>         of_gpiochip_add_pin_range(chip);
>         of_node_get(chip->of_node);
>
> -       of_gpiochip_scan_hogs(chip);
> +       chip->hog_error = of_gpiochip_scan_hogs(chip);
>  }
>
>  void of_gpiochip_remove(struct gpio_chip *chip)
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 957ede5664cfe168..b0fe7a459d8835bb 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -759,6 +759,7 @@ int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
>
>         list_add_tail(&pin_range->node, &chip->pin_ranges);
>
> +       of_gpiochip_pin_range_changed(chip);
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(gpiochip_add_pin_range);
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index c8393cd4d44f2d87..9396b68dced2c5b1 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -146,6 +146,7 @@ struct gpio_chip {
>          * corresponding pins for gpio usage.
>          */
>         struct list_head pin_ranges;
> +       int hog_error;
>  #endif
>  };
>
> diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
> index 69dbe312b11b23f6..34421f17f4712d0b 100644
> --- a/include/linux/of_gpio.h
> +++ b/include/linux/of_gpio.h
> @@ -54,6 +54,7 @@ extern int of_mm_gpiochip_add(struct device_node *np,
>                               struct of_mm_gpio_chip *mm_gc);
>  extern void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc);
>
> +extern void of_gpiochip_pin_range_changed(struct gpio_chip *chip);
>  extern void of_gpiochip_add(struct gpio_chip *gc);
>  extern void of_gpiochip_remove(struct gpio_chip *gc);
>  extern int of_gpio_simple_xlate(struct gpio_chip *gc,
> @@ -76,6 +77,7 @@ static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
>         return -ENOSYS;
>  }
>
> +static inline void of_gpiochip_pin_range_changed(struct gpio_chip *chip) { }
>  static inline void of_gpiochip_add(struct gpio_chip *gc) { }
>  static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[Geert Uytterhoeven]

Hi Alex,

On Sun, Jun 21, 2015 at 9:21 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Tue, Jun 16, 2015 at 9:36 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>> If a GPIO driver uses gpiochip_add_pin_range() (which is usually the
>> case for GPIO/PFC combos), the GPIO hogging mechanism configured from DT
>> doesn't work:
>>
>>     requesting hog GPIO lcd0 (chip r8a7740_pfc, offset 176) failed
>>
>> The actual error code is -517 == -EPROBE_DEFER.
>>
>> The problem is that PFC+GPIO registration is handled in multiple steps:
>>   1. pinctrl_register(),
>>   2. gpiochip_add(),
>>   3. gpiochip_add_pin_range().
>>
>> Configuration of the hogs is handled in gpiochip_add():
>>
>>     gpiochip_add
>>         of_gpiochip_add
>>             of_gpiochip_scan_hogs
>>                 gpiod_hog
>>                     gpiochip_request_own_desc
>>                         __gpiod_request
>>                             chip->request
>>                                 pinctrl_request_gpio
>>                                     pinctrl_get_device_gpio_range
>>
>> However, at this point the GPIO controller hasn't been added to
>> pinctrldev_list yet, so the range can't be found, and the operation fails
>> with -EPROBE_DEFER.
>>
>>   - Exchanging the order of the calls to gpiochip_add() and
>>     gpiochip_add_pin_range() is not an option, as the latter depends on
>>     initialization done by the former.
>>   - Just moving the call of of_gpiochip_scan_hogs() from gpiochip_add()
>>     to gpiochip_add_pin_range() is also not an option, as the latter is
>>     optional, and thus not used by all drivers.
>>
>> Hence if of_gpiochip_scan_hogs() fails with -EPROBE_DEFER, call it
>> again every time the pin range is changed, until it succeeded.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Questions:
>>   - Is there a better solution to handle this?
>
> I do not understand the issue well enough to propose a better
> solution, but I really hope there is one. This turns GPIO into a
> slightly bigger yarn mess that what it already is and does not help
> understanding how probing is performed. Yielding in the middle of
> adding hogs and re-trying later sounds like a recipe for issues like
> hogs being added several times.
>
> So I am not really fond of this, to be honest. The GPIO hogging
> mechanism is still quite new, so there is certainly a way to fix such
> issues by addressing the fundamental cause instead of duct taping it?

Sure, I'm all for fixing this properly, hence the "RFC" and my questions.

I also don't understand how this interacts with non-PFC drivers calling
gpiochip_add_pin_range():
  - gpio-em, but only for legacy platform devices, which are no longer used
    (I will remove the legacy support),
  - gpio-rcar, but only for legacy platform devices, which is used on R-Car
    Gen1 only until -legacy is removed,
  - gpiolib-of, which handles this for the bulk of modern GPIO drivers using
    the "gpio-ranges" and "gpio-ranges-group-names" properties in DT.

When I noticed the failure on r8a7740/armadillo (sh-pfc provides both pfc
and gpio), I tried GPIO hogging on r8a7791/koelsch (sh-pfc provides pfc
only, gpio-rcar provides gpio, "gpio-ranges" is in DT), and there it worked
fine without my patch.

Thanks!

>>   - Should the pin ranges be configured by passing an array of data to
>>     gpiochip_add() instead of having calls to gpiochip_add_pin_range()?
>>     That would require changing all drivers.
>>
>>   - What happens if you have multiple hogs in multiple ranges?
>>     The first hog(s) may be configured multiple times.  Is that a problem?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change

[Geert Uytterhoeven]

On Sun, Jun 21, 2015 at 10:52 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Sun, Jun 21, 2015 at 9:21 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Tue, Jun 16, 2015 at 9:36 PM, Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>>> If a GPIO driver uses gpiochip_add_pin_range() (which is usually the
>>> case for GPIO/PFC combos), the GPIO hogging mechanism configured from DT
>>> doesn't work:
>>>
>>>     requesting hog GPIO lcd0 (chip r8a7740_pfc, offset 176) failed
>>>
>>> The actual error code is -517 == -EPROBE_DEFER.
>>>
>>> The problem is that PFC+GPIO registration is handled in multiple steps:
>>>   1. pinctrl_register(),
>>>   2. gpiochip_add(),
>>>   3. gpiochip_add_pin_range().
>>>
>>> Configuration of the hogs is handled in gpiochip_add():
>>>
>>>     gpiochip_add
>>>         of_gpiochip_add
>>>             of_gpiochip_scan_hogs
>>>                 gpiod_hog
>>>                     gpiochip_request_own_desc
>>>                         __gpiod_request
>>>                             chip->request
>>>                                 pinctrl_request_gpio
>>>                                     pinctrl_get_device_gpio_range
>>>
>>> However, at this point the GPIO controller hasn't been added to
>>> pinctrldev_list yet, so the range can't be found, and the operation fails
>>> with -EPROBE_DEFER.
>>>
>>>   - Exchanging the order of the calls to gpiochip_add() and
>>>     gpiochip_add_pin_range() is not an option, as the latter depends on
>>>     initialization done by the former.
>>>   - Just moving the call of of_gpiochip_scan_hogs() from gpiochip_add()
>>>     to gpiochip_add_pin_range() is also not an option, as the latter is
>>>     optional, and thus not used by all drivers.
>>>
>>> Hence if of_gpiochip_scan_hogs() fails with -EPROBE_DEFER, call it
>>> again every time the pin range is changed, until it succeeded.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Questions:
>>>   - Is there a better solution to handle this?
>>
>> I do not understand the issue well enough to propose a better
>> solution, but I really hope there is one. This turns GPIO into a
>> slightly bigger yarn mess that what it already is and does not help
>> understanding how probing is performed. Yielding in the middle of
>> adding hogs and re-trying later sounds like a recipe for issues like
>> hogs being added several times.
>>
>> So I am not really fond of this, to be honest. The GPIO hogging
>> mechanism is still quite new, so there is certainly a way to fix such
>> issues by addressing the fundamental cause instead of duct taping it?
>
> Sure, I'm all for fixing this properly, hence the "RFC" and my questions.
>
> I also don't understand how this interacts with non-PFC drivers calling
> gpiochip_add_pin_range():
>   - gpio-em, but only for legacy platform devices, which are no longer used
>     (I will remove the legacy support),
>   - gpio-rcar, but only for legacy platform devices, which is used on R-Car
>     Gen1 only until -legacy is removed,
>   - gpiolib-of, which handles this for the bulk of modern GPIO drivers using
>     the "gpio-ranges" and "gpio-ranges-group-names" properties in DT.
>
> When I noticed the failure on r8a7740/armadillo (sh-pfc provides both pfc
> and gpio), I tried GPIO hogging on r8a7791/koelsch (sh-pfc provides pfc
> only, gpio-rcar provides gpio, "gpio-ranges" is in DT), and there it worked
> fine without my patch.

"gpio-ranges" and gpiochip_add_pin_range() turned out to be the solution
to the problem: on DT platforms, parsing "gpio-ranges" is doing from
of_gpiochip_add(), which is called from gpiochip_add().
Hence the ranges are set up from DT just before the hogs are handled:

void of_gpiochip_add(struct gpio_chip *chip)
{
        ...
        of_gpiochip_add_pin_range(chip);
        ...
        of_gpiochip_scan_hogs(chip);
}

Sticking a "gpio-ranges" in arch/arm/boot/dts/r8a7740.dtsi:

@@ -288,12 +288,13 @@
        pfc: pfc@e6050000 {
                compatible = "renesas,pfc-r8a7740";
                reg = <0xe6050000 0x8000>,
                      <0xe605800c 0x20>;
                gpio-controller;
                #gpio-cells = <2>;
+               gpio-ranges = <&pfc 0 0 212>;
                interrupts-extended =
                        <&irqpin0 0 0>, <&irqpin0 1 0>, <&irqpin0 2
0>, <&irqpin0 3 0>,
                        <&irqpin0 4 0>, <&irqpin0 5 0>, <&irqpin0 6
0>, <&irqpin0 7 0>,
                        <&irqpin1 0 0>, <&irqpin1 1 0>, <&irqpin1 2
0>, <&irqpin1 3 0>,
                        <&irqpin1 4 0>, <&irqpin1 5 0>, <&irqpin1 6
0>, <&irqpin1 7 0>,
                        <&irqpin2 0 0>, <&irqpin2 1 0>, <&irqpin2 2
0>, <&irqpin2 3 0>,

solved the problem for me.

Note that "&pfc" is a reference to the gpio device node itself, as it provides
both GPIO and PFC functionalities.

After that, the calls to gpiochip_add_pin_range() in
drivers/pinctrl/sh-pfc/gpio.c
can be removed, at least for the ARM multi-platform case where GPIO is
instantiated from DT (_and_ "gpio-ranges" is present --- I don't think we have
to care about DT backwards compatibility for sh73a0/r8a73a4/r8a7740).

Does this makes sense?

I couldn't find any other in-tree DTS that has a gpio-controller with a
gpio-ranges pointing to itself. All other GPIO+PFC combos lack such properties,
and thus probably won't work with DT gpio-hogs.

Thanks for your comments!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in