[coreboot-gerrit] Change in coreboot[master]: util/sconfig: Add support for multiple device instances

Furquan Shaikh (Code Review) gerrit at coreboot.org
Mon Jun 4 02:49:51 CEST 2018


Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/26800


Change subject: util/sconfig: Add support for multiple device instances
......................................................................

util/sconfig: Add support for multiple device instances

This change adds support for a device to have multiple instances. This
is required to support multidev devices i.e. device with same id
existing under the same parent. Currently, multidev devices were
handled by adding all the devices to devicetree and then skipping over
the nodes by adjusting next and sibling pointers in fold_in. This
change fixes this by letting a device hold of list of instances if it
is a multidev. Each instance can have its own list of children
indicating the child nodes for that instance. Device node in such
cases has children pointer set to the first child among all
instances.

BUG=b:80081934
TEST=Verified using abuild that all boards compile successfully and
static.c generated with and without this change is the same. (In order
to compare static.c, count was incremented even when adding device
instance).

Change-Id: Ic8c8a73a247e8e992ab6b1b2cc3131e06fa2e5a1
Signed-off-by: Furquan Shaikh <furquan at google.com>
---
M util/sconfig/main.c
M util/sconfig/sconfig.h
M util/sconfig/sconfig.tab.c_shipped
M util/sconfig/sconfig.y
4 files changed, 282 insertions(+), 180 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/26800/1

diff --git a/util/sconfig/main.c b/util/sconfig/main.c
index b6f2754..04ffb98 100644
--- a/util/sconfig/main.c
+++ b/util/sconfig/main.c
@@ -135,18 +135,23 @@
 	return dev;
 }
 
-static int device_match(struct device *a, struct device *b)
+/*
+ * This function returns 1 if device has instances i.e. linkcnt is not
+ * zero. Else, it returns 0.
+ */
+static int device_has_instance(struct device *d)
 {
-	if ((a->bustype == b->bustype) && (a->parent == b->parent)
-	    && (a->path_a == b->path_a) && (a->path_b == b->path_b))
-		return 1;
-	return 0;
+	return !!d->linkcnt;
 }
 
 void fold_in(struct device *parent)
 {
-	struct device *child = parent->children;
+	struct device *child;
 	struct device *latest = 0;
+
+	child = device_has_instance(parent) ? parent->last_instance->children
+		: parent->children;
+
 	while (child != latest) {
 		if (child->children) {
 			if (!latest)
@@ -179,17 +184,6 @@
 	 * here to have the next_sibling and latestchild setup correctly.
 	 */
 	fold_in(&root);
-
-	struct device *dev = &root;
-	while (dev) {
-		/* skip functions of the same device in sibling chain */
-		while (dev->sibling && dev->sibling->used)
-			dev->sibling = dev->sibling->sibling;
-		/* skip duplicate function elements in next chain */
-		while (dev->next && dev->next->used)
-			dev->next = dev->next->next;
-		dev = dev->next_sibling;
-	}
 }
 
 char *translate_name(const char *str, translate_t mode)
@@ -274,21 +268,103 @@
 	return instance;
 }
 
+/*
+ * This function creates a new instance of the device. If instances of the
+ * device already exist, then current instance is appended at the end of the
+ * instance list. If this is the first instance, then dev_instance and
+ * last_instance for the device are updated to point to the current instance.
+ * Additionally, linkcnt is incremented to indicate the number of instances that
+ * the device has.
+ */
+static struct dev_instance *alloc_dev_instance(struct device *d)
+{
+	struct dev_instance *ins = S_ALLOC(sizeof(*ins));
+
+	if (!device_has_instance(d))
+		d->dev_instance = ins;
+	else
+		d->last_instance->next = ins;
+
+	d->last_instance = ins;
+	d->linkcnt++;
+	return ins;
+}
+
+static void make_dev_multidev(struct device *d)
+{
+	struct dev_instance *ins = alloc_dev_instance(d);
+
+	/* Make children of device the children of first instance. */
+	ins->children = d->children;
+}
+
+/*
+ * Check if device exists under the provided parent that matches the new device
+ * properties i.e. path, bustype and chip instance. If yes, then make it a
+ * multidev (if not already) and allocate a new instance for it.
+ */
+static struct device *get_dev(struct device *parent, int path_a, int path_b,
+			      int bustype, struct chip_instance *chip_ins)
+{
+	/*
+	 * Get children of parent if parent doesn't have instances. Else get the
+	 * children of last instance.
+	 */
+	struct device *child = device_has_instance(parent) ?
+		parent->last_instance->children : parent->children;
+
+	while (child) {
+		if ((child->path_a == path_a) && (child->path_b == path_b) &&
+		    (child->bustype == bustype) &&
+		    (child->chip_instance == chip_ins))
+			break;
+
+		child = child->sibling;
+	}
+
+	if (!child)
+		return NULL;
+
+	/*
+	 * If the matched device does not have any instances yet, then it first
+	 * needs to be converted to a multidev device.
+	 */
+	if (child->dev_instance == NULL)
+		make_dev_multidev(child);
+
+	/* Now a new instance can be allocated. */
+	alloc_dev_instance(child);
+
+	return child;
+}
+
 struct device *new_device(struct device *parent,
 			  struct chip_instance *chip_instance,
 			  const int bustype, const char *devnum,
 			  int enabled)
 {
-	struct device *new_d = new_dev(parent);
-	new_d->bustype = bustype;
-
 	char *tmp;
-	new_d->path_a = strtol(devnum, &tmp, 16);
+	int path_a;
+	int path_b = 0;
+	struct device *new_d;
+
+	path_a = strtol(devnum, &tmp, 16);
 	if (*tmp == '.') {
 		tmp++;
-		new_d->path_b = strtol(tmp, NULL, 16);
+		path_b = strtol(tmp, NULL, 16);
 	}
 
+	/* If device is found under parent, no need to allocate new device. */
+	new_d = get_dev(parent, path_a, path_b, bustype, chip_instance);
+	if (new_d)
+		return new_d;
+
+	new_d = new_dev(parent);
+	new_d->bustype = bustype;
+
+	new_d->path_a = path_a;
+	new_d->path_b = path_b;
+
 	char *name = S_ALLOC(10);
 	sprintf(name, "_dev%d", new_d->id);
 	new_d->name = name;
@@ -296,14 +372,40 @@
 	new_d->enabled = enabled;
 	new_d->chip_instance = chip_instance;
 
-	if (parent->latestchild) {
+	/*
+	 * If parent is multidev, then update current node's linknum to indicate
+	 * which instance of the parent node is the actual parent of current
+	 * node.
+	 */
+	if (parent->linkcnt)
+		new_d->linknum = parent->linkcnt - 1;
+
+	if (parent->latestchild)
 		parent->latestchild->next_sibling = new_d;
-		parent->latestchild->sibling = new_d;
-	}
+
 	parent->latestchild = new_d;
+
+	struct device *c;
+	c = device_has_instance(parent) ? parent->last_instance->children :
+			parent->children;
+
+	if (c) {
+		while (c->sibling)
+			c = c->sibling;
+		c->sibling = new_d;
+	}
+
+	/* Set children of parent if this is its first child. */
 	if (!parent->children)
 		parent->children = new_d;
 
+	/*
+	 * If parent has instances and the last instance does not have a child,
+	 * then set children for last instance.
+	 */
+	if (device_has_instance(parent) && !parent->last_instance->children)
+		parent->last_instance->children = new_d;
+
 	lastnode->next = new_d;
 	lastnode = new_d;
 
@@ -360,29 +462,6 @@
 	return new_d;
 }
 
-void alias_siblings(struct device *d)
-{
-	while (d) {
-		int link = 0;
-		struct device *cmp = d->next_sibling;
-		while (cmp && (cmp->parent == d->parent) && (cmp->path_a == d->path_a)
-		       && (cmp->path_b == d->path_b)) {
-			if (!cmp->used) {
-				if (device_match(d, cmp)) {
-					d->multidev = 1;
-
-					cmp->id = d->id;
-					cmp->name = d->name;
-					cmp->used = 1;
-					cmp->link = ++link;
-				}
-			}
-			cmp = cmp->next_sibling;
-		}
-		d = d->next_sibling;
-	}
-}
-
 void add_resource(struct device *dev, int type, int index, int base)
 {
 	struct resource *r = S_ALLOC(sizeof(struct resource));
@@ -472,21 +551,94 @@
 
 static void pass0(FILE * fil, struct device *ptr)
 {
-	if (ptr->id == 0)
+	if (ptr->id == 0) {
 		fprintf(fil, "DEVTREE_CONST struct bus %s_links[];\n",
 			ptr->name);
+		return;
+	}
 
-	if ((ptr->id != 0) && (!ptr->used)) {
-		fprintf(fil, "DEVTREE_CONST static struct device %s;\n",
+	fprintf(fil, "DEVTREE_CONST static struct device %s;\n", ptr->name);
+	if (ptr->rescnt > 0)
+		fprintf(fil, "DEVTREE_CONST struct resource %s_res[];\n",
 			ptr->name);
-		if (ptr->rescnt > 0)
-			fprintf(fil,
-				"DEVTREE_CONST struct resource %s_res[];\n",
-				ptr->name);
-		if (ptr->children || ptr->multidev)
+	if (ptr->children || device_has_instance(ptr))
 			fprintf(fil, "DEVTREE_CONST struct bus %s_links[];\n",
 				ptr->name);
+}
+
+static void emit_resources(FILE *fil, struct device *ptr)
+{
+	if (ptr->rescnt == 0)
+		return;
+
+	int i = 1;
+	fprintf(fil, "DEVTREE_CONST struct resource %s_res[] = {\n", ptr->name);
+	struct resource *r = ptr->res;
+	while (r) {
+		fprintf(fil,
+			"\t\t{ .flags=IORESOURCE_FIXED | IORESOURCE_ASSIGNED | IORESOURCE_");
+		if (r->type == IRQ)
+			fprintf(fil, "IRQ");
+		if (r->type == DRQ)
+			fprintf(fil, "DRQ");
+		if (r->type == IO)
+			fprintf(fil, "IO");
+		fprintf(fil, ", .index=0x%x, .base=0x%x,", r->index,
+			r->base);
+		if (r->next)
+			fprintf(fil, ".next=&%s_res[%d]},\n", ptr->name,
+				i++);
+		else
+			fprintf(fil, ".next=NULL },\n");
+		r = r->next;
 	}
+
+	fprintf(fil, "\t };\n");
+}
+
+static void emit_dev_children(FILE *fil, struct device *ptr)
+{
+	fprintf(fil, "DEVTREE_CONST struct bus %s_links[] = {\n",
+		ptr->name);
+
+	fprintf(fil, "\t\t[0] = {\n");
+	fprintf(fil, "\t\t\t.link_num = 0,\n");
+	fprintf(fil, "\t\t\t.dev = &%s,\n", ptr->name);
+	fprintf(fil, "\t\t\t.children = &%s,\n", ptr->children->name);
+	fprintf(fil, "\t\t\t.next = NULL,\n");
+	fprintf(fil, "\t\t},\n");
+
+	fprintf(fil, "\t};\n");
+}
+
+static void emit_dev_instances(FILE *fil, struct device *ptr)
+{
+	fprintf(fil, "DEVTREE_CONST struct bus %s_links[] = {\n",
+		ptr->name);
+
+	struct dev_instance *ins = ptr->dev_instance;
+	int count = 0;
+
+	while (ins) {
+		fprintf(fil, "\t\t[%d] = {\n", count);
+		fprintf(fil, "\t\t\t.link_num = %d,\n", count);
+		fprintf(fil, "\t\t\t.dev = &%s,\n", ptr->name);
+		if (ins->children)
+			fprintf(fil, "\t\t\t.children = &%s,\n",
+				ins->children->name);
+		if (ins->next)
+			fprintf(fil, "\t\t\t.next=&%s_links[%d],\n",
+				ptr->name, count + 1);
+		else
+			fprintf(fil,
+				"\t\t\t.next = NULL,\n");
+		fprintf(fil, "\t\t},\n");
+
+		ins = ins->next;
+		count++;
+	}
+
+	fprintf(fil, "\t};\n");
 }
 
 static void pass1(FILE * fil, struct device *ptr)
@@ -494,129 +646,66 @@
 	int pin;
 	struct chip_instance *chip_ins = ptr->chip_instance;
 
-	if (!ptr->used) {
-		if (ptr->id != 0)
-			fprintf(fil, "static ");
-		fprintf(fil, "DEVTREE_CONST struct device %s = {\n",
-			ptr->name);
-		fprintf(fil, "#if !DEVTREE_EARLY\n");
-		fprintf(fil, "\t.ops = %s,\n", (ptr->ops) ? (ptr->ops) : "0");
-		fprintf(fil, "#endif\n");
-		fprintf(fil, "\t.bus = &%s_links[%d],\n", ptr->parent->name,
-			ptr->parent->link);
-		fprintf(fil, "\t.path = {");
-		fprintf(fil, ptr->path, ptr->path_a, ptr->path_b);
-		fprintf(fil, "},\n");
-		fprintf(fil, "\t.enabled = %d,\n", ptr->enabled);
-		fprintf(fil, "\t.on_mainboard = 1,\n");
-		if (ptr->subsystem_vendor > 0)
-			fprintf(fil, "\t.subsystem_vendor = 0x%04x,\n",
-				ptr->subsystem_vendor);
+	if (ptr->id != 0)
+		fprintf(fil, "static ");
+	fprintf(fil, "DEVTREE_CONST struct device %s = {\n", ptr->name);
+	fprintf(fil, "#if !DEVTREE_EARLY\n");
+	fprintf(fil, "\t.ops = %s,\n", (ptr->ops) ? (ptr->ops) : "0");
+	fprintf(fil, "#endif\n");
+	fprintf(fil, "\t.bus = &%s_links[%d],\n", ptr->parent->name,
+		ptr->linknum);
+	fprintf(fil, "\t.path = {");
+	fprintf(fil, ptr->path, ptr->path_a, ptr->path_b);
+	fprintf(fil, "},\n");
+	fprintf(fil, "\t.enabled = %d,\n", ptr->enabled);
+	fprintf(fil, "\t.on_mainboard = 1,\n");
+	if (ptr->subsystem_vendor > 0)
+		fprintf(fil, "\t.subsystem_vendor = 0x%04x,\n",
+			ptr->subsystem_vendor);
 
-		for (pin = 0; pin < 4; pin++) {
-			if (ptr->pci_irq_info[pin].ioapic_irq_pin > 0)
-				fprintf(fil, "\t.pci_irq_info[%d].ioapic_irq_pin = %d,\n",
-					pin, ptr->pci_irq_info[pin].ioapic_irq_pin);
+	for (pin = 0; pin < 4; pin++) {
+		if (ptr->pci_irq_info[pin].ioapic_irq_pin > 0)
+			fprintf(fil, "\t.pci_irq_info[%d].ioapic_irq_pin = %d,\n",
+				pin, ptr->pci_irq_info[pin].ioapic_irq_pin);
 
-			if (ptr->pci_irq_info[pin].ioapic_dst_id > 0)
-				fprintf(fil, "\t.pci_irq_info[%d].ioapic_dst_id = %d,\n",
-					pin, ptr->pci_irq_info[pin].ioapic_dst_id);
-		}
-
-		if (ptr->subsystem_device > 0)
-			fprintf(fil, "\t.subsystem_device = 0x%04x,\n",
-				ptr->subsystem_device);
-
-		if (ptr->rescnt > 0) {
-			fprintf(fil, "\t.resource_list = &%s_res[0],\n",
-				ptr->name);
-		}
-		if (ptr->children || ptr->multidev)
-			fprintf(fil, "\t.link_list = &%s_links[0],\n",
-				ptr->name);
-		else
-			fprintf(fil, "\t.link_list = NULL,\n");
-		if (ptr->sibling)
-			fprintf(fil, "\t.sibling = &%s,\n", ptr->sibling->name);
-		fprintf(fil, "#if !DEVTREE_EARLY\n");
-		fprintf(fil, "\t.chip_ops = &%s_ops,\n", chip_ins->chip->name_underscore);
-		if (chip_ins == &mainboard_instance)
-			fprintf(fil, "\t.name = mainboard_name,\n");
-		fprintf(fil, "#endif\n");
-		if (chip_ins->chip->chiph_exists)
-			fprintf(fil, "\t.chip_info = &%s_info_%d,\n",
-				chip_ins->chip->name_underscore, chip_ins->id);
-		if (ptr->next)
-			fprintf(fil, "\t.next=&%s\n", ptr->next->name);
-		fprintf(fil, "};\n");
+		if (ptr->pci_irq_info[pin].ioapic_dst_id > 0)
+			fprintf(fil, "\t.pci_irq_info[%d].ioapic_dst_id = %d,\n",
+				pin, ptr->pci_irq_info[pin].ioapic_dst_id);
 	}
+
+	if (ptr->subsystem_device > 0)
+		fprintf(fil, "\t.subsystem_device = 0x%04x,\n",
+			ptr->subsystem_device);
+
 	if (ptr->rescnt > 0) {
-		int i = 1;
-		fprintf(fil, "DEVTREE_CONST struct resource %s_res[] = {\n",
+		fprintf(fil, "\t.resource_list = &%s_res[0],\n",
 			ptr->name);
-		struct resource *r = ptr->res;
-		while (r) {
-			fprintf(fil,
-				"\t\t{ .flags=IORESOURCE_FIXED | IORESOURCE_ASSIGNED | IORESOURCE_");
-			if (r->type == IRQ)
-				fprintf(fil, "IRQ");
-			if (r->type == DRQ)
-				fprintf(fil, "DRQ");
-			if (r->type == IO)
-				fprintf(fil, "IO");
-			fprintf(fil, ", .index=0x%x, .base=0x%x,", r->index,
-				r->base);
-			if (r->next)
-				fprintf(fil, ".next=&%s_res[%d]},\n", ptr->name,
-					i++);
-			else
-				fprintf(fil, ".next=NULL },\n");
-			r = r->next;
-		}
-		fprintf(fil, "\t };\n");
 	}
-	if (!ptr->used && (ptr->children || ptr->multidev)) {
-		fprintf(fil, "DEVTREE_CONST struct bus %s_links[] = {\n",
+	if (ptr->children || device_has_instance(ptr))
+		fprintf(fil, "\t.link_list = &%s_links[0],\n",
 			ptr->name);
-		if (ptr->multidev) {
-			struct device *d = ptr;
-			while (d) {
-				if (device_match(d, ptr)) {
-					fprintf(fil, "\t\t[%d] = {\n", d->link);
-					fprintf(fil, "\t\t\t.link_num = %d,\n",
-						d->link);
-					fprintf(fil, "\t\t\t.dev = &%s,\n",
-						d->name);
-					if (d->children)
-						fprintf(fil,
-							"\t\t\t.children = &%s,\n",
-							d->children->name);
-					if (d->next_sibling
-					    && device_match(d->next_sibling,
-							    ptr))
-						fprintf(fil,
-							"\t\t\t.next=&%s_links[%d],\n",
-							d->name, d->link + 1);
-					else
-						fprintf(fil,
-							"\t\t\t.next = NULL,\n");
-					fprintf(fil, "\t\t},\n");
-				}
-				d = d->next_sibling;
-			}
-		} else {
-			if (ptr->children) {
-				fprintf(fil, "\t\t[0] = {\n");
-				fprintf(fil, "\t\t\t.link_num = 0,\n");
-				fprintf(fil, "\t\t\t.dev = &%s,\n", ptr->name);
-				fprintf(fil, "\t\t\t.children = &%s,\n",
-					ptr->children->name);
-				fprintf(fil, "\t\t\t.next = NULL,\n");
-				fprintf(fil, "\t\t},\n");
-			}
-		}
-		fprintf(fil, "\t};\n");
-	}
+	else
+		fprintf(fil, "\t.link_list = NULL,\n");
+	if (ptr->sibling)
+		fprintf(fil, "\t.sibling = &%s,\n", ptr->sibling->name);
+	fprintf(fil, "#if !DEVTREE_EARLY\n");
+	fprintf(fil, "\t.chip_ops = &%s_ops,\n", chip_ins->chip->name_underscore);
+	if (chip_ins == &mainboard_instance)
+		fprintf(fil, "\t.name = mainboard_name,\n");
+	fprintf(fil, "#endif\n");
+	if (chip_ins->chip->chiph_exists)
+		fprintf(fil, "\t.chip_info = &%s_info_%d,\n",
+			chip_ins->chip->name_underscore, chip_ins->id);
+	if (ptr->next)
+		fprintf(fil, "\t.next=&%s\n", ptr->next->name);
+	fprintf(fil, "};\n");
+
+	emit_resources(fil, ptr);
+
+	if (device_has_instance(ptr))
+		emit_dev_instances(fil, ptr);
+	else if(ptr->children)
+		emit_dev_children(fil, ptr);
 }
 
 static void walk_device_tree(FILE * fil, struct device *ptr,
diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h
index 2bb816d..76913c5 100644
--- a/util/sconfig/sconfig.h
+++ b/util/sconfig/sconfig.h
@@ -77,13 +77,18 @@
 	struct chip *next;
 };
 
+struct dev_instance {
+	/* Pointer to first child of this instance. */
+	struct device *children;
+
+	/* Pointer to next instance of the same device. */
+	struct dev_instance *next;
+};
+
 struct device;
 struct device {
 	int id;
 	int enabled;
-	int used;
-	int multidev;
-	int link;
 	int rescnt;
 	int subsystem_vendor;
 	int subsystem_device;
@@ -97,6 +102,12 @@
 	struct pci_irq_info pci_irq_info[4];
 
 	struct device *parent;
+	/*
+	 * Indicates which instance of parent is the true parent of this node,
+	 * if parent is multidev. Else, set to 0.
+	 */
+	int linknum;
+
 	struct device *next;
 	struct device *children;
 	struct device *latestchild;
@@ -105,6 +116,11 @@
 	struct resource *res;
 
 	struct chip_instance *chip_instance;
+
+	/* Handle multiple devices with same id under the same parent. */
+	int linkcnt;
+	struct dev_instance *dev_instance;
+	struct dev_instance *last_instance;
 };
 
 extern struct device *head;
@@ -117,7 +133,6 @@
 			  struct chip_instance *chip_instance,
 			  const int bustype, const char *devnum,
 			  int enabled);
-void alias_siblings(struct device *d);
 void add_resource(struct device *dev, int type, int index, int base);
 void add_pci_subsystem_ids(struct device *dev, int vendor, int device,
 			   int inherit);
diff --git a/util/sconfig/sconfig.tab.c_shipped b/util/sconfig/sconfig.tab.c_shipped
index 3f0e0af..61f3b50 100644
--- a/util/sconfig/sconfig.tab.c_shipped
+++ b/util/sconfig/sconfig.tab.c_shipped
@@ -487,8 +487,8 @@
 static const yytype_uint8 yyrline[] =
 {
        0,    36,    36,    36,    38,    38,    38,    38,    40,    40,
-      40,    40,    40,    40,    42,    42,    51,    51,    61,    64,
-      67,    70,    73
+      40,    40,    40,    40,    42,    42,    51,    51,    60,    63,
+      66,    69,    72
 };
 #endif
 
@@ -1329,7 +1329,6 @@
     {
 	cur_parent = (yyvsp[-2].device)->parent;
 	fold_in((yyvsp[-2].device));
-	alias_siblings((yyvsp[-2].device)->children);
 }
 
     break;
diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y
index a355c77..37ac95f 100755
--- a/util/sconfig/sconfig.y
+++ b/util/sconfig/sconfig.y
@@ -55,7 +55,6 @@
 	devicechildren END {
 	cur_parent = $<device>5->parent;
 	fold_in($<device>5);
-	alias_siblings($<device>5->children);
 };
 
 resource: RESOURCE NUMBER /* == resnum */ EQUALS NUMBER /* == resval */

-- 
To view, visit https://review.coreboot.org/26800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic8c8a73a247e8e992ab6b1b2cc3131e06fa2e5a1
Gerrit-Change-Number: 26800
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180604/c3855887/attachment-0001.html>


More information about the coreboot-gerrit mailing list