* [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-16 12:36 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-16 12:36 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot, Benoit Parrot, Maxime Ripard,
Boris Brezillon
Cc: linux-gpio, linux-sh, linux-kernel, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-16 12:36 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-16 12:36 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot, Benoit Parrot, Maxime Ripard,
Boris Brezillon
Cc: linux-gpio, linux-sh, linux-kernel, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
2015-06-16 12:36 ` Geert Uytterhoeven
@ 2015-06-16 17:27 ` Maxime Ripard
-1 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2015-06-16 17:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Alexandre Courbot, Benoit Parrot, Boris Brezillon,
linux-gpio, linux-sh, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-16 17:27 ` Maxime Ripard
0 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2015-06-16 17:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Alexandre Courbot, Benoit Parrot, Boris Brezillon,
linux-gpio, linux-sh, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
2015-06-16 12:36 ` Geert Uytterhoeven
(?)
@ 2015-06-21 7:21 ` Alexandre Courbot
-1 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2015-06-21 7:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Benoit Parrot, Maxime Ripard, Boris Brezillon,
linux-gpio, Linux-SH, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-21 7:21 ` Alexandre Courbot
0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2015-06-21 7:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Benoit Parrot, Maxime Ripard, Boris Brezillon,
linux-gpio, Linux-SH, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-21 7:21 ` Alexandre Courbot
0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2015-06-21 7:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Benoit Parrot, Maxime Ripard, Boris Brezillon,
linux-gpio, Linux-SH, Linux Kernel Mailing List
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-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
2015-06-21 7:21 ` Alexandre Courbot
(?)
@ 2015-06-21 8:52 ` Geert Uytterhoeven
-1 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-21 8:52 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-21 8:52 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-21 8:52 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-21 8:52 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-21 8:52 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
2015-06-21 8:52 ` Geert Uytterhoeven
(?)
@ 2015-06-22 10:25 ` Geert Uytterhoeven
-1 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 10:25 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-22 10:25 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 10:25 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change
@ 2015-06-22 10:25 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 10:25 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Geert Uytterhoeven, Linus Walleij, Benoit Parrot, Maxime Ripard,
Boris Brezillon, linux-gpio, Linux-SH, Linux Kernel Mailing List,
Laurent Pinchart
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-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-06-22 10:25 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-16 12:36 [PATCH] [RFC] gpio: Retry deferred GPIO hogging on pin range change Geert Uytterhoeven
2015-06-16 12:36 ` Geert Uytterhoeven
2015-06-16 17:27 ` Maxime Ripard
2015-06-16 17:27 ` Maxime Ripard
2015-06-21 7:21 ` Alexandre Courbot
2015-06-21 7:21 ` Alexandre Courbot
2015-06-21 7:21 ` Alexandre Courbot
2015-06-21 8:52 ` Geert Uytterhoeven
2015-06-21 8:52 ` Geert Uytterhoeven
2015-06-21 8:52 ` Geert Uytterhoeven
2015-06-22 10:25 ` Geert Uytterhoeven
2015-06-22 10:25 ` Geert Uytterhoeven
2015-06-22 10:25 ` Geert Uytterhoeven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.