Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Hi Simon,
Il giorno ven 11 ago 2023 alle ore 02:29 Simon Glass
ha scritto:
>
> Hi Jonas,
>
> On Sat, 5 Aug 2023 at 08:29, Jonas Karlman 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 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@fe66: 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
> > >> ---
> > >> 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
Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Hi Jonas,
On Sat, 5 Aug 2023 at 08:29, Jonas Karlman 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 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@fe66: 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
> >> ---
> >> 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
Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
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 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@fe66: 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
>> ---
>> 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-massimo.pegorer+...@gmail.com/
>
> [2]
> https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-massimo.pegorer+...@gmail.com/
>
Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman 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@fe66: 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
> ---
> 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-massimo.pegorer+...@gmail.com/
[2]
https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-massimo.pegorer+...@gmail.com/
[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@fe66: 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
---
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
