On Fri, May 27, 2016 at 05:52:41PM +0300, Pantelis Antoniou wrote:
> 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).
I can't quite make sense of that comment. No matter the encoding,
you're always going to need some sort of a loader.
> > 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.
Yeah. I'm not entirely sure I'm convinced by the specific examples
given so far. However, in general I can see the value in providing a
way we can extend to add more metadata. The two level structure with
__overlay__ gives us that, whereas the one level approach doesn't.
> > 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
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
