All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
       [not found] <20201116142950.28919-1-a-govindraju@ti.com>
@ 2020-11-18 14:37 ` Simon Glass
  2020-11-18 15:14   ` [EXTERNAL] " Aswath Govindraju
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-11-18 14:37 UTC (permalink / raw)
  To: u-boot

Hi Aswath,

On Mon, 16 Nov 2020 at 07:29, Aswath Govindraju <a-govindraju@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@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?

> +
>                 val = trailing_strtol(name);
>                 if (val != -1) {
>                         *seqp = val;
> --
> 2.17.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [EXTERNAL] Re: [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
  2020-11-18 14:37 ` [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name Simon Glass
@ 2020-11-18 15:14   ` Aswath Govindraju
  2020-11-18 16:55     ` Vignesh Raghavendra
  0 siblings, 1 reply; 6+ messages in thread
From: Aswath Govindraju @ 2020-11-18 15:14 UTC (permalink / raw)
  To: u-boot

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@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@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;
        };

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
  2020-11-18 15:14   ` [EXTERNAL] " Aswath Govindraju
@ 2020-11-18 16:55     ` Vignesh Raghavendra
  2020-11-21 23:07       ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2020-11-18 16:55 UTC (permalink / raw)
  To: u-boot



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@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@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
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
  2020-11-18 16:55     ` Vignesh Raghavendra
@ 2020-11-21 23:07       ` Simon Glass
  2020-11-27 14:05         ` Aswath Govindraju
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-11-21 23:07 UTC (permalink / raw)
  To: u-boot

Hi,

On Wed, 18 Nov 2020 at 10:55, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
>
>
> 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@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@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.

Thank you both for the detai. I understand it and in fact I think this
has come up before.

Would it be OK to use livetree?

If not, can we put this code behind a Kconfig so the extra time
penalty is only incurred on boards that need it?

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
  2020-11-21 23:07       ` Simon Glass
@ 2020-11-27 14:05         ` Aswath Govindraju
  2020-11-30 20:11           ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Aswath Govindraju @ 2020-11-27 14:05 UTC (permalink / raw)
  To: u-boot

On 22/11/20 4:37 am, Simon Glass wrote:
> Hi,
> 
> On Wed, 18 Nov 2020 at 10:55, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>>
>>
>>
>> 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@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@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.
> 

I apologize for the delay in response.


> Thank you both for the detai. I understand it and in fact I think this
> has come up before.
> 
> Would it be OK to use livetree?
> 
Currently live tree has not been enabled in the configurations of the
AM65 board and there are some issues that I am facing after enabling it.

> If not, can we put this code behind a Kconfig so the extra time
> penalty is only incurred on boards that need it?
> 

I think putting it under Kconfig is a good idea and I think I will do that.

Also, I have alternate method to implement the comparison that does not
use fdt_path_offset(), compares by getting the path name. I have
attached it to this mail. I think it is slightly better in terms of time
penalty. Can you please look at it and suggest if it is better to implement.

Thanks,
Aswath


> Regards,
> Simon
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Check-the-complete-path-after-node-names-match.patch
Type: text/x-patch
Size: 1829 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201127/9ec1abd2/attachment.bin>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name
  2020-11-27 14:05         ` Aswath Govindraju
@ 2020-11-30 20:11           ` Simon Glass
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2020-11-30 20:11 UTC (permalink / raw)
  To: u-boot

Hi Aswath,

On Fri, 27 Nov 2020 at 07:05, Aswath Govindraju <a-govindraju@ti.com> wrote:
>
> On 22/11/20 4:37 am, Simon Glass wrote:
> > Hi,
> >
> > On Wed, 18 Nov 2020 at 10:55, Vignesh Raghavendra <vigneshr@ti.com> wrote:
> >>
> >>
> >>
> >> 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@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@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.
> >
>
> I apologize for the delay in response.

No hurry!

>
>
> > Thank you both for the detai. I understand it and in fact I think this
> > has come up before.
> >
> > Would it be OK to use livetree?
> >
> Currently live tree has not been enabled in the configurations of the
> AM65 board and there are some issues that I am facing after enabling it.

OK. I think it would be a good idea to figure that out. It should be a
case of making sure all drivers are converted.

>
> > If not, can we put this code behind a Kconfig so the extra time
> > penalty is only incurred on boards that need it?
> >
>
> I think putting it under Kconfig is a good idea and I think I will do that.

OK.

>
> Also, I have alternate method to implement the comparison that does not
> use fdt_path_offset(), compares by getting the path name. I have
> attached it to this mail. I think it is slightly better in terms of time
> penalty. Can you please look at it and suggest if it is better to implement.

Unfortunately fdt_parent_offset() has the same problem since it needs
to scan the tree again to find the parent.

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-30 20:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20201116142950.28919-1-a-govindraju@ti.com>
2020-11-18 14:37 ` [RESEND PATCH] fdt: Use phandle to distinguish DT nodes with same name Simon Glass
2020-11-18 15:14   ` [EXTERNAL] " Aswath Govindraju
2020-11-18 16:55     ` Vignesh Raghavendra
2020-11-21 23:07       ` Simon Glass
2020-11-27 14:05         ` Aswath Govindraju
2020-11-30 20:11           ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.