Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi Pantelis,

On 5/26/2016 6:49 AM, Rob Herring wrote:
> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> Hi David,
>>
>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6rL0yjEaa7Ru@public.gmane.orgau> wrote:
>>>
>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>>> Hi David,
>>>>
>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6vUpdFzICT1y@public.gmane.orgd.au> wrote:
>>>>>
>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>>> Hi David,
>>>>>>
>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnQMXKaNCjofrA@public.gmane.org.id.au> wrote:
>>>>>>>
>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>>> Hi Frank,
>>>>>>>>
>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>>> plugins and options.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>>> ---
>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++

< snip >

>>>>>>>>>> +So the bar peripheral's DTS format would be of the form:
>>>>>>>>>> +
>>>>>>>>>> +/dts-v1/ /plugin/;    /* allow undefined references and record them */
>>>>>>>>>> +/ {
>>>>>>>>>> +      ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>>> +      fragment@0 {
>>>>>>>>>> +              target = <&ocp>;
>>>>>>>>>> +              __overlay__ {
>>>>>>>>>> +                      /* bar peripheral */
>>>>>>>>>> +                      bar {
>>>>>>>>>> +                              compatible = "corp,bar";
>>>>>>>>>> +                              ... /* various properties and child nodes */
>>>>>>>>>> +                      }
>>>>>>>>>
>>>>>>>>>                     };
>>>>>>>>>
>>>>>>>>>> +              };
>>>>>>>>>> +      };
>>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>>> extra value they provide.
>>>>>>>>>
>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>>> was used for the target node):
>>>>>>>>>
>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>>> +/ {
>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>> +       ocp {
>>>>>>>>> +                       /* bar peripheral */
>>>>>>>>> +                       bar {
>>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>>> +                       };
>>>>>>>>> +       };
>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>
>>>>>>>> No.
>>>>>>>>
>>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>>
>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>>> platform.
>>>>>>>
>>>>>>> Huh?  How so..
>>>>>>>
>>>>>>
>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>>
>>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>>
>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>>> applied on multiple platforms.
>>>>>
>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>>> contains exactly the same information in a simpler encoding.
>>>>>
>>>>
>>>> It breaks it because it’s missing the target property.
>>>>
>>>> The layout of the base tree is not going to be the same in different
>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>>> instance.
>>>
>>> I think you're misinterpreting Frank's suggestion.  As I understand it
>>> the node names of the top level nodes in his format aren't treated as
>>> literal node names, but instead treated as label names which are
>>> resolved similarly to the phandle external fixups.
>>>
>>> Actually.. that is one serious problem with Frank's format, it doesn't
>>> (easily) allow multiple fragments to be applied to the same target.
>>>
>>
>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
>> I queued with multiple target support.

OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
email from 4/27 (just to provide an easy link for everyone).  I'm still
trying to figure that out.

So other than that, am I missing something else about what extra
functionality the extra layers of nodes provides?


> Queued implies accepted which they are not. The multiple ways of
> expressing targets bothers me. Upstream still has no external
> interface to overlays, so I think there is still room to change things
> if we decide it is worthwhile. Better now than stuck with something
> forever.
> 
> I too was wondering about the current syntax before this thread
> started. We have 2 levels of nodes before we get to any useful
> information with the current syntax.
> 
> Rob
>