I managed to break dtc while working on PCI bridges: dtc only uses dev_fn as identifier for a PCI device. That gets us a name collision if we have the same dev_fn combination on multiple buses. Either we add a random unique ID to the struct name or we integrate the number of the parent device as well. A third option would be to store the complete hierarchy in the name. I decided to go for integration of parent device name.
With the following device tree
/{ cpus {}; domain@0 { bus@0 { pci@0,0 { }; pci@1,1 { }; pci@f,0 { bus@1 { pci@0,0 { }; }; }; }; }; };
we get the old names: dev_root dev_cpus dev_domain_0 dev_bus_0 dev_pci_0_0 dev_pci_1_1 dev_pci_f_0 dev_bus_1 dev_pci_0_0 COLLISION!!!
and the new names: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_bus_0_pci_0_0 dev_bus_0_pci_1_1 dev_bus_0_pci_f_0 dev_pci_f_0_bus_1 dev_bus_1_pci_0_0
and the third option (not used) would have looked like this: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_domain_0_bus_0_pci_0_0 dev_domain_0_bus_0_pci_1_1 dev_domain_0_bus_0_pci_f_0 dev_domain_0_bus_0_pci_f_0_bus_1 dev_domain_0_bus_0_pci_f_0_bus_1_pci_0_0
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: corebootv3-pci_scan_bus/util/dtc/flattree.c =================================================================== --- corebootv3-pci_scan_bus/util/dtc/flattree.c (Revision 846) +++ corebootv3-pci_scan_bus/util/dtc/flattree.c (Arbeitskopie) @@ -931,6 +931,7 @@ emit->endnode(etarget, treelabel); }
+ //fprintf(f, "//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); /* now emit the device for this node, with sibling and child pointers etc. */ emit->special(f, tree);
@@ -1313,8 +1314,23 @@ labeltree(struct node *tree) { struct node *child; + char *tmp1; + char *tmp2;
+ //printf("//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); tree->label = clean(tree->name, 1); + if (tree->parent && tree->label) { + tmp1 = clean(tree->parent->name, 1); + 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); + free(tmp2); + } + free(tmp1); + } if (tree->next_sibling) labeltree(tree->next_sibling);
Hi Ron,
here's my patch proposal for struct device naming again. It has the advantage of adding only 14 lines of code.
Regards, Carl-Daniel
On 30.08.2008 00:57, Carl-Daniel Hailfinger wrote:
I managed to break dtc while working on PCI bridges: dtc only uses dev_fn as identifier for a PCI device. That gets us a name collision if we have the same dev_fn combination on multiple buses. Either we add a random unique ID to the struct name or we integrate the number of the parent device as well. A third option would be to store the complete hierarchy in the name. I decided to go for integration of parent device name.
With the following device tree
/{ cpus {}; domain@0 { bus@0 { pci@0,0 { }; pci@1,1 { }; pci@f,0 { bus@1 { pci@0,0 { }; }; }; }; }; };
we get the old names: dev_root dev_cpus dev_domain_0 dev_bus_0 dev_pci_0_0 dev_pci_1_1 dev_pci_f_0 dev_bus_1 dev_pci_0_0 COLLISION!!!
and the new names: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_bus_0_pci_0_0 dev_bus_0_pci_1_1 dev_bus_0_pci_f_0 dev_pci_f_0_bus_1 dev_bus_1_pci_0_0
and the third option (not used) would have looked like this: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_domain_0_bus_0_pci_0_0 dev_domain_0_bus_0_pci_1_1 dev_domain_0_bus_0_pci_f_0 dev_domain_0_bus_0_pci_f_0_bus_1 dev_domain_0_bus_0_pci_f_0_bus_1_pci_0_0
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: corebootv3-pci_scan_bus/util/dtc/flattree.c
--- corebootv3-pci_scan_bus/util/dtc/flattree.c (Revision 846) +++ corebootv3-pci_scan_bus/util/dtc/flattree.c (Arbeitskopie) @@ -931,6 +931,7 @@ emit->endnode(etarget, treelabel); }
- //fprintf(f, "//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); /* now emit the device for this node, with sibling and child pointers etc. */ emit->special(f, tree);
@@ -1313,8 +1314,23 @@ labeltree(struct node *tree) { struct node *child;
char *tmp1;
char *tmp2;
//printf("//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); tree->label = clean(tree->name, 1);
if (tree->parent && tree->label) {
tmp1 = clean(tree->parent->name, 1);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);free(tmp2);}free(tmp1);}
if (tree->next_sibling) labeltree(tree->next_sibling);
I like it. it's better than mine in the sense that it is simple and gets the job done.
But it does not add the new info to the nodes (the ints that contain domain, bus, etc) in the tree that I think we ought to add. I also prefer the consolidation of lots of work into labeltree() that my patch does. Also, my patch does delete some code (although, sadly, I grow the size overall). Finally, if we really want to use the links[] support we can't do it with your patch.
So, your patch is better if we are trying to fix the uniquess problem only; my patch is better if we want to ever use links[].
Now what :-)
ron
Acked-by: Ronald G. Minnich rminnich@gmail.com
not yet tested. But some test builds look good.
ron
On Wed, Sep 3, 2008 at 8:09 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hi Ron,
here's my patch proposal for struct device naming again. It has the advantage of adding only 14 lines of code.
Regards, Carl-Daniel
On 30.08.2008 00:57, Carl-Daniel Hailfinger wrote:
I managed to break dtc while working on PCI bridges: dtc only uses dev_fn as identifier for a PCI device. That gets us a name collision if we have the same dev_fn combination on multiple buses. Either we add a random unique ID to the struct name or we integrate the number of the parent device as well. A third option would be to store the complete hierarchy in the name. I decided to go for integration of parent device name.
With the following device tree
/{ cpus {}; domain@0 { bus@0 { pci@0,0 { }; pci@1,1 { }; pci@f,0 { bus@1 { pci@0,0 { }; }; }; }; }; };
we get the old names: dev_root dev_cpus dev_domain_0 dev_bus_0 dev_pci_0_0 dev_pci_1_1 dev_pci_f_0 dev_bus_1 dev_pci_0_0 COLLISION!!!
and the new names: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_bus_0_pci_0_0 dev_bus_0_pci_1_1 dev_bus_0_pci_f_0 dev_pci_f_0_bus_1 dev_bus_1_pci_0_0
and the third option (not used) would have looked like this: dev_root dev_cpus dev_domain_0 dev_domain_0_bus_0 dev_domain_0_bus_0_pci_0_0 dev_domain_0_bus_0_pci_1_1 dev_domain_0_bus_0_pci_f_0 dev_domain_0_bus_0_pci_f_0_bus_1 dev_domain_0_bus_0_pci_f_0_bus_1_pci_0_0
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: corebootv3-pci_scan_bus/util/dtc/flattree.c
--- corebootv3-pci_scan_bus/util/dtc/flattree.c (Revision 846) +++ corebootv3-pci_scan_bus/util/dtc/flattree.c (Arbeitskopie) @@ -931,6 +931,7 @@ emit->endnode(etarget, treelabel); }
//fprintf(f, "//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); /* now emit the device for this node, with sibling and child pointers etc. */ emit->special(f, tree);@@ -1313,8 +1314,23 @@ labeltree(struct node *tree) { struct node *child;
char *tmp1;char *tmp2;//printf("//tree->label is %s, tree->parent->label is %s\n", tree->label, tree->parent ? tree->parent->label : NULL); tree->label = clean(tree->name, 1);if (tree->parent && tree->label) {tmp1 = clean(tree->parent->name, 1);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);free(tmp2);}free(tmp1);} if (tree->next_sibling) labeltree(tree->next_sibling);
On 05.09.2008 10:17, ron minnich wrote:
On Wed, Sep 3, 2008 at 8:09 PM, Carl-Daniel Hailfinger wrote:
Hi Ron,
here's my patch proposal for struct device naming again. It has the advantage of adding only 14 lines of code.
Regards, Carl-Daniel
On 30.08.2008 00:57, Carl-Daniel Hailfinger wrote:
I managed to break dtc while working on PCI bridges: dtc only uses dev_fn as identifier for a PCI device. That gets us a name collision if we have the same dev_fn combination on multiple buses. [...] Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Ronald G. Minnich rminnich@gmail.com
not yet tested. But some test builds look good.
Thanks. Per the discussion in other threads, I committed this as r860. Feel free to change the naming scheme anytime.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
ron minnich