[Adding the coreboot mailing list to CC. It's moderated for non-subscribers, but it won't take long for legitimate mails to be approved.]
On 13.02.2009 03:17, David Gibson wrote:
On Fri, Feb 13, 2009 at 03:11:20AM +0100, Carl-Daniel Hailfinger wrote:
On 13.02.2009 01:43, David Gibson wrote:
On Thu, Feb 12, 2009 at 11:26:46AM +0100, Markus Armbruster wrote:
I didn't mean to say they are a bad idea for FDTs, just that they're on an awkward level of abstraction for QEMU configuration. There, I'd rather express a PCI address as "02:01.0" than as <0x00000220>. Translating text to binary is the machine's job, not the user's.
Ah, I see what you mean. Hrm, there are several possibilities here, we'll have to see which works out best for your purposes.
Using the DTC version included in the coreboot v3 sources would solve that problem and give you a readable PCI address representation.
Hrm.. it would be nice if you'd co-ordinated with Jon and I about this. Then we could have at least the bits which make sense in upstream dtc...
Probably the biggest obstacle for a full merge right now is that the coreboot v3 DTC is rather old and has been extended not only for a more readable DTS syntax variant, but also for additional output modes (C header and C code).
We (coreboot developers) are interested in reducing our diff with upstream DTC in order to improve maintainability of our DTC code.
Regards, Carl-Daniel
On Fri, Feb 13, 2009 at 03:45:45AM +0100, Carl-Daniel Hailfinger wrote:
[Adding the coreboot mailing list to CC. It's moderated for non-subscribers, but it won't take long for legitimate mails to be approved.]
On 13.02.2009 03:17, David Gibson wrote:
On Fri, Feb 13, 2009 at 03:11:20AM +0100, Carl-Daniel Hailfinger wrote:
On 13.02.2009 01:43, David Gibson wrote:
On Thu, Feb 12, 2009 at 11:26:46AM +0100, Markus Armbruster wrote:
I didn't mean to say they are a bad idea for FDTs, just that they're on an awkward level of abstraction for QEMU configuration. There, I'd rather express a PCI address as "02:01.0" than as <0x00000220>. Translating text to binary is the machine's job, not the user's.
Ah, I see what you mean. Hrm, there are several possibilities here, we'll have to see which works out best for your purposes.
Using the DTC version included in the coreboot v3 sources would solve that problem and give you a readable PCI address representation.
Hrm.. it would be nice if you'd co-ordinated with Jon and I about this. Then we could have at least the bits which make sense in upstream dtc...
Probably the biggest obstacle for a full merge right now is that the coreboot v3 DTC is rather old and has been extended not only for a more readable DTS syntax variant, but also for additional output modes (C header and C code).
If the C output mode is what I'm guessing, it should be pretty easy to add (we already have an asm output mode upstream).
The syntax changes will be trickier. I want to review any new syntax for dts very carefully, because I really, really don't want to have to break backwards compatibility in future (I'm unhappy enough about the dts-v0 to dts-v1 transition we've already have).
Can you summarise what the syntax changes are? Maybe start a new thread with just devicetree-discuss not the other lists for that.
We (coreboot developers) are interested in reducing our diff with upstream DTC in order to improve maintainability of our DTC code.
Good :)
Here is the sum total of the differences from when we checked it in over 2 years ago until now (parser). Our real changes are to flattree.c and livetree.c, where we do some ugly by-hand parsing of the ids such that pci@1,0 etc. work. I'd love to see a way to bring this into the real syntax. I've tried to do as little as possible to .y and .l.
The diff with comments is attached.
But this brings up a bigger issue and we could use your help.
OK, what did we do? We implemented the ability to have a sort of template. Here is a sample from real use.
/{ mainboard_vendor = "Artec"; mainboard_name = "DBE62"; cpus { }; apic@0 { /config/("northbridge/amd/geodelx/apic"); }; domain@0 { /config/("northbridge/amd/geodelx/domain"); pci@1,0 { /config/("northbridge/amd/geodelx/pci"); /* Video RAM has to be in 2MB chunks. */ geode_video_mb = "16"; }; etc.
so what's going on here?
The config file in most cases is pretty straightforward. It's actually just a list of properties with a standard setting for chip control. We MUST have this; we don't want hundreds of settings in each mainboard, because sometimes a chip fix comes along and we want that to go into one chip file, and set the correct value, and have all mainboards get the new value next time they are built.
Let's look at /config/("northbridge/amd/geodelx/pci");
{ device_operations = "geodelx_mc";
/* Video RAM has to be in 2MB chunks. */ geode_video_mb = "0"; };
The device_operations property is processed by flattree and is of no importance to you, but it is used in coreboot .h and .c code generation. For coreboot use, we have several property names that are special.
Note that we create a property, geode_video_mb, and set it to 0.
In the mainboard dts, we over-ride this value, and set it to 16.
These are pretty much the changes and, again, they work. But I'd like more, as would our community.
Right now, we can take a file containing a list of dts properties, read them in, and modify them as above. It's not really ideal, and I am sure you can already see it could be done better. But what we really want is the ability to read in a dts node (with subnodes, etc.) and then elide them in the mainboard file.
So, for example, we have this subsection of one mainboard:
pci@6{ /* Port 2 */ /config/("southbridge/amd/rs690/pcie.dts"); }; pci@7{ /* Port 3 */ /config/("southbridge/amd/rs690/pcie.dts"); }; pci@12{ /config/("southbridge/amd/sb600/hda.dts"); }; pci@13,0{ /config/("southbridge/amd/sb600/usb.dts"); }; pci@13,1{ /config/("southbridge/amd/sb600/usb.dts"); }; pci@13,2{ /config/("southbridge/amd/sb600/usb.dts"); };
This is not a bunch of chips, but one chip. It has lots of pci devices in it; this one chip is equivalent to a whole mainboard from previous years. What we'd really like is the ability to do what my wife calls restrict, add, and remove (I don't have these terms just right, it's some kind of compiler-speak which is what she does for a living).
Restrict we have; change property values from a default. Add is what we'd like: add a node to a tree in some way. Remove we would also like: remove a node from a dts we have read in via /config/.
Note that the syntax is only suggested here, the right way to do this is up to you experts.
/{ device_operations="dbm690t"; mainboard_vendor = "AMD"; mainboard_name = "dbm690t"; cpus { }; apic@0 { }; domain@0 /config/("northbridge/amd/k8/domain") = { pci@1,0 /config/("southbridge/amd/sb600/dts") = { /* change default xyz to "1" */ xyz = "1"; /* disable pcie port 6. Note this is over-riding a value in a node. This is new. */ pcie@1,0{disable;}; /* don't even put port 7 in the tree -- what is a remove going to look like?. Also new. */ - pcie@5,0; /* add the superio; default values are acceptable. Also new. We can't add nodes. */ pnp@2e /config/("superio/winbond/1234"); }; }; };
The result would be more compact files and easier maintenance.
I realize these changes may be too large for the dts to take on; there is an ongoing discussion as to whether some other language might not be more appropriate.
But, at the same time, people are comfortable with dts. They have found it very comfortable to use.
I'd like to thank you for this excellent tool. It is being used to build production BIOSes that are shipping in products as I write this. It really saved us a lot of work on coreboot v3 and it is a much better job, certainly, than I could have done myself.
Thanks, and I hope we can discuss this and work together.
ron
On Fri, Feb 13, 2009 at 09:07:08AM -0800, ron minnich wrote:
Here is the sum total of the differences from when we checked it in over 2 years ago until now (parser). Our real changes are to flattree.c and livetree.c, where we do some ugly by-hand parsing of the ids such that pci@1,0 etc. work. I'd love to see a way to bring this into the real syntax. I've tried to do as little as possible to .y and .l.
The diff with comments is attached.
But this brings up a bigger issue and we could use your help.
OK, what did we do? We implemented the ability to have a sort of template. Here is a sample from real use.
/{ mainboard_vendor = "Artec"; mainboard_name = "DBE62"; cpus { }; apic@0 { /config/("northbridge/amd/geodelx/apic"); }; domain@0 { /config/("northbridge/amd/geodelx/domain"); pci@1,0 { /config/("northbridge/amd/geodelx/pci"); /* Video RAM has to be in 2MB chunks. */ geode_video_mb = "16"; }; etc.
so what's going on here?
The config file in most cases is pretty straightforward. It's actually just a list of properties with a standard setting for chip control. We MUST have this; we don't want hundreds of settings in each mainboard, because sometimes a chip fix comes along and we want that to go into one chip file, and set the correct value, and have all mainboards get the new value next time they are built.
Let's look at /config/("northbridge/amd/geodelx/pci");
{ device_operations = "geodelx_mc";
/* Video RAM has to be in 2MB chunks. */ geode_video_mb = "0"; };
The device_operations property is processed by flattree and is of no importance to you, but it is used in coreboot .h and .c code generation. For coreboot use, we have several property names that are special.
Note that we create a property, geode_video_mb, and set it to 0.
In the mainboard dts, we over-ride this value, and set it to 16.
These are pretty much the changes and, again, they work. But I'd like more, as would our community.
Right now, we can take a file containing a list of dts properties, read them in, and modify them as above. It's not really ideal, and I am sure you can already see it could be done better. But what we really want is the ability to read in a dts node (with subnodes, etc.) and then elide them in the mainboard file.
So, for example, we have this subsection of one mainboard:
pci@6{ /* Port 2 */ /config/("southbridge/amd/rs690/pcie.dts"); }; pci@7{ /* Port 3 */ /config/("southbridge/amd/rs690/pcie.dts"); }; pci@12{ /config/("southbridge/amd/sb600/hda.dts"); }; pci@13,0{ /config/("southbridge/amd/sb600/usb.dts"); }; pci@13,1{ /config/("southbridge/amd/sb600/usb.dts"); }; pci@13,2{ /config/("southbridge/amd/sb600/usb.dts"); };This is not a bunch of chips, but one chip. It has lots of pci devices in it; this one chip is equivalent to a whole mainboard from previous years. What we'd really like is the ability to do what my wife calls restrict, add, and remove (I don't have these terms just right, it's some kind of compiler-speak which is what she does for a living).
Hrm, I see. So, if we added the ability to list properties multiple times, with the last definition overriding earlier ones, then I believe that, along with include files, which are already supported would accomplish what you have implemented with /config/. Does that seem correct?
Restrict we have; change property values from a default. Add is what we'd like: add a node to a tree in some way. Remove we would also like: remove a node from a dts we have read in via /config/.
Hrm. Well, this sort of thing is certainly on the cards with the expression support stuff we had in mind.
On Thu, Feb 19, 2009 at 6:29 PM, David Gibson dwg@au1.ibm.com wrote:
Hrm, I see. So, if we added the ability to list properties multiple times, with the last definition overriding earlier ones, then I believe that, along with include files, which are already supported would accomplish what you have implemented with /config/. Does that seem correct?
That ought to do it.
Restrict we have; change property values from a default. Add is what we'd like: add a node to a tree in some way. Remove we would also like: remove a node from a dts we have read in via /config/.
Hrm. Well, this sort of thing is certainly on the cards with the expression support stuff we had in mind.
Would be just what we need.
Thanks
ron
-
Carl-Daniel Hailfinger -
David Gibson -
ron minnich