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

[Jonas Karlman]

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 at 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 at 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 at 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-massimo.pegorer+oss@gmail.com/
> 
> [2]
> https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-massimo.pegorer+oss@gmail.com/
>