[PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively

Pinconfig nodes normally bind recursively with PINCTRL_FULL and PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation any node marked with e.g. bootph-all will not bind unless its parent is also marked for pre-reloc.
group1 { pinconf1 { bootph-all; }; };
This cause the following warning message to be shown during U-Boot proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe660000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to make pinconfig_post_bind work same at both U-Boot proper pre-reloc and at TPL/SPL stage.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- drivers/pinctrl/pinctrl-uclass.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 73dd7b1038bb..fe2ba5021a78 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) return 0; }
+static bool ofnode_pre_reloc_recursive(ofnode parent) +{ + ofnode child; + + if (ofnode_pre_reloc(parent)) + return true; + + if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) { + ofnode_for_each_subnode(child, parent) + if (ofnode_pre_reloc_recursive(child)) + return true; + } + + return false; +} + /** * pinconfig_post_bind() - post binding for PINCONFIG uclass * Recursively bind its children as pinconfig devices. @@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
dev_for_each_subnode(node, dev) { if (pre_reloc_only && - !ofnode_pre_reloc(node)) + !ofnode_pre_reloc_recursive(node)) continue; /* * If this node has "compatible" property, this is not

Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman jonas@kwiboo.se ha scritto:
Pinconfig nodes normally bind recursively with PINCTRL_FULL and PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation any node marked with e.g. bootph-all will not bind unless its parent is also marked for pre-reloc.
group1 { pinconf1 { bootph-all; }; };
This cause the following warning message to be shown during U-Boot proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe660000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to make pinconfig_post_bind work same at both U-Boot proper pre-reloc and at TPL/SPL stage.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/pinctrl/pinctrl-uclass.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 73dd7b1038bb..fe2ba5021a78 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) return 0; }
+static bool ofnode_pre_reloc_recursive(ofnode parent) +{
ofnode child;
if (ofnode_pre_reloc(parent))
return true;
if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) {
ofnode_for_each_subnode(child, parent)
if (ofnode_pre_reloc_recursive(child))
return true;
}
return false;
+}
/**
- pinconfig_post_bind() - post binding for PINCONFIG uclass
- Recursively bind its children as pinconfig devices.
@@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
dev_for_each_subnode(node, dev) { if (pre_reloc_only &&
!ofnode_pre_reloc(node))
!ofnode_pre_reloc_recursive(node)) continue; /* * If this node has "compatible" property, this is not
-- 2.41.0
I've fixed the same issue for Rockchip RK3308 a couple of days ago with patch [1] as part of the series [2], adding required nodes with 'bootph-some-ram' property into *-u-boot.dtsi.
With your solution, is there any risk not to warn about any pin that should be configured but is not due to any ancestor missing in device tree (in pre-reloc boot phase, I mean)?
Thanks.
Massimo
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230803110813.175956-4-mas...
[2] https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-mas...

Hi Massimo,
On 2023-08-05 16:06, Massimo Pegorer wrote:
Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman jonas@kwiboo.se ha scritto:
Pinconfig nodes normally bind recursively with PINCTRL_FULL and PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation any node marked with e.g. bootph-all will not bind unless its parent is also marked for pre-reloc.
group1 { pinconf1 { bootph-all; }; };
This cause the following warning message to be shown during U-Boot proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe660000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to make pinconfig_post_bind work same at both U-Boot proper pre-reloc and at TPL/SPL stage.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/pinctrl/pinctrl-uclass.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 73dd7b1038bb..fe2ba5021a78 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) return 0; }
+static bool ofnode_pre_reloc_recursive(ofnode parent) +{
ofnode child;
if (ofnode_pre_reloc(parent))
return true;
if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) {
ofnode_for_each_subnode(child, parent)
if (ofnode_pre_reloc_recursive(child))
return true;
}
return false;
+}
/**
- pinconfig_post_bind() - post binding for PINCONFIG uclass
- Recursively bind its children as pinconfig devices.
@@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
dev_for_each_subnode(node, dev) { if (pre_reloc_only &&
!ofnode_pre_reloc(node))
!ofnode_pre_reloc_recursive(node)) continue; /* * If this node has "compatible" property, this is not
-- 2.41.0
I've fixed the same issue for Rockchip RK3308 a couple of days ago with patch [1] as part of the series [2], adding required nodes with 'bootph-some-ram' property into *-u-boot.dtsi.
With your solution, is there any risk not to warn about any pin that should be configured but is not due to any ancestor missing in device tree (in pre-reloc boot phase, I mean)?
Yes, with this any pinconf group that may contain a child/leaf node that have bootph prop will bind so that the child/leaf node also can bind and be referenced at pre-reloc phase. It is also fully recursive so should fix the issue with any missed ancestor, when it comes to pinconfig.
For TPL/SPL the ofnode_pre_reloc always return true so this has not been an issue at that phase. This change should make pinconfig_post_bind behave same as it does in TPL/SPL where any child/leaf node bind.
Regards, Jonas
Thanks.
Massimo
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230803110813.175956-4-mas...
[2] https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-mas...

Hi Jonas,
On Sat, 5 Aug 2023 at 08:29, Jonas Karlman jonas@kwiboo.se wrote:
Hi Massimo,
On 2023-08-05 16:06, Massimo Pegorer wrote:
Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman jonas@kwiboo.se ha scritto:
Pinconfig nodes normally bind recursively with PINCTRL_FULL and PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation any node marked with e.g. bootph-all will not bind unless its parent is also marked for pre-reloc.
group1 { pinconf1 { bootph-all; }; };
This cause the following warning message to be shown during U-Boot proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe660000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to make pinconfig_post_bind work same at both U-Boot proper pre-reloc and at TPL/SPL stage.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/pinctrl/pinctrl-uclass.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
This is actually something we want to change about fdtgrep. I believe we should try to adjust that tool rather than loading this ontoU-Boot, not least because it controls what properties are visible in SPL.
Regards, Simon

Hi Simon,
Il giorno ven 11 ago 2023 alle ore 02:29 Simon Glass sjg@chromium.org ha scritto:
Hi Jonas,
On Sat, 5 Aug 2023 at 08:29, Jonas Karlman jonas@kwiboo.se wrote:
Hi Massimo,
On 2023-08-05 16:06, Massimo Pegorer wrote:
Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman jonas@kwiboo.se ha scritto:
Pinconfig nodes normally bind recursively with PINCTRL_FULL and PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation any node marked with e.g. bootph-all will not bind unless its parent is also marked for pre-reloc.
group1 { pinconf1 { bootph-all; }; };
This cause the following warning message to be shown during U-Boot proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe660000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to make pinconfig_post_bind work same at both U-Boot proper pre-reloc and at TPL/SPL stage.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/pinctrl/pinctrl-uclass.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
This is actually something we want to change about fdtgrep. I believe we should try to adjust that tool rather than loading this ontoU-Boot, not least because it controls what properties are visible in SPL.
So IIUC your idea is that fdtgrep tool will automatically add bootph-xxx property to any ancestor of a node with that property? Or to log a warning, or exit with an error, during build?
BTW, fdtgrep is applied to DT only for xPL builds, not for U-Boot proper build. Therefore, Jonas approach could be used at least during U-Boot proper pre-relocation phase to manage "bootph-some-ram" and "bootph-all".
Side note: I've noticed that quiet_cmd_fdtgrep in Makefile.lib does not strip bootph-some-sram. I think it should, doesn't it?
Regards, Simon
participants (3)
-
Jonas Karlman
-
Massimo Pegorer
-
Simon Glass