On Tue, 2019-10-29 at 09:21 +1300, Chris Packham wrote:

On Fri, 2019-10-25 at 10:26 -0700, Florian Fainelli wrote:

quoted

On 10/24/19 9:00 PM, Chris Packham wrote:

quoted

The pinctrl node is used by the gpioa node. Which may have more
descendants at a board level. If the pinctrl node isn't probed first the
gpio is deferred and anything that needs a gpio pin on that chip is also
deferred.

If what you care is to optimize your boot flow such that no re-probing
occurs, maybe another solution to look at is to re-order the order in
which subsystems are initialized or built (_initcall changes or
drivers/Makefile changes), because changing Device Tree certainly does
not scale over platforms and I recall Rob indicating that he wanted to
introduce randomized platform_device creation from
of_platform_bus_populate() at one point or another.

Hmm. I might be missing something. pinctrl-nsp-gpio.c uses
arch_initcall_sync() and pinctrl-nsp-mux.c uses arch_initcall() so in
theory they are already in the right order.

Actually the init calls are made in the required order w.r.t each
other. But they are both before of_platform_populate, so it's back to
the device tree being the determining factor for when the probe()
functions are run.

With the current kernel I get

nsp_pinmux_init:
nsp_gpio_init:
OF: of_platform_populate:
OF: of_platform_bus_create: /axi@18000000/gpio@20
nsp_gpio_probe:
gpiochip_add_data_with_key: GPIOs 480..511 (18000020.gpio) failed to
register, -517
nsp-gpio-a 18000020.gpio: unable to add GPIO chip
OF: of_platform_bus_create: /axi@18000000/pinctrl@3f1c0
nsp_pinmux_probe:
... much later ...
nsp_gpio_probe:

Would it be acceptable to change the init calls to device_initcall()
and device_initcall_sync()? pinctrl-nsp-mux.c could even be converted
to (builtin_)platform_driver.

quoted

quoted

Normally we and nodes in the device tree to be listed in their natural
memory mapped address order but putting the pinctrl node first avoids
the deferral of numerous devices so make an exception in this case.

That is a workaround more than a real solution, though I understand why
you would to do that. One downside is that the entries are no longer in
incrementing register address order and that is visually disturbing and
who knows, maybe a drive by contributor whose pet project will be to
order the Device Tree entries by incrementing addresses will change that
in the future...

I guess really what's needed is something that understands phandles and
tries to produce a dependency tree based on that.

quoted

quoted

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/bcm-nsp.dtsi | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index da6d70f09ef1..dd7a65743c08 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi

@@ -172,6 +172,13 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		pinctrl: pinctrl@3f1c0 {
+			compatible = "brcm,nsp-pinmux";
+			reg = <0x3f1c0 0x04>,
+			      <0x30028 0x04>,
+			      <0x3f408 0x04>;
+		};
+
 		gpioa: gpio@20 {
 			compatible = "brcm,nsp-gpio-a";
 			reg = <0x0020 0x70>,

@@ -458,13 +465,6 @@
 					     "sata2";
 		};
 
-		pinctrl: pinctrl@3f1c0 {
-			compatible = "brcm,nsp-pinmux";
-			reg = <0x3f1c0 0x04>,
-			      <0x30028 0x04>,
-			      <0x3f408 0x04>;
-		};
-
 		thermal: thermal@3f2c0 {
 			compatible = "brcm,ns-thermal";
 			reg = <0x3f2c0 0x10>;