patch: more path support

[ron minnich]

This patch adds more path support to coreboot and to the dtc.

This support in turn reduces annoying warning messages as coreboot comes up.

ron

[Peter Stuge]

On Sat, Feb 09, 2008 at 11:03:47AM -0800, ron minnich wrote:
> How should this really be done? I'd be happier (much) if we could
> just say "path" in any given node and somehow have Magic
> figure out what the path type was. This is really easy to fix
> as soon as some smart person figures it out, so I think we should
> move forward with these changes until we get it. Plus, we may never
> figure it out.

Dunno. This is taking a really easy way out. :p

It should be simple enough to get the type if struct property had
a struct node *node; member so that the owner->parent node could
be reached from within the property foreach loop.

//Peter

[ron minnich]

On Feb 9, 2008 9:21 PM, Peter Stuge  wrote:
> It should be simple enough to get the type if struct property had
> a struct node *node; member so that the owner->parent node could
> be reached from within the property foreach loop.

it's there. but that doesn't totally tell you what type of resource
you have. I don't think.

Anyway, if we don't get a better idea on this path stuff in the next
while, I'd like to just commit what I've got.

ron

[Peter Stuge]

On Sat, Feb 09, 2008 at 09:35:49PM -0800, ron minnich wrote:
> > It should be simple enough to get the type if struct property had
> > a struct node *node; member so that the owner->parent node could
> > be reached from within the property foreach loop.
>
> it's there.

Hmm. How?
> but that doesn't totally tell you what type of resource you have. I
> don't think.

Maybe I'm oversimplifying? I was thinking that:
(from your patch)

cpus {
+               cpupath="0";

apic {
                /config/("northbridge/amd/geodelx/apic");
+               apicpath="0,0,0";

superio {
                        /config/("superio/winbond/w83627hf/dts");
+                       superiopath = "0x2e";

..it would be possible to determine resource type from the name of
the containing block. "cpus" => cpupath, "apic" => apicpath etc.

//Peter

[Stefan Reinauer]

Peter Stuge wrote:
> On Sat, Feb 09, 2008 at 09:35:49PM -0800, ron minnich wrote:
> > > It should be simple enough to get the type if struct property had
> > > a struct node *node; member so that the owner->parent node could
> > > be reached from within the property foreach loop.
> >
> > it's there.
>
> Hmm. How?
> > but that doesn't totally tell you what type of resource you have. I
> > don't think.
>
> Maybe I'm oversimplifying? I was thinking that:
> (from your patch)
>     cpus {
>           cpupath="0";
>
>   apic {
>           /config/("northbridge/amd/geodelx/apic");
>           apicpath="0,0,0";
>
>           superio {
>                   /config/("superio/winbond/w83627hf/dts");
>                   superiopath = "0x2e";
>
>
> ..it would be possible to determine resource type from the name of
> the containing block. "cpus" => cpupath, "apic" => apicpath etc.

Ron suggested to make this cpu@0 { ... } and apic@0,0,0 { ... }

I like that idea a lot.

I think that would be the way to go.. It means we can not call the apic 
node "my_weird_apic" anymore, which is an improvement, too.

Stefan

[ron minnich]

On Feb 10, 2008 8:27 AM, Stefan Reinauer  wrote:
> Ron suggested to make this cpu@0 { ... } and apic@0,0,0 { ... }
>
> I like that idea a lot.
>
> I think that would be the way to go.. It means we can not call the apic
> node "my_weird_apic" anymore, which is an improvement, too.

I'll work up a patch for this. So look for the weird pcipath etc.
stuff to go away, and we can use the dts
more sensibly.

Can somebody think about the problem of the ID? How do we name the ID
of something? In v2 we punted and made everything a
device, vendor tuple --- i.e. it was all PCI ids.

thanks

ron

[Peter Stuge]

On Sun, Feb 10, 2008 at 08:35:18AM -0800, ron minnich wrote:
> > Ron suggested to make this cpu@0 { ... } and apic@0,0,0 { ... }
> >
> > I like that idea a lot.

Are there enough common cases to make @<path> optional?

For superio it's required.

For cpu and apic on LX boards not so much.

For at least cpu on n-way K8 boards it will be useful again.

The <path> part has to accomodate also complex links and buses.
I guess the patch is already resource-type specific. (acpi has PCI,
superio has ioport..)
> I'll work up a patch for this. So look for the weird pcipath etc.
> stuff to go away, and we can use the dts more sensibly.

Cool.
> Can somebody think about the problem of the ID? How do we name the
> ID of something? In v2 we punted and made everything a
> device, vendor tuple --- i.e. it was all PCI ids.

What would such a global identifier be used for? One way to identify
something is by position in the dt, but you're asking for a new
"namespace" right?

//Peter

[ron minnich]

On Feb 10, 2008 8:42 AM, Peter Stuge  wrote:
> What would such a global identifier be used for? One way to identify
> something is by position in the dt, but you're asking for a new
> "namespace" right?

You have found a  type of a thing (e.g. southbridge) and you want to
find the operations for that thing. You need an ID to do that.

One option is we just adopt a very standardized format for the string
that defines a thing, e.g. your earlier suggestion:
"PCI:8086.8259" is the standard string for a PCI identifier.

ron

[Peter Stuge]

On Sun, Feb 10, 2008 at 10:17:01AM -0800, ron minnich wrote:
> > What would such a global identifier be used for?
>
> You have found a  type of a thing (e.g. southbridge) and you want
> to find the operations for that thing. You need an ID to do that.

Ah! Of course.
> One option is we just adopt a very standardized format for the
> string that defines a thing, e.g. your earlier suggestion:
> "PCI:8086.8259" is the standard string for a PCI identifier.

Hm. Would the coreboot source tree path work?

"southbridge/intel/i82371eb"

Too complicated to deal with in code maybe?

The compile time name and the PCI device ID are the only two things
uniquely identifying a component right now.

Should we add a name to struct constructor for this purpose?
Maybe it can be automatically defined by the build system using the
directory name. Does that still work with the LX since we consider
it a multifunction device?

//Peter

[ron minnich]

On Feb 10, 2008 12:08 PM, Peter Stuge  wrote:
> "southbridge/intel/i82371eb"

That doesn't help. PCI gives you two 16-bit numbers. From those two
16-bit numbers, you have to find a constructor.

The current ID system, for all its flaws, does this job very well.

ron

[Peter Stuge]

On Sun, Feb 10, 2008 at 01:35:01PM -0800, ron minnich wrote:
> > "southbridge/intel/i82371eb"
>
> That doesn't help. PCI gives you two 16-bit numbers. From those two
> 16-bit numbers, you have to find a constructor.

Doh. We have to use the PCI ids then.

//Peter

[ron minnich]

On Feb 10, 2008 9:32 PM, Peter Stuge  wrote:
> On Sun, Feb 10, 2008 at 01:35:01PM -0800, ron minnich wrote:
> > > "southbridge/intel/i82371eb"
> >
> > That doesn't help. PCI gives you two 16-bit numbers. From those two
> > 16-bit numbers, you have to find a constructor.
>
> Doh. We have to use the PCI ids then.

Right, but there is no rule that says we can't take those two PCI IDs
and convert them to a string: "PCI:xxxx,xxxx" and match that string.

So we have to use those PCI ids but can transform them if needed. I do
that now i the code, but not to a string.

ron

[Carl-Daniel Hailfinger]

On 11.02.2008 14:05, ron minnich wrote:
> On Feb 10, 2008 9:32 PM, Peter Stuge  wrote:
> > On Sun, Feb 10, 2008 at 01:35:01PM -0800, ron minnich wrote:
> > > > "southbridge/intel/i82371eb"
> > >
> > > That doesn't help. PCI gives you two 16-bit numbers. From those two
> > > 16-bit numbers, you have to find a constructor.
> >
> > Doh. We have to use the PCI ids then.
>
> Right, but there is no rule that says we can't take those two PCI IDs
> and convert them to a string: "PCI:xxxx,xxxx" and match that string.
>
> So we have to use those PCI ids but can transform them if needed. I do
> that now i the code, but not to a string.

How do you handle the case where two identical PCI devices need
different settings?

Regards,
Carl-Daniel

[ron minnich]

On Feb 11, 2008 5:46 AM, Carl-Daniel Hailfinger
 wrote:
> How do you handle the case where two identical PCI devices need
> different settings?

That's actually very easy. The patch differentiates each device, while
the id identifies what kind of device they are.

I have the patch ready and tested. Here is how the qemu dts now looks.
This boots linux fine. If this looks OK I will submit the patch.

ron
/{
        mainboard-vendor = "Emulation";
        mainboard-name = "QEMU x86";
        enabled;
        constructor = "qemuvga_constructors";
        cpus {
                enabled;
        };
        domain@0 {
                /config/("northbridge/intel/i440bxemulation/dts");
                enabled;
                bus@0 {
                        pci@0,0 {
                                enabled;
                        };
                        pci@1,0 {
                                /config/("southbridge/intel/i82371eb/dts");
                                enabled;
                        };
                };
        };
};

[Carl-Daniel Hailfinger]

On 11.02.2008 22:26, ron minnich wrote:
> On Feb 11, 2008 5:46 AM, Carl-Daniel Hailfinger
>  wrote:
> > How do you handle the case where two identical PCI devices need
> > different settings?
>
> That's actually very easy. The patch differentiates each device, while
> the id identifies what kind of device they are.

OK.
> I have the patch ready and tested. Here is how the qemu dts now looks.
> This boots linux fine. If this looks OK I will submit the patch.

I like the new structure, but there is one thing that irritates me to no
end: "@"
When I first read the dts without the accompanying discussion, I
completely misunderstood the structure because of the "@". I thought the
"@" really meant "at" in the sense that it refers to the address of the
parent entity, e.g "pci@0,0" means "the pci device at bus 0,0". However,
the true meaning/translation of "@" seems to be "device with
address/number", e.g. "pci@0,0" means "pci device with address 0,0".
Can we please have another separator like "-" or ":" or "_"? All of
those alternative separators convey the meaning better.

A patch with a changed separator would be really appreciated.
> ron
> /{
>         mainboard-vendor = "Emulation";
>         mainboard-name = "QEMU x86";
>         enabled;
>         constructor = "qemuvga_constructors";
>         cpus {
>                 enabled;
>         };
>         domain@0 {
>                 /config/("northbridge/intel/i440bxemulation/dts");
>                 enabled;
>                 bus@0 {
>                         pci@0,0 {
>                                 enabled;
>                         };
>                         pci@1,0 {
>                                 /config/("southbridge/intel/i82371eb/dts");
>                                 enabled;
>                         };
>                 };
>         };
> };

Regards,
Carl-Daniel

[ron minnich]

On Feb 11, 2008 4:00 PM, Carl-Daniel Hailfinger
 wrote:
> I like the new structure, but there is one thing that irritates me to no
> end: "@"
> When I first read the dts without the accompanying discussion, I
> completely misunderstood the structure because of the "@". I thought the
> "@" really meant "at" in the sense that it refers to the address of the
> parent entity, e.g "pci@0,0" means "the pci device at bus 0,0". However,
> the true meaning/translation of "@" seems to be "device with
> address/number", e.g. "pci@0,0" means "pci device with address 0,0".
> Can we please have another separator like "-" or ":" or "_"? All of
> those alternative separators convey the meaning better.

I actually like @. It's part of the standard. Anyone else care to comment here?

Here is the latest: enabled is assumed, and 'disabled' will disable it.

ron
/{
        mainboard-vendor = "Emulation";
        mainboard-name = "QEMU x86";
        enabled;
        constructor = "qemuvga_constructors";
        cpus {};
        domain@0 {
                /config/("northbridge/intel/i440bxemulation/dts");
                bus@0 {
                        pci@0,0 {
                        };
                        pci@1,0 {
                                /config/("southbridge/intel/i82371eb/dts");
                        };
                };
        };
};

[Carl-Daniel Hailfinger]

On 12.02.2008 01:14, ron minnich wrote:
> On Feb 11, 2008 4:00 PM, Carl-Daniel Hailfinger
>  wrote:
> > I like the new structure, but there is one thing that irritates me to no
> > end: "@"
> > When I first read the dts without the accompanying discussion, I
> > completely misunderstood the structure because of the "@". I thought the
> > "@" really meant "at" in the sense that it refers to the address of the
> > parent entity, e.g "pci@0,0" means "the pci device at bus 0,0". However,
> > the true meaning/translation of "@" seems to be "device with
> > address/number", e.g. "pci@0,0" means "pci device with address 0,0".
> > Can we please have another separator like "-" or ":" or "_"? All of
> > those alternative separators convey the meaning better.
>
> I actually like @. It's part of the standard. Anyone else care to comment here?

Could you write something about the standard and our syntax in the v3
design document? That way, the knowledge will not be lost.
> Here is the latest: enabled is assumed, and 'disabled' will disable it.
>
> ron
> /{
>         mainboard-vendor = "Emulation";
>         mainboard-name = "QEMU x86";
>         enabled;
>         constructor = "qemuvga_constructors";
>         cpus {};
>         domain@0 {
>                 /config/("northbridge/intel/i440bxemulation/dts");
>                 bus@0 {
>                         pci@0,0 {
>                         };
>                         pci@1,0 {
>                                 /config/("southbridge/intel/i82371eb/dts");
>                         };
>                 };
>         };
> };

Very nice. The structure is pretty clean and short.

Regards,
Carl-Daniel

[ron minnich]

here is the proposed patch.

Comments welcome.

ron

[Peter Stuge]

On Mon, Feb 11, 2008 at 08:03:11PM -0800, ron minnich wrote:
> So, instead of what we had before, a pathname property in 
> a node, e.g.
> pcipath = "0xf, 1";
> Now you label the node with the path, e.g.
> pci@0xf,1
>
> This results in a far cleaner and simpler dts.

I still think it is very confusing.

The path describes where within the parent node space this new node
is located, but the path is written "within" the new node.
(No, not within {} but after the node is created.)

This is confusing because the rest of the dts is hierarchical (good!)
(ordered top-down) and here the order is suddenly backwards;
information related to a higher level comes later in the file.
> +struct superio_path
> +{
> unsigned iobase;
>
> +};

I would also like to indicate the path type in the path somehow, to
avoid confusion. This will also allow higher resolution syntax
checking of dts files.

Maybe:

devfn:0,0 pci {}

io:2e superio {}

?

The paths are still somewhat confusing. It is not clear how to reach
the new node. devfn is particularly confusing since it might be
expected to only exist _within_ the PCI.
> +++ mainboard/emulation/qemu-x86/dts	(working copy)
> @@ -23,22 +23,15 @@
>    mainboard-name = "QEMU x86";
>    enabled;

Is this enable still required?
> domain@0 {
> /config/("northbridge/intel/i440bxemulation/dts");
> bus@0 {
> 	pci@0,0 {
> 	};
> 	pci@1,0 {
> 		/config/("southbridge/intel/i82371eb/dts");
> 	};
> };
> };
>
> };

I like how this is turning out. Just that path thing..
> +++ northbridge/intel/i440bxemulation/dts	(working copy)
> @@ -21,4 +21,5 @@
>  {
>    ramsize = "128";
>    constructor = "i440bx_constructors";
> domainid = "0x8086, 0x7190";

This is the id of this component within a domain, or does it mean
that this component creates a new domain at that PCI id?

I really want to avoid this ambiguity.
> path = index(tree->name, '@');
> if (path && path[1]) {
> path++;
> if (!strncmp(tree->name, "cpu", 3)){

I would prefer comparing (path-tree->name) bytes instead of 3 to make
sure we can have both "bus" and "busmaster" nodes. (stupid example,
but you get the idea)
> if (!strncmp(tree->name, "lpc", 3)){
> 	fprintf(f, "\t.path = {.type=DEVICE_PATH_SUPERIO,.u={.superio={.iobase=%s}}},\n", 

Now this one isn't very clean. Which is it? lpc or superio? I don't
think all superios are on LPC.

The cleanup and the enabled/disabled change are great!

//Peter

[ron minnich]

let's get the dts right before I submit any more code. I want us all
to be happy with this.

Let's start with this dts:
/{
        mainboard-vendor = "Emulation";
        mainboard-name = "QEMU x86";
        enabled;
        constructor = "qemuvga_constructors";
        cpus {};
        domain@0 {
                /config/("northbridge/intel/i440bxemulation/dts");
                bus@0 {
                        pci@0,0 {
                        };
                        pci@1,0 {
                                /config/("southbridge/intel/i82371eb/dts");
                        };
                };
        };
};

how do you want it to look? Recall, however, that the x@y syntax is
well understood by various OFW tools; changes to this should be
carefully done.

Thanks for comments and patience!

ron

[Peter Stuge]

On Tue, Feb 12, 2008 at 06:17:57AM -0800, ron minnich wrote:
> let's get the dts right before I submit any more code. I want us
> all to be happy with this.

Cool.
> Let's start with this dts:
> /{
>         mainboard-vendor = "Emulation";
>         mainboard-name = "QEMU x86";
>         enabled;

Again, is this enabled still needed?
>     constructor = "qemuvga_constructors";
>     cpus {};
>     domain@0 {
>             /config/("northbridge/intel/i440bxemulation/dts");
>             bus@0 {
>                     pci@0,0 {
>                     };
>                     pci@1,0 {
>                             /config/("southbridge/intel/i82371eb/dts");
>                     };
>             };
>     };
>
> };
>
> how do you want it to look?

I'm very happy with the above except for the x@y syntax.
> Recall, however, that the x@y syntax is well understood by various
> OFW tools; changes to this should be carefully done.

Yeah, I thought to myself before that it's very unfortunate.

I don't even understand all the path types. What are they for the
different node types?

//Peter

[ron minnich]

On Feb 12, 2008 11:13 AM, Peter Stuge  wrote:
> >     mainboard-vendor = "Emulation";
> >     mainboard-name = "QEMU x86";
> >     enabled;
>
> Again, is this enabled still needed?

no, it is the default, I keep forgetting to yank it. Fixed.
> >     constructor = "qemuvga_constructors";
> >     cpus {};
> >     domain@0 {

domain@0 --> domain 0
> >             /config/("northbridge/intel/i440bxemulation/dts");
> >             bus@0 {

bus@0 --> bus 0
> >                     pci@0,0 {

pci@0,0 --> dev 0 function 1
> >                     };
> >                     pci@1,0 {

pci@1,0 --> dev 1 function 0
> I'm very happy with the above except for the x@y syntax.

I'm happy to see an alternate, although I don't have a problem with
the @ myself. I'm used to it from Suns. But propose something
reasonable that won't break the parser and we can try it.
> I don't even understand all the path types. What are they for the
> different node types?

see comments above, do they make sense?

ron

[Corey Osgood]

On Feb 12, 2008 3:15 PM, ron minnich  wrote:
> On Feb 12, 2008 11:13 AM, Peter Stuge  wrote:
> > >     mainboard-vendor = "Emulation";
> > >     mainboard-name = "QEMU x86";

Can we possibly pull this info (or defaults for this info) from Kconfig?
> > >     enabled;
> >
> > Again, is this enabled still needed?
>
> no, it is the default, I keep forgetting to yank it. Fixed.
> > >     constructor = "qemuvga_constructors";
> > >     cpus {};
> > >     domain@0 {
>
> domain@0 --> domain 0
> > >             /config/("northbridge/intel/i440bxemulation/dts");
> > >             bus@0 {
>
> bus@0 --> bus 0
> > >                     pci@0,0 {
>
> pci@0,0 --> dev 0 function 1
> > >                     };
> > >                     pci@1,0 {
>
> pci@1,0 --> dev 1 function 0

why shorten device to dev and not fuction to func? is it possible to set it
up so people/developers can use either dev or device?
> I'm very happy with the above except for the x@y syntax.
>
> I'm happy to see an alternate, although I don't have a problem with
> the @ myself. I'm used to it from Suns. But propose something
> reasonable that won't break the parser and we can try it.

I don't see the problem either, except that it looks ugly (IMO). I'm fine
with it either way.
> > I don't even understand all the path types. What are they for the
> > different node types?
>
> see comments above, do they make sense?
>
> ron

-Corey

[ron minnich]

On Feb 12, 2008 3:02 PM, Corey Osgood  wrote:
> On Feb 12, 2008 3:15 PM, ron minnich  wrote:
> > On Feb 12, 2008 11:13 AM, Peter Stuge  wrote:
> > > >     mainboard-vendor = "Emulation";
> > > >     mainboard-name = "QEMU x86";

Sure, I am fine with that. Comments?
> > > >                     pci@1,0 {
> >
> > pci@1,0 --> dev 1 function 0
>
> why shorten device to dev and not fuction to func? is it possible to set it
> up so people/developers can use either dev or device?

I confused you. This line
> pci@1,0 --> dev 1 function 0

is an explanatory comment. It is not in the dts. Here is the dts again.

/{
        mainboard-vendor = "Emulation";
        mainboard-name = "QEMU x86";
        constructor = "qemuvga_constructors";
        cpus {};
        domain@0 {
                /config/("northbridge/intel/i440bxemulation/dts");
                bus@0 {
                        pci@0,0 {
                        };
                        pci@1,0 {
                                /config/("southbridge/intel/i82371eb/dts");
                        };
                };
        };
};

That's the whole file.

Let's try to bring this to closure. The score so far.

Stepan: is ok with '@' syntax (as are segher etc. )
Ron: is not that concerned either way as long as an alternative
carl-daniel, peter, corey: do not like @ syntax but need to propose a
reasonable alternative (they have tried but not totally there yet)
which does not make the parser hard to modify.

ron

[Carl-Daniel Hailfinger]

On 13.02.2008 04:58, ron minnich wrote:
> On Feb 12, 2008 3:02 PM, Corey Osgood  wrote:
> > On Feb 12, 2008 3:15 PM, ron minnich  wrote:
> > > On Feb 12, 2008 11:13 AM, Peter Stuge  wrote:
> > > > >     mainboard-vendor = "Emulation";
> > > > >     mainboard-name = "QEMU x86";
> >
> > Can we possibly pull this info (or defaults for this info) from Kconfig?
>
> Sure, I am fine with that. Comments?

Why not? But let's do that step in another patch.
> Here is the dts again.
>
> /{
>         mainboard-vendor = "Emulation";
>         mainboard-name = "QEMU x86";
>         constructor = "qemuvga_constructors";
>         cpus {};
>         domain@0 {
>                 /config/("northbridge/intel/i440bxemulation/dts");
>                 bus@0 {
>                         pci@0,0 {
>                         };
>                         pci@1,0 {
>                                 /config/("southbridge/intel/i82371eb/dts");
>                         };
>                 };
>         };
> };
>
> That's the whole file.
>
> Let's try to bring this to closure. The score so far.
>
> Stepan: is ok with '@' syntax (as are segher etc. )
> Ron: is not that concerned either way as long as an alternative
> carl-daniel, peter, corey: do not like @ syntax but need to propose a
> reasonable alternative (they have tried but not totally there yet)
> which does not make the parser hard to modify.

Alternatives are "-" (used by memreserve and inside names) or ":" (used
by memreg) or "_" (used inside names). I hope at least one of them is
considered as reasonable alternative.

Regards,
Carl-Daniel

[ron minnich]

On Feb 13, 2008 9:05 AM, Carl-Daniel Hailfinger
 wrote:
> Alternatives are "-" (used by memreserve and inside names) or ":" (used
> by memreg) or "_" (used inside names). I hope at least one of them is
> considered as reasonable alternative.

@ in this case seperates the type of resources (pci) from the instance
of it (device, function).

So @ is a seperator. Using _ will add ambiguity as it is NOT a
seperator. same problem for -.

Stepan, will the OFW guys kill us if we allow ':' as well as @.

I still am not sure that allowing @ AND : is really that good an idea
-- it means some dts files will not look like others; and if we put
the dts binary blob in the lar, tools that translate the blob to
source (such as the dtc itself) will print @, not :.

I'm happy to allow ':' but be aware it's not problem-free.

thanks

ron

[Carl-Daniel Hailfinger]

On 13.02.2008 18:11, ron minnich wrote:
> On Feb 13, 2008 9:05 AM, Carl-Daniel Hailfinger
>  wrote:
> > Alternatives are "-" (used by memreserve and inside names) or ":" (used
> > by memreg) or "_" (used inside names). I hope at least one of them is
> > considered as reasonable alternative.
>
> @ in this case seperates the type of resources (pci) from the instance
> of it (device, function).
>
> So @ is a seperator. Using _ will add ambiguity as it is NOT a
> seperator. same problem for -.

OK.
> Stepan, will the OFW guys kill us if we allow ':' as well as @.
>
> I still am not sure that allowing @ AND : is really that good an idea
> -- it means some dts files will not look like others; and if we put
> the dts binary blob in the lar, tools that translate the blob to
> source (such as the dtc itself) will print @, not :.

In that case I'd retract my opposition to @, but only if the v3 design
doc gets an additional paragraph specifying the syntax with a hint to
potential misunderstandings with @.
> I'm happy to allow ':' but be aware it's not problem-free.

Thanks for giving an analysis of the separator problem.

Regards,
Carl-Daniel

[ron minnich]

On Feb 13, 2008 9:17 AM, Carl-Daniel Hailfinger
 wrote:
> In that case I'd retract my opposition to @, but only if the v3 design
> doc gets an additional paragraph specifying the syntax with a hint to
> potential misunderstandings with @.

OK, I'll wait on an ack and commit and then you'll see a patch to the document.

Then vcpi is next.

thanks

ron

[Carl-Daniel Hailfinger]

On 13.02.2008 18:18, ron minnich wrote:
> On Feb 13, 2008 9:17 AM, Carl-Daniel Hailfinger
>  wrote:
> > In that case I'd retract my opposition to @, but only if the v3 design
> > doc gets an additional paragraph specifying the syntax with a hint to
> > potential misunderstandings with @.
>
> OK, I'll wait on an ack and commit and then you'll see a patch to the document.

Acked-by:  Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[ron minnich]

Committed revision 593.

I want to once again thank you all for your thoughtful comments.

ron

[Segher Boessenkool]

> @ in this case seperates the type of resources (pci) from the instance
> of it (device, function).
>
> So @ is a seperator. Using _ will add ambiguity as it is NOT a
> seperator. same problem for -.
>
> Stepan, will the OFW guys kill us if we allow ':' as well as @.

The general syntax of a pathname component in OF is:

name@unit-addr:args

"name" is the name of the node, like "pci" or "cpu" or "rtc".

"unit-addr" is (the text representation of) the address for the node,
in the parent's address space.  On simple memory-mapped busses, you
would see for example "pci@f2000000"; on a pci bus, you see "ethernet@4"
for dev 4 fn 0, or "ethernet@4,1" for dev 4 fn 1 (pci bus number is
implicit here); and on simple enumerated (pseudo-)busses you can find
"cpu@2" for a cpu with id 2.

"args" is used when you open a device node; for example, on a storage
device it can select a partition and a file name on a filesystem on
there, or on a network device node it can tell you what ip address to
or to perform dhcp etc. etc.

The text representation of a unit address corresponds directly to the
binary representation of it, which typically comes from the "reg"
property in the device node.  In "real" OF, this mapping is done via
the encode-unit and decode-unit methods in the parent node, and the
text representation isn't actually stored in the (static) tree.

In the "flat tree" system however, the text representation of the unit
address is stored together with the node name as one string.  This
doesn't cause too many problems for consumers of the flat tree, but
_producers_ of flat trees run into more and more problems, that 
currently
are "solved" in various ad-hoc ways.

If you want to stay close to original OF with your use of the device 
tree
(I think you should, but it's entirely up to you), I can push for 
creating
a new revision of the flat tree format that will include a decent 
solution
to this problem once and for all.

Comments?  Questions?

Segher

[Stefan Reinauer]

Segher Boessenkool wrote:
> > @ in this case seperates the type of resources (pci) from the instance
> > of it (device, function).
> >
> > So @ is a seperator. Using _ will add ambiguity as it is NOT a
> > seperator. same problem for -.
> >
> > Stepan, will the OFW guys kill us if we allow ':' as well as @.

We are of course free to go away from the ePAPR flat device tree (we did 
already to some extent).

Though I wonder whether we should really do this though for something as 
cosmetic as the address seperator.
@ is "at", naturally describing a path or an address where something can 
be found. I am biased, I used this way of writing paths for 10 years.
> If you want to stay close to original OF with your use of the device 
> tree
> (I think you should, but it's entirely up to you), I can push for 
> creating
> a new revision of the flat tree format that will include a decent 
> solution
> to this problem once and for all.

I agree with Segher we should stay close to the "original". That's why 
we used dtc for our device tree creation to begin with. It has a 
recognition effect, making it easier for others to join in.
Also, at some point we're not writing the DTS manually anymore anyways, 
so it doesn't really matter ;-))

Stefan

[Segher Boessenkool]

> We are of course free to go away from the ePAPR flat device tree (we 
> did already to some extent).

I am not talking about the ePAPR flat tree.  I am talking about the
flat tree as used in PowerPC Linux for many years now.
> I agree with Segher we should stay close to the "original". That's why 
> we used dtc for our device tree creation to begin with. It has a 
> recognition effect, making it easier for others to join in.

It's also well-proven technology.  And lastly, wouldn't it be nice if
we could hand-off the DTB to the kernel at boot -- bye-bye legacy
interfaces.
> Also, at some point we're not writing the DTS manually anymore 
> anyways, so it doesn't really matter ;-))

You'll always have to write some of it by hand, in one form or another,
for some system-specific devices that cannot be probed or that you don't
want to probe for (PHBs, I/O bridges, ...)

Segher

[ron minnich]

Thanks for the notes. We did in the end resolve in favor of '@'.

ron

On Thu, Feb 14, 2008 at 2:01 PM, Segher Boessenkool
 wrote:
> > We are of course free to go away from the ePAPR flat device tree (we
> > did already to some extent).
>
> I am not talking about the ePAPR flat tree.  I am talking about the
>  flat tree as used in PowerPC Linux for many years now.
> > I agree with Segher we should stay close to the "original". That's why
> > we used dtc for our device tree creation to begin with. It has a
> > recognition effect, making it easier for others to join in.
>
> It's also well-proven technology.  And lastly, wouldn't it be nice if
>  we could hand-off the DTB to the kernel at boot -- bye-bye legacy
>  interfaces.
> > Also, at some point we're not writing the DTS manually anymore
> > anyways, so it doesn't really matter ;-))
>
> You'll always have to write some of it by hand, in one form or another,
>  for some system-specific devices that cannot be probed or that you don't
>  want to probe for (PHBs, I/O bridges, ...)
>
> Segher

[Peter Stuge]

On Thu, Feb 14, 2008 at 07:12:54PM +0100, Segher Boessenkool wrote:
> The general syntax of a pathname component in OF is:
>
> name@unit-addr:args

Thanks for the insight!
> "unit-addr" is (the text representation of) the address for the
> node, in the parent's address space.

This statement sounds so good. But the current suggestion is:

+++ mainboard/pcengines/alix1c/dts	(working copy)
@@ -22,26 +22,17 @@
+	apic@0 {

Enumerated "bus", just a list of devices. This is the first, addr 0.

+	domain@0 {

Another "device". Also addr 0. Can this really be right?

+		pci@1,0 {

Within domain 0 a PCI device devfn 1,0. No problems.

+		pci@15,0 {

Ditto devfn 15,0.

+		lpc@46 {

This is quite different.

The unit-addr type depends not only on the parent node, but also on
the name of the new node. For pci nodes, unit-addr is a pci devfn
within domain 0. For lpc nodes (superio), it is an io port which
supposed also is within domain 0, but that isn't really the case
since ioports exist in a separate address space on x86, which isn't
even parallell to the PCI domain devfn space, but parallell even to
the domain space. (ie. global in the system)

What to do? Mixing-and-matching address spaces is confusing.

Maybe we can hack around it, and not have to implement three
dimensions in a two-dimensional tree just yet? :p

//Peter