Hello,
these three patches make sure that there's an explicit dependency from
the GPIO chip in Tegra SoCs to the corresponding pinctrl device, without
having duplicated gpio ranges.
By having an explicit dependency, we can do things such as probing the
pinctrl device before the GPIO chip device to avoid deferred probes.
Thanks,
Tomeu
Tomeu Vizoso (3):
gpio: defer probe if pinctrl cannot be found
pinctrl: tegra: Only set the gpio range if needed
ARM: tegra: Add gpio-ranges property
arch/arm/boot/dts/tegra114.dtsi | 1 +
arch/arm/boot/dts/tegra124.dtsi | 1 +
arch/arm/boot/dts/tegra20.dtsi | 1 +
arch/arm/boot/dts/tegra30.dtsi | 1 +
drivers/gpio/gpiolib-of.c | 27 ++++++++++++++++++---------
drivers/gpio/gpiolib.c | 5 ++++-
drivers/pinctrl/pinctrl-tegra.c | 19 ++++++++++++++++++-
include/linux/of_gpio.h | 4 ++--
8 files changed, 46 insertions(+), 13 deletions(-)
--
2.4.1
When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
the pin controller isn't available.
Otherwise, the GPIO range wouldn't be set at all unless the pin
controller probed always before the GPIO chip.
With this change, the probe of the GPIO chip will be deferred and will
be retried at a later point, hopefully once the pin controller has been
registered and probed already.
Signed-off-by: Tomeu Vizoso <[email protected]>
---
drivers/gpio/gpiolib-of.c | 27 ++++++++++++++++++---------
drivers/gpio/gpiolib.c | 5 ++++-
include/linux/of_gpio.h | 4 ++--
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 9a0ec48..4e1f73b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -338,7 +338,7 @@ void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc)
EXPORT_SYMBOL(of_mm_gpiochip_remove);
#ifdef CONFIG_PINCTRL
-static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
+static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
{
struct device_node *np = chip->of_node;
struct of_phandle_args pinspec;
@@ -349,7 +349,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
struct property *group_names;
if (!np)
- return;
+ return 0;
group_names = of_find_property(np, group_names_propname, NULL);
@@ -361,7 +361,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
pctldev = of_pinctrl_get(pinspec.np);
if (!pctldev)
- break;
+ return -EPROBE_DEFER;
if (pinspec.args[2]) {
if (group_names) {
@@ -381,7 +381,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
pinspec.args[1],
pinspec.args[2]);
if (ret)
- break;
+ return ret;
} else {
/* npins == 0: special range */
if (pinspec.args[1]) {
@@ -411,32 +411,41 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
ret = gpiochip_add_pingroup_range(chip, pctldev,
pinspec.args[0], name);
if (ret)
- break;
+ return ret;
}
}
+
+ return 0;
}
#else
-static void of_gpiochip_add_pin_range(struct gpio_chip *chip) {}
+static int of_gpiochip_add_pin_range(struct gpio_chip *chip) { return 0; }
#endif
-void of_gpiochip_add(struct gpio_chip *chip)
+int of_gpiochip_add(struct gpio_chip *chip)
{
+ int status;
+
if ((!chip->of_node) && (chip->dev))
chip->of_node = chip->dev->of_node;
if (!chip->of_node)
- return;
+ return 0;
if (!chip->of_xlate) {
chip->of_gpio_n_cells = 2;
chip->of_xlate = of_gpio_simple_xlate;
}
- of_gpiochip_add_pin_range(chip);
+ status = of_gpiochip_add_pin_range(chip);
+ if (status)
+ return status;
+
of_node_get(chip->of_node);
of_gpiochip_scan_hogs(chip);
+
+ return 0;
}
void of_gpiochip_remove(struct gpio_chip *chip)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bf4bd1d..a8cab33 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -287,7 +287,10 @@ int gpiochip_add(struct gpio_chip *chip)
INIT_LIST_HEAD(&chip->pin_ranges);
#endif
- of_gpiochip_add(chip);
+ status = of_gpiochip_add(chip);
+ if (status)
+ goto err_remove_chip;
+
acpi_gpiochip_add(chip);
status = gpiochip_sysfs_register(chip);
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 69dbe31..f319182 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -54,7 +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_add(struct gpio_chip *gc);
+extern int 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,
const struct of_phandle_args *gpiospec,
@@ -76,7 +76,7 @@ static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
return -ENOSYS;
}
-static inline void of_gpiochip_add(struct gpio_chip *gc) { }
+static inline int of_gpiochip_add(struct gpio_chip *gc) { return 0; }
static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
#endif /* CONFIG_OF_GPIO */
--
2.4.1
If the gpio DT node has the gpio-ranges property, the range will be
added by the gpio core and doesn't need to be added by the pinctrl
driver.
By having the gpio-ranges property, we have an explicit dependency from
the gpio node to the pinctrl node and we can stop using the deprecated
pinctrl_add_gpio_range() function.
Signed-off-by: Tomeu Vizoso <[email protected]>
---
drivers/pinctrl/pinctrl-tegra.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
index 0f982b8..0fd7fd2 100644
--- a/drivers/pinctrl/pinctrl-tegra.c
+++ b/drivers/pinctrl/pinctrl-tegra.c
@@ -624,6 +624,22 @@ static struct pinctrl_desc tegra_pinctrl_desc = {
.owner = THIS_MODULE,
};
+static bool gpio_node_has_range(void)
+{
+ struct device_node *np;
+ bool has_prop = false;
+
+ np = of_find_compatible_node(NULL, NULL, "nvidia,tegra30-gpio");
+ if (!np)
+ return has_prop;
+
+ has_prop = of_find_property(np, "gpio-ranges", NULL);
+
+ of_node_put(np);
+
+ return has_prop;
+}
+
int tegra_pinctrl_probe(struct platform_device *pdev,
const struct tegra_pinctrl_soc_data *soc_data)
{
@@ -708,7 +724,8 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
return PTR_ERR(pmx->pctl);
}
- pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range);
+ if (!gpio_node_has_range())
+ pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range);
platform_set_drvdata(pdev, pmx);
--
2.4.1
Specify how the GPIOs map to the pins in Tegra SoCs, so the dependency is
explicit.
This currently will add a duplicated entry in the map from pins to gpios
in the pinmux controller but it should be harmless and will be fixed in a
later commit.
Signed-off-by: Tomeu Vizoso <[email protected]>
---
arch/arm/boot/dts/tegra114.dtsi | 1 +
arch/arm/boot/dts/tegra124.dtsi | 1 +
arch/arm/boot/dts/tegra20.dtsi | 1 +
arch/arm/boot/dts/tegra30.dtsi | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index f58a3d9..7e1e171 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -234,6 +234,7 @@
gpio-controller;
#interrupt-cells = <2>;
interrupt-controller;
+ gpio-ranges = <&pinmux 0 0 246>;
};
apbmisc@70000800 {
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 87318a7..4779df3 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -255,6 +255,7 @@
gpio-controller;
#interrupt-cells = <2>;
interrupt-controller;
+ gpio-ranges = <&pinmux 0 0 251>;
};
apbdma: dma@0,60020000 {
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index f444b67..b44277c 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -244,6 +244,7 @@
gpio-controller;
#interrupt-cells = <2>;
interrupt-controller;
+ gpio-ranges = <&pinmux 0 0 224>;
};
apbmisc@70000800 {
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 782b11b..28c547f 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -349,6 +349,7 @@
gpio-controller;
#interrupt-cells = <2>;
interrupt-controller;
+ gpio-ranges = <&pinmux 0 0 248>;
};
apbmisc@70000800 {
--
2.4.1
On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
> the pin controller isn't available.
>
> Otherwise, the GPIO range wouldn't be set at all unless the pin
> controller probed always before the GPIO chip.
>
> With this change, the probe of the GPIO chip will be deferred and will
> be retried at a later point, hopefully once the pin controller has been
> registered and probed already.
This will break cases where the pinctrl driver does not exist, but the
DT contains pinctrl bindings. We can have similar problems already
with clocks though. However, IMO this problem is a bit different in
that pinctrl is more likely entirely optional while clocks are often
required. You may do all pin setup in bootloader/firmware on some
boards and not others. Of course then why put pinctrl in the DT in
that case? They could be present just due to how chip vs. board dts
files are structured.
We could address this by simply marking the pin controller node
disabled. However, ...
> @@ -361,7 +361,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
>
> pctldev = of_pinctrl_get(pinspec.np);
> if (!pctldev)
> - break;
> + return -EPROBE_DEFER;
But you cannot distinguish that case here. I think of_pinctrl_get
needs to set the error code appropriately.
Rob
Hi Tomeu,
On Wed, Jul 1, 2015 at 2:45 PM, Tomeu Vizoso <[email protected]> wrote:
> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
> the pin controller isn't available.
>
> Otherwise, the GPIO range wouldn't be set at all unless the pin
> controller probed always before the GPIO chip.
>
> With this change, the probe of the GPIO chip will be deferred and will
> be retried at a later point, hopefully once the pin controller has been
> registered and probed already.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
I was a bit afraid this would break the case of gpio controllers that are
also pin controllers, i.e. where "gpio-ranges" points to the gpio controller
itself[*], but it doesn't.
Tested-by: Geert Uytterhoeven <[email protected]>
[*] E.g. "[PATCH 2/7] ARM: shmobile: r8a7740 dtsi: Add missing "gpio-ranges"
to gpio node" (http://www.spinics.net/lists/linux-sh/msg43077.html)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Rob,
On Wed, Jul 1, 2015 at 7:36 PM, Rob Herring <[email protected]> wrote:
> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>> the pin controller isn't available.
>>
>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>> controller probed always before the GPIO chip.
>>
>> With this change, the probe of the GPIO chip will be deferred and will
>> be retried at a later point, hopefully once the pin controller has been
>> registered and probed already.
>
> This will break cases where the pinctrl driver does not exist, but the
> DT contains pinctrl bindings. We can have similar problems already
> with clocks though. However, IMO this problem is a bit different in
> that pinctrl is more likely entirely optional while clocks are often
> required. You may do all pin setup in bootloader/firmware on some
> boards and not others. Of course then why put pinctrl in the DT in
> that case? They could be present just due to how chip vs. board dts
> files are structured.
Isn't that already the case?
If I change the compatible value of a pinctrl node to an invalid value, I get:
sh-sci e6c50000.serial: could not find pctldev for node
/pfc@e6050000/serial1, deferring probe
> We could address this by simply marking the pin controller node
> disabled. However, ...
Doesn't seem to make any difference.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Thu, Jul 2, 2015 at 3:59 AM, Geert Uytterhoeven <[email protected]> wrote:
> Hi Rob,
>
> On Wed, Jul 1, 2015 at 7:36 PM, Rob Herring <[email protected]> wrote:
>> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
>>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>>> the pin controller isn't available.
>>>
>>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>>> controller probed always before the GPIO chip.
>>>
>>> With this change, the probe of the GPIO chip will be deferred and will
>>> be retried at a later point, hopefully once the pin controller has been
>>> registered and probed already.
>>
>> This will break cases where the pinctrl driver does not exist, but the
>> DT contains pinctrl bindings. We can have similar problems already
>> with clocks though. However, IMO this problem is a bit different in
>> that pinctrl is more likely entirely optional while clocks are often
>> required. You may do all pin setup in bootloader/firmware on some
>> boards and not others. Of course then why put pinctrl in the DT in
>> that case? They could be present just due to how chip vs. board dts
>> files are structured.
>
> Isn't that already the case?
> If I change the compatible value of a pinctrl node to an invalid value, I get:
>
> sh-sci e6c50000.serial: could not find pctldev for node
> /pfc@e6050000/serial1, deferring probe
I guess so.
>> We could address this by simply marking the pin controller node
>> disabled. However, ...
>
> Doesn't seem to make any difference.
No doubt. I'm proposing that it should, not that it does already. Of
course, the callers will also have to test for -ENODEV and ignore
those errors.
Rob
On 07/01/2015 06:45 AM, Tomeu Vizoso wrote:
> Specify how the GPIOs map to the pins in Tegra SoCs, so the dependency is
> explicit.
>
> This currently will add a duplicated entry in the map from pins to gpios
> in the pinmux controller but it should be harmless and will be fixed in a
> later commit.
Isn't it in an earlier commit now (patch 2/3)? :-)
At a quick glance, I think this approach will be fine, so the series,
Acked-by: Stephen Warren <[email protected]>
On 1 July 2015 at 19:36, Rob Herring <[email protected]> wrote:
> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>> the pin controller isn't available.
>>
>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>> controller probed always before the GPIO chip.
>>
>> With this change, the probe of the GPIO chip will be deferred and will
>> be retried at a later point, hopefully once the pin controller has been
>> registered and probed already.
>
> This will break cases where the pinctrl driver does not exist, but the
> DT contains pinctrl bindings. We can have similar problems already
> with clocks though. However, IMO this problem is a bit different in
> that pinctrl is more likely entirely optional while clocks are often
> required. You may do all pin setup in bootloader/firmware on some
> boards and not others. Of course then why put pinctrl in the DT in
> that case? They could be present just due to how chip vs. board dts
> files are structured.
I see. My instinct tells me that it would be better if the gpio-ranges
property was set in the board dts, but I don't really know what each
mach does with its DTSs.
> We could address this by simply marking the pin controller node
> disabled. However, ...
>
>> @@ -361,7 +361,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
>>
>> pctldev = of_pinctrl_get(pinspec.np);
>> if (!pctldev)
>> - break;
>> + return -EPROBE_DEFER;
>
> But you cannot distinguish that case here. I think of_pinctrl_get
> needs to set the error code appropriately.
Why not? I was thinking of just doing this before we call of_pinctrl_get():
if (!of_device_is_available(pinspec.np))
continue;
Thanks,
Tomeu
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On 07/10/2015 03:29 AM, Tomeu Vizoso wrote:
> On 1 July 2015 at 19:36, Rob Herring <[email protected]> wrote:
>> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
>>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>>> the pin controller isn't available.
>>>
>>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>>> controller probed always before the GPIO chip.
>>>
>>> With this change, the probe of the GPIO chip will be deferred and will
>>> be retried at a later point, hopefully once the pin controller has been
>>> registered and probed already.
>>
>> This will break cases where the pinctrl driver does not exist, but the
>> DT contains pinctrl bindings. We can have similar problems already
>> with clocks though. However, IMO this problem is a bit different in
>> that pinctrl is more likely entirely optional while clocks are often
>> required. You may do all pin setup in bootloader/firmware on some
>> boards and not others. Of course then why put pinctrl in the DT in
>> that case? They could be present just due to how chip vs. board dts
>> files are structured.
>
> I see. My instinct tells me that it would be better if the gpio-ranges
> property was set in the board dts, but I don't really know what each
> mach does with its DTSs.
That doesn't make sense; the mapping between GPIO controller pins and
pin controller pins is a property of the SoC not the board.
On 10 July 2015 at 17:27, Stephen Warren <[email protected]> wrote:
> On 07/10/2015 03:29 AM, Tomeu Vizoso wrote:
>>
>> On 1 July 2015 at 19:36, Rob Herring <[email protected]> wrote:
>>>
>>> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]>
>>> wrote:
>>>>
>>>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>>>> the pin controller isn't available.
>>>>
>>>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>>>> controller probed always before the GPIO chip.
>>>>
>>>> With this change, the probe of the GPIO chip will be deferred and will
>>>> be retried at a later point, hopefully once the pin controller has been
>>>> registered and probed already.
>>>
>>>
>>> This will break cases where the pinctrl driver does not exist, but the
>>> DT contains pinctrl bindings. We can have similar problems already
>>> with clocks though. However, IMO this problem is a bit different in
>>> that pinctrl is more likely entirely optional while clocks are often
>>> required. You may do all pin setup in bootloader/firmware on some
>>> boards and not others. Of course then why put pinctrl in the DT in
>>> that case? They could be present just due to how chip vs. board dts
>>> files are structured.
>>
>>
>> I see. My instinct tells me that it would be better if the gpio-ranges
>> property was set in the board dts, but I don't really know what each
>> mach does with its DTSs.
>
>
> That doesn't make sense; the mapping between GPIO controller pins and pin
> controller pins is a property of the SoC not the board.
>From what Rob said above, apparently some boards will rely on the pin
setup done by the bootloader, and some other boards with the same soc
will want to do it in the kernel. So it's not really a difference in
the hw itself, but what expectations exist about the firmware on a
specific board.
Regards,
Tomeu
On 07/10/2015 10:21 AM, Tomeu Vizoso wrote:
> On 10 July 2015 at 17:27, Stephen Warren <[email protected]> wrote:
>> On 07/10/2015 03:29 AM, Tomeu Vizoso wrote:
>>>
>>> On 1 July 2015 at 19:36, Rob Herring <[email protected]> wrote:
>>>>
>>>> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]>
>>>> wrote:
>>>>>
>>>>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>>>>> the pin controller isn't available.
>>>>>
>>>>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>>>>> controller probed always before the GPIO chip.
>>>>>
>>>>> With this change, the probe of the GPIO chip will be deferred and will
>>>>> be retried at a later point, hopefully once the pin controller has been
>>>>> registered and probed already.
>>>>
>>>>
>>>> This will break cases where the pinctrl driver does not exist, but the
>>>> DT contains pinctrl bindings. We can have similar problems already
>>>> with clocks though. However, IMO this problem is a bit different in
>>>> that pinctrl is more likely entirely optional while clocks are often
>>>> required. You may do all pin setup in bootloader/firmware on some
>>>> boards and not others. Of course then why put pinctrl in the DT in
>>>> that case? They could be present just due to how chip vs. board dts
>>>> files are structured.
>>>
>>>
>>> I see. My instinct tells me that it would be better if the gpio-ranges
>>> property was set in the board dts, but I don't really know what each
>>> mach does with its DTSs.
>>
>>
>> That doesn't make sense; the mapping between GPIO controller pins and pin
>> controller pins is a property of the SoC not the board.
>
> From what Rob said above, apparently some boards will rely on the pin
> setup done by the bootloader, and some other boards with the same soc
> will want to do it in the kernel. So it's not really a difference in
> the hw itself, but what expectations exist about the firmware on a
> specific board.
Sure, but none of that changes the mapping between the GPIO and pin
controller pins.
On Fri, Jul 10, 2015 at 4:29 AM, Tomeu Vizoso
<[email protected]> wrote:
> On 1 July 2015 at 19:36, Rob Herring <[email protected]> wrote:
>> On Wed, Jul 1, 2015 at 7:45 AM, Tomeu Vizoso <[email protected]> wrote:
>>> When an OF node has a pin range for its GPIOs, return -EPROBE_DEFER if
>>> the pin controller isn't available.
>>>
>>> Otherwise, the GPIO range wouldn't be set at all unless the pin
>>> controller probed always before the GPIO chip.
>>>
>>> With this change, the probe of the GPIO chip will be deferred and will
>>> be retried at a later point, hopefully once the pin controller has been
>>> registered and probed already.
>>
>> This will break cases where the pinctrl driver does not exist, but the
>> DT contains pinctrl bindings. We can have similar problems already
>> with clocks though. However, IMO this problem is a bit different in
>> that pinctrl is more likely entirely optional while clocks are often
>> required. You may do all pin setup in bootloader/firmware on some
>> boards and not others. Of course then why put pinctrl in the DT in
>> that case? They could be present just due to how chip vs. board dts
>> files are structured.
>
> I see. My instinct tells me that it would be better if the gpio-ranges
> property was set in the board dts, but I don't really know what each
> mach does with its DTSs.
>
>> We could address this by simply marking the pin controller node
>> disabled. However, ...
>>
>>> @@ -361,7 +361,7 @@ static void of_gpiochip_add_pin_range(struct gpio_chip *chip)
>>>
>>> pctldev = of_pinctrl_get(pinspec.np);
>>> if (!pctldev)
>>> - break;
>>> + return -EPROBE_DEFER;
>>
>> But you cannot distinguish that case here. I think of_pinctrl_get
>> needs to set the error code appropriately.
>
> Why not? I was thinking of just doing this before we call of_pinctrl_get():
>
> if (!of_device_is_available(pinspec.np))
> continue;
That is exactly what you need, but that should be of_pinctrl_get's
responsibility to check, not the caller's. IIRC, this is the only user
of of_pinctrl_get, so it should be just as easy to change.
Rob