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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Frank,

> On May 27, 2016, at 00:31 , Frank Rowand <frowand.list@xxxxxxxxx> wrote:
> 
> On 5/26/2016 10:09 AM, Pantelis Antoniou wrote:
>> Hi Frank,
>> 
>>> On May 26, 2016, at 19:55 , Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>>> 
>>> 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@xxxxxxxxxxxx> wrote:
>>>>> Hi David,
>>>>> 
>>>>>> On May 26, 2016, at 10:12 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxx>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> 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?
>>> 
>> 
>> No, I’m talking about the new target options patchset.
>> 
>> "of: overlays: New target methods” & in particular
>> 
>> "of: overlay: Implement target index support"
> 
> Thanks for the pointer.  I don't think the target options approach
> is the way to handle the issue (see my reply a couple of minutes
> ago in that thread).
> 
>> 
>>> 
>>>> 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.
>>>> 
>> 
>> That’s on purpose. The first level is to contain load manager specific details,
>> i.e. part-numbers and other platform specific properties.
>> 
>> The second level contains the per fragment properties (at the moment targets).
> 
> My suggestion removed the target property.
> 

Removing the target property requires using a loader. That may be fine
in general and in fact that’s what the target root methods do (setting
the target root to ‘/‘ makes them equivalent to your version).

> What other properties are you envisioning?  (Looking for the architectural
> vision that you have.)
> 

Oh, there are a lot of properties that can be provided.

For instance you can declare manufacturing info (like part numbers, version numbers,
serial numbers that can be used for quirking). You can declare things like load order
when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
should be disabled when an add on cape with display capability is attached).
You can declare resources (i.e. pins or power draw figures) to make a decision
whether enabling an expansion board is safe.

I’m sure more ideas will come when we put it into wide-spread use.  

> If load manager specific details are appropriate in the devicetree (a whole
> different discussion) then maybe a /chosen/load-manager node could exist to
> hold them instead of putting them in /, where the patch currently locates
> "/* various properties for loader use; i.e. part id etc. */".

Oh yes. But that’s something for us to figure out.

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux