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.
What other properties are you envisioning? (Looking for the architectural
vision that you have.)
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. */".
--
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
