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

From: Geert Uytterhoeven
Date: Tue Jun 16 2015 - 09:05:14 EST


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@xxxxxxxxx>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/