[RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name

[Vignesh Raghavendra]

On 11/18/20 8:44 PM, Aswath Govindraju wrote:
> Hi Simon,
> 
> On 18/11/20 8:07 pm, Simon Glass wrote:
>> Hi Aswath,
>>
>> On Mon, 16 Nov 2020 at 07:29, Aswath Govindraju <a-govindraju at ti.com> wrote:
>>>
>>> While assigning the sequence number to subsystem instances by reading the
>>> aliases property, only DT nodes names are compared and not the complete
>>> path. This causes a problem when there are two DT nodes with same name but
>>> have different paths.
>>>
>>> Fix it by comparing the phandles of DT nodes after the node names match.
>>>
>>> Signed-off-by: Aswath Govindraju <a-govindraju at ti.com>
>>> ---
>>>
>>> Resending this patch as it was held awaiting for moderator approval because
>>> patch was sent by non-member.
>>>
>>>  lib/fdtdec.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>>> index 2015907dee7d..9e1bfe0b519e 100644
>>> --- a/lib/fdtdec.c
>>> +++ b/lib/fdtdec.c
>>> @@ -478,6 +478,11 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>>>                 slash = strrchr(prop, '/');
>>>                 if (strcmp(slash + 1, find_name))
>>>                         continue;
>>> +
>>> +               if (fdt_get_phandle(blob, offset) !=
>>> +                   fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
>>> +                       continue;
>>
>> The call to fdt_path_offset() is very slow. Perhaps we can do this
>> check only with livetree? What situation is causing a problem for you?
>> What are the node / alias names?
> 
> In the case of live tree for getting the sequence number the node
> pointers are compared. So, I don't think this problem would come up.
> 
> As for the use case,
> 
> In AM654 Device tree there are two instances of USB controllers and both
> the controller nodes have the same name usb at 10000
> 
> If dfu is performed through the port connected to second controller.
> Then based on the dr_mode of first controller the instance number to be
> used in dfu command will vary. In order to make the instance number for
> dfu command to be independent, aliases can be used(If aliases are
> defined then the sequence number is assigned as the alias number.).
> 
> The problem with current method for acquiring sequence number using
> aliases is that only the name of the node is compared with node name
> from the aliases property. So in the above case both the controllers
> will have the same name. This leads to the first alias number being used
> for the both the controllers to assign sequence number.
> 
> 
> aliases {
>                 serial2 = &main_uart0;
>                 ethernet0 = &cpsw_port1;
>                 usb0 = &usb0;            // This property being used to
> 					//alias both the controllers
>                 usb1 = &usb1;
>         };


To explain a bit more, here is the DT snippet around usb0 and usb1

        dwc3_0: dwc3 at 4000000 {
                compatible = "ti,am654-dwc3";
                reg = <0x0 0x4000000 0x0 0x4000>;
                #address-cells = <1>;
                #size-cells = <1>;
                ranges = <0x0 0x0 0x4000000 0x20000>;
                ...

                usb0: usb at 10000 {
                        compatible = "snps,dwc3";
                        reg = <0x10000 0x10000>;
                        ...
		};
	};

        dwc3_1: dwc3 at 4020000 {
                compatible = "ti,am654-dwc3";
                reg = <0x0 0x4020000 0x0 0x4000>;
                #address-cells = <1>;
                #size-cells = <1>;
                ranges = <0x0 0x0 0x4020000 0x20000>;
		...

		usb1: usb at 10000 {
                        compatible = "snps,dwc3";
                        reg = <0x10000 0x10000>;
			...
		};
	};

In above case, (with CONFIG_OF_LIVE disabled),
fdtdec_get_alias_seq() fails to pick the correct instance for USB
controller for a given index. This is because fdtdec_get_alias_seq()
only compares the leaf node name (usb at 10000) with alias path and thus
both usb instances match to usb0.

> 
> So, to distinguish nodes with same name, phandles can be used while
> assigning sequence numbers.
> 
> Thanks,
> Aswath
> 
>>
>>> +
>>>                 val = trailing_strtol(name);
>>>                 if (val != -1) {
>>>                         *seqp = val;
>>> --
>>> 2.17.1
>>>
>>
>> Regards,
>> Simon
>>
>