This has been tested and builds a working boot-to-linux coreboot on
the dbe62 (old style dts) and builds a serengeti target (new dts, but
that new dts not submitted yet).
ron
Not much changed but new capabilities added.
Much cleanup as well.
Here are the changes for the dts. I believe these changes answer all the
questions
and comments we all have had to date.
I also believe this is a huge step beyond both v2
and the way we have done v3.
So, here is the serengeti dts now
(also, PLEASE NOTE, one thing at a time! Please focus on the
southbridge dts; the rest of this can change later)
[[[which also means these changes are backward-compatible;
I'm not breaking any existing platform]]]
/{
device_operations="serengeti";
mainboard_vendor = "AMD";
mainboard_name = "Serengeti";
subsystem_vendor = "PCI_VENDOR_ID_AMD";
subsystem_device = "0x2b80";
cpus { };
[EMAIL PROTECTED] {
};
[EMAIL PROTECTED] {
/config/("northbridge/amd/k8/domain");
[EMAIL PROTECTED],0 {
/config/("northbridge/amd/k8/pci");
[EMAIL PROTECTED],0 {
/config/("southbridge/amd/amd8111/dts");
ide { ide0_enable = 1;};
};
};
[EMAIL PROTECTED],0 {
/config/("northbridge/amd/k8/pci");
};
[EMAIL PROTECTED],0 {
/config/("northbridge/amd/k8/pci");
};
[EMAIL PROTECTED],1 {};
[EMAIL PROTECTED],2 {};
[EMAIL PROTECTED],3 {};
};
[EMAIL PROTECTED] {
/config/("superio/winbond/w83627hf/dts");
kbenable = "1";
com1enable = "1";
hwmenable = "1";
};
};
So the key piece is this:
[EMAIL PROTECTED],0 {
/config/("southbridge/amd/amd8111/dts");
ide { ide0_enable = 1;};
};
};
we have a south, the pci address is mainboard-dependent so we have to set it,
there is a dts, and we have one device we need to set properties in: the ide.
Note that we call it 'ide', not '[EMAIL PROTECTED],y' since its address is
determined by the chipset;
no need to put this in the mainboard dts. We just dropped an annoying v2 feature
that we had brought over to v3 by mistake.
What's the amd8111 dts look like? Why can we use 'ide' and not '[EMAIL
PROTECTED],y':
We're using the dts label for the first time. So the amd8111 dts is:
{
device_operations = "amd8111_pci";
bridge;
nic:[EMAIL PROTECTED],0{
device_operations = "amd8111_nic";
phy_lowreset = "0";
};
ide:[EMAIL PROTECTED],1 {
device_operations = "amd8111_ide";
ide0_enable = "1";
ide1_enable = "1";
};
smbus:[EMAIL PROTECTED],2{
device_operations = "amd8111_smbus";
};
usb11:[EMAIL PROTECTED],0{
device_operations = "amd8111_usb";
};
usb12:[EMAIL PROTECTED],1{
device_operations = "amd8111_usb";
};
usb2:[EMAIL PROTECTED],2{
device_operations = "amd8111_usb2";
};
};
Key piece:
ide:[EMAIL PROTECTED],1 {
device_operations = "amd8111_ide";
ide0_enable = "0";
ide1_enable = "0";
};
So in the mainboard dts, we can say 'ide', not having to say '[EMAIL
PROTECTED],1'
And there is now ONE dts file for the amd8111.
This works and builds. The code could be cleaner,
but I'm more concerned here with how
this all looks to you. Comments welcome.
I took the opportunity to do some cleanup.
Signed-off-by: Ronald G. Minnich <[EMAIL PROTECTED]>
Index: util/dtc/flattree.c
===================================================================
--- util/dtc/flattree.c (revision 1046)
+++ util/dtc/flattree.c (working copy)
@@ -110,6 +110,12 @@
if (*src == '/')
*cp = '_';
else
+ if (*src == '@')
+ *cp = '_';
+ else
+ if (*src == ',')
+ *cp = '_';
+ else
*cp = *src;
}
*cp = 0;
@@ -443,10 +449,7 @@
{
struct data d = p->val;
FILE *f = e;
- int i;
char *cleanname;
- int vallen = d.len > 4 ? 4 : d.len;
-
/* nothing to do? */
if (d.len == 0)
return;
@@ -565,9 +568,9 @@
int enabled = 1;
int linkcount = 0;
- fprintf(f, "struct device dev_%s = {\n", tree->label);
+ fprintf(f, "struct device dev_%s = {\n", tree->codelabel);
/* special case -- the root has a distinguished path */
- if (! strncmp(tree->label, "root", 4)){
+ if (! strncmp(tree->dtslabel, "root", 4)){
is_root = 1;
fprintf(f, "\t.path = { .type = DEVICE_PATH_ROOT },\n");
}
@@ -617,7 +620,7 @@
}
if (tree->config){
- configname = clean(tree->label, 0);
+ configname = clean(tree->codelabel, 0);
printf("\t.device_configuration = &%s,\n", configname);
/* The config property list for a device is derived from the
* device dts, e.g. northbridge/intel/i440bx/dts, not the
@@ -640,6 +643,13 @@
fprintf(f, "\t.ops_pci_bus = &%s,\n",
clean((char *)prop->val.val, 0));
ops_set = 1;
}
+ if (streq(prop->name, "enabled")){
+ enabled = 1;
+ }
+ if (streq(prop->name, "disabled")){
+ enabled = 0;
+ }
+
}
}
/* Process the properties specified in the mainboard dts.
@@ -673,7 +683,7 @@
}
if (streq(prop->name, "config")){
- fprintf(f, "\t.device_configuration = &%s,\n",
clean(tree->label, 1));
+ fprintf(f, "\t.device_configuration = &%s,\n",
clean(tree->dtslabel, 1));
}
if (streq(prop->name, "ops")){
@@ -697,7 +707,7 @@
}
}
if (tree->next_sibling)
- fprintf(f, "\t.sibling = &dev_%s,\n",
tree->next_sibling->label);
+ fprintf(f, "\t.sibling = &dev_%s,\n",
tree->next_sibling->codelabel);
/* If we are a bridge, and we have not been linked, then set up our
links.
* There is a good chance we could expand the for loop to contain this
first bit of code.
@@ -707,20 +717,20 @@
struct node *siblings;
fprintf(f,"\t.link = {\n");
fprintf(f,"\t\t[%d] = {\n", linkcount);
- fprintf(f,"\t\t\t.dev = &dev_%s,\n", tree->label);
+ fprintf(f,"\t\t\t.dev = &dev_%s,\n", tree->codelabel);
fprintf(f,"\t\t\t.link = %d,\n", linkcount);
if (tree->children)
- fprintf(f,"\t\t\t.children = &dev_%s\n",
tree->children->label);
+ fprintf(f,"\t\t\t.children = &dev_%s\n",
tree->children->codelabel);
fprintf(f,"\t\t},\n");
/* now we need to handle our siblings. */
linkcount++;
for_all_siblings(tree, siblings) {
if (is_bridge(siblings) && (!siblings->linked)){
fprintf(f,"\t\t[%d] = {\n", linkcount);
- fprintf(f,"\t\t\t.dev = &dev_%s,\n",
siblings->label);
+ fprintf(f,"\t\t\t.dev = &dev_%s,\n",
siblings->codelabel);
fprintf(f,"\t\t\t.link = %d,\n", linkcount);
if (siblings->children) {
- fprintf(f,"\t\t\t.children =
&dev_%s\n", siblings->children->label);
+ fprintf(f,"\t\t\t.children =
&dev_%s\n", siblings->children->codelabel);
siblings->children->linked = 1;
siblings->children->linknode = tree;
siblings->children->whichlink =
linkcount;
@@ -738,19 +748,19 @@
/* fill in the 'bus I am on' entry */
/* being 'linked' on a bus overrides the parent link */
if (tree->linked)
- fprintf(f, "\t.bus = &dev_%s.link[%d],\n",
tree->linknode->label, tree->whichlink);
+ fprintf(f, "\t.bus = &dev_%s.link[%d],\n",
tree->linknode->codelabel, tree->whichlink);
else if (tree->parent)
- fprintf(f, "\t.bus = &dev_%s.link[0],\n", tree->parent->label);
+ fprintf(f, "\t.bus = &dev_%s.link[0],\n",
tree->parent->codelabel);
else /* this is a very unusual case: the root */
- fprintf(f, "\t.bus = &dev_%s.link[0],\n", tree->label);
+ fprintf(f, "\t.bus = &dev_%s.link[0],\n", tree->codelabel);
if (tree->next)
- fprintf(f, "\t.next = &dev_%s,\n", tree->next->label);
+ fprintf(f, "\t.next = &dev_%s,\n", tree->next->codelabel);
if ((! ops_set) && is_root)
fprintf(f, "\t.ops = &default_dev_ops_root,\n");
- fprintf(f, "\t.dtsname = \"%s\",\n", tree->label);
+ fprintf(f, "\t.dtsname = \"%s\",\n", tree->codelabel);
fprintf(f, "\t.enabled = %d\n", enabled);
fprintf(f, "};\n");
@@ -865,7 +875,7 @@
if (tree->config){
int i;
-// treename = clean(tree->label, 0);
+// treename = clean(tree->dtslabel, 0);
treename = toname(tree->config->label, "_config");
for(i = 0; i < emitted_names_count; i++)
if (!strcmp(treename, emitted_names[i]))
@@ -962,11 +972,10 @@
void *etarget, struct data *strbuf,
struct version_info *vi)
{
- char *treename, *treelabel, *structname;
+ char *treelabel, *structname;
struct property *configprop, *dtsprop;
struct node *child;
- int seen_name_prop = 0;
FILE *f = etarget;
/* here is the real action. What we have to do, given a -> config
entry, is this:
* foreach property(tree->config)
@@ -979,7 +988,7 @@
*/
if (tree->config){
- treelabel = clean(tree->label, 0);
+ treelabel = clean(tree->codelabel, 0);
structname = toname(tree->config->label, "_config");
/* beginnode does not work here.
* the design of this code is wrong and must be fixed.
@@ -988,7 +997,6 @@
printf("struct %s %s = {\n", structname, treelabel);
for_each_config(tree, configprop) {
- char *cleanname;
int found = 0;
if (streq(configprop->name, "constructor")) /* this is
special */
continue;
@@ -1023,11 +1031,11 @@
void *etarget, struct data *strbuf,
struct version_info *vi)
{
- struct property *prop, *config;
+ struct property *prop;
struct node *child;
int seen_name_prop = 0;
- emit->beginnode(etarget, tree->label);
+ emit->beginnode(etarget, tree->dtslabel);
if (vi->flags & FTF_FULLPATH)
emit->string(etarget, tree->fullpath, 0);
@@ -1091,7 +1099,7 @@
}
- emit->endnode(etarget, tree->label);
+ emit->endnode(etarget, tree->dtslabel);
}
static struct data flatten_reserve_list(struct reserve_info *reservelist,
@@ -1394,19 +1402,25 @@
char *tmp1;
char *tmp2;
- tree->label = clean(tree->name, 1);
- if (tree->parent && tree->label) {
- tmp1 = strdup(tree->parent->label);
+ tree->codelabel = clean(tree->name, 1);
+ if (tree->parent && tree->codelabel) {
+ tmp1 = strdup(tree->parent->codelabel);
if (strlen(tmp1)) {
- tmp2 = tree->label;
- tree->label = malloc(strlen(tmp1) + strlen(tmp2) + 2);
- strcpy(tree->label, tmp1);
- strcat(tree->label, "_");
- strcat(tree->label, tmp2);
+ tmp2 = tree->codelabel;
+ tree->codelabel = malloc(strlen(tmp1) + strlen(tmp2) +
2);
+ strcpy(tree->codelabel, tmp1);
+ strcat(tree->codelabel, "_");
+ strcat(tree->codelabel, tmp2);
free(tmp2);
}
free(tmp1);
}
+
+ /* they are not required to provide a label; provide one if they
+ * have not.
+ */
+ if (! tree->dtslabel)
+ tree->dtslabel = tree->name;
if (tree->next_sibling)
labeltree(tree->next_sibling);
@@ -1435,7 +1449,6 @@
struct version_info *vi = NULL;
int i;
struct data strbuf = empty_data;
- char *symprefix = "dt";
extern char *code;
struct node *next;
extern struct node *first_node;
@@ -1453,14 +1466,14 @@
/* the root is special -- the parser gives it no name. We fix that
here.
* fix in parser?
*/
- bi->dt->name = bi->dt->label = "root";
+ bi->dt->name = bi->dt->codelabel = bi->dt->dtslabel = "root";
/* steps: emit all structs. Then emit the initializers, with the
pointers to other structs etc. */
fix_next(bi->dt);
fprintf(f, "#include <statictree.h>\n");
/* forward declarations */
for(next = first_node; next; next = next->next)
- fprintf(f, "struct device dev_%s;\n", next->label);
+ fprintf(f, "struct device dev_%s;\n", next->codelabel);
/* special for the root. Emit the names for the mainboard vendor and
part # */
for_each_property(bi->dt, prop) {
@@ -1522,9 +1535,7 @@
struct version_info *vi = NULL;
int i;
struct data strbuf = empty_data;
- char *symprefix = "dt";
extern char *code;
- struct node *next;
struct property *prop;
int found_mainboard_subsys = 0;
extern struct node *first_node;
@@ -1541,7 +1552,7 @@
/* the root is special -- the parser gives it no name. We fix that
here.
* fix in parser?
*/
- bi->dt->name = bi->dt->label = "root";
+ bi->dt->name = bi->dt->dtslabel = "root";
/* steps: emit all structs. Then emit the initializers, with the
pointers to other structs etc. */
fix_next(bi->dt);
Index: util/dtc/livetree.c
===================================================================
--- util/dtc/livetree.c (revision 1046)
+++ util/dtc/livetree.c (working copy)
@@ -25,6 +25,7 @@
*/
struct node *first_node = NULL;
+struct node *last_node = NULL;
struct property *build_property(char *name, struct data val, char *label)
{
@@ -43,6 +44,74 @@
return new;
}
+/* XXX HACK */
+void move_proplist_to_config(struct node *f)
+{
+ struct node *n = f;
+ while (n) {
+ n->config = n->proplist;
+ n->proplist = NULL;
+ move_proplist_to_config(n->children);
+ n = n->next_sibling;
+ }
+}
+
+/**
+ * Given a set of nodes from a chip dts, and a set of nodes from the mainboard
dts,
+ * update the chip dts nodes with values from the mainboard dts.
+ * All this work is so that a chip dts can set default values and a mainboard
+ * can override them.
+ * Node definitions come from the chip dts.
+ * These definitions have properties that are used in turn to create the
+ * C structures.
+ * Changing the vlaues of properties in the node definitions is done by
+ * creating nodes in the mainboard dts.
+ * For each of the node defintions from 'config', we
+ * have to match the node definitions in 'subnodes',
+ * that have matching labels. For each matching label, we modify the
+ * property settings for the node in config.
+ * Finally, for all subnodes that are leftover, i.e. did not match anything in
the
+ * config, we simply append them to the list of subnodes from config.
+ * Later, it might be an error to have a missing match.
+ *
+ */
+void
+fixup_properties(struct node *chipnodes, struct node *mainboardnodes)
+{
+ struct node *c;
+ struct node *m = mainboardnodes;
+ struct node *del = NULL;
+ struct node *head = mainboardnodes;
+
+ while (m) {
+ for (c = chipnodes; c; c = c->next) {
+ if (strcmp(c->dtslabel, m->dtslabel)) {
+ continue;
+ }
+ /* match */
+ /* the chip properties are in c->config list
+ * the dts properties are in the m->proplist.
+ * So copy the mainboard properties to the chip
proplist.
+ */
+ c->proplist = m->proplist;
+ c->children = m->children;
+ /* since we matched this node we have to delete it. */
+ del = m;
+ break;
+ }
+ /* must track head node for append_node below. */
+ if (m == head)
+ head = m->next;
+ m = m->next;
+ if (del)
+ del_node(del);
+ del = NULL;
+ }
+ /* backwards compatibility: anything left gets appended to the chip
list */
+ if (head)
+ append_node(chipnodes, head);
+}
+
struct property *chain_property(struct property *first, struct property *list)
{
assert(first->next == NULL);
@@ -51,12 +120,9 @@
return first;
}
-struct node *build_node(struct property *config, struct property *proplist,
struct node *children)
+struct node *new_node(struct property *config, struct property *proplist,
struct node *children)
{
- static struct node *last_node = NULL;
-
struct node *new = xmalloc(sizeof(*new));
- struct node *child;
memset(new, 0, sizeof(*new));
@@ -64,6 +130,39 @@
new->proplist = proplist;
new->children = children;
+ return new;
+}
+
+/* we should only ever delete simple nodes: nodes with no kids. */
+void del_node(struct node *node)
+{
+ struct node *n;
+ assert(! node->children);
+ if (first_node == node) {
+ first_node = node->next;
+ free(node);
+ return;
+ }
+ for_all_nodes(n) {
+ if (n->next != node)
+ continue;
+ n->next = node->next;
+ n->next_sibling = node->next_sibling;
+ if (node == last_node)
+ last_node = n;
+ break;
+ }
+ free(node);
+}
+
+struct node *build_node(struct property *config, struct property *proplist,
struct node *children)
+{
+
+ struct node *new;
+ struct node *child;
+
+ new = new_node(config, proplist, children);
+
for_each_child(new, child) {
child->parent = new;
}
@@ -85,7 +184,7 @@
node->name = name;
- node->label = label;
+ node->dtslabel = label;
return node;
}
@@ -98,6 +197,16 @@
return first;
}
+struct node *append_node(struct node *first, struct node *list)
+{
+ struct node *n = first;
+ while(n->next_sibling) {
+ n = n->next_sibling;
+ }
+ n->next_sibling = list;
+ return first;
+}
+
void add_property(struct node *node, struct property *prop)
{
struct property **p;
@@ -462,7 +571,6 @@
static int check_root(struct node *root)
{
- struct property *prop;
int ok = 1;
// CHECK_HAVE_STRING(root, "model");
@@ -479,7 +587,6 @@
{
struct node *cpus, *cpu;
struct property *prop;
- struct node *bootcpu = NULL;
int ok = 1;
cpus = get_subnode(root, "cpus");
Index: util/dtc/dtc.h
===================================================================
--- util/dtc/dtc.h (revision 1046)
+++ util/dtc/dtc.h (working copy)
@@ -165,7 +165,8 @@
cell_t phandle;
int addr_cells, size_cells;
- char *label;
+ char *dtslabel;
+ char *codelabel;
/* coreboot-specific */
int linked; /* has this node been added to the links for a bridge? */
struct node *linknode; /* If I am linked, which node is it? */
@@ -175,6 +176,9 @@
#define for_each_property(n, p) \
for ((p) = (n)->proplist; (p); (p) = (p)->next)
+#define for_all_nodes(n)\
+ for (n = first_node; n; n = n->next)
+
#define for_each_child(n, c) \
for ((c) = (n)->children; (c); (c) = (c)->next_sibling)
@@ -186,11 +190,16 @@
for ((p) = (n)->config; (p); (p) = (p)->next)
struct property *build_property(char *name, struct data val, char *label);
+void move_proplist_to_config(struct node *f);
+void fixup_properties(struct node *chipnodes, struct node *mainboardnodes);
struct property *chain_property(struct property *first, struct property *list);
-
+struct property *append_property(struct property *first, struct property
*list);
+void del_node(struct node *node);
struct node *build_node(struct property *config, struct property *proplist,
struct node *children);
+struct node *new_node(struct property *config, struct property *proplist,
struct node *children);
struct node *name_node(struct node *node, char *name, char *label);
struct node *chain_node(struct node *first, struct node *list);
+struct node *append_node(struct node *first, struct node *list);
void add_property(struct node *node, struct property *prop);
void add_child(struct node *parent, struct node *child);
Index: util/dtc/tests/run_all_tests.sh
===================================================================
--- util/dtc/tests/run_all_tests.sh (revision 1046)
+++ util/dtc/tests/run_all_tests.sh (working copy)
@@ -1,2 +1,6 @@
#! /bin/sh
+for i in *.dts;
+do
+../dtc $i
+done
Index: util/dtc/dtc-parser.y
===================================================================
--- util/dtc/dtc-parser.y (revision 1046)
+++ util/dtc/dtc-parser.y (working copy)
@@ -69,7 +69,7 @@
%type <node> devicetree
%type <node> nodedef
%type <node> subnode
-%type <proplist> config
+%type <node> config
%type <nodelist> subnodes
%type <str> label
%type <str> nodename
@@ -106,7 +106,17 @@
;
nodedef: '{' config proplist subnodes '}' ';' {
- $$ = build_node($2, $3, $4);
+ struct node *n = $2;
+ struct node *subnodes = $4;
+ struct property *c = NULL;
+ if (n){
+ c = n->config;
+ if (n->children){
+ fixup_properties(n->children, subnodes);
+ subnodes = n->children;
+ }
+ }
+ $$ = build_node(c, $3, subnodes);
}
;
@@ -134,12 +144,13 @@
exit(1);
}
switchin(f);
- } '{' proplist '}' ';' {
+ } '{' proplist subnodes '}' ';' {
void switchback(void);
switchback();
}
')' ';' {
+ struct node *n = $7;
int namelen;
char *name = strdup((char *)$3.val);
/* convention: first property is labeled with
path */
@@ -149,8 +160,30 @@
namelen = strlen($6->label);
if ((namelen > 4) && (!
strncmp(&name[namelen-4], ".dts", 4)))
$6->label[namelen-4] = '\0';
+ /* Since we're in the config of a chip, the
properties need to be part of the config list,
+ * not the proplist. This change is new since
before we did not allow subnodes in the
+ * config of a device.
+ * slight temporary hack here until we're sure
this is the way to go. Once we are
+ * sure, we will have new nonterminals for
'configsubnodes' so that properties from this
+ * path end up in the config list, not the
properties list.
+ */
+ for(n = $7; n; n = n->next) {
+ char *conflabel;
+ int lablen = strlen($6->label) +
strlen(n->name) + 3;
+ struct property *p = n->proplist;
+ if (! p)
+ continue;
+ conflabel = malloc(lablen);
+ sprintf(conflabel,
+ "%s_%s", $6->label, n->name);
+ p->label = conflabel;
+ }
+ n = $7;
+ move_proplist_to_config(n);
- $$ = $6;
+ n = new_node($6, NULL, $7);
+
+ $$ = n;
}
|
;
@@ -196,6 +229,7 @@
nodename: DT_NODENAME { $$ = $1; }
| DT_PROPNAME { $$ = $1; }
+ | /* empty */ { $$ = NULL; }
;
label: DT_LABEL { $$ = $1; }
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
