Change in coreboot[master]: device/resource_allocator_v4.c: Loop over struct bus linked list

[Arthur Heymans (Code Review)]

Attention is currently required from: Arthur Heymans.
Hello Arthur Heymans,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/62967

to review the following change.

Change subject: device/resource_allocator_v4.c: Loop over struct bus linked list
......................................................................

device/resource_allocator_v4.c: Loop over struct bus linked list

The allocator code assumes only one downstream bus per device on
.link_list. Even though this is the most common setup this would be
consistent with the rest of device.c.

Change-Id: Ie4eec48c7008c19896996a40b1fa9b09008b3354
Signed-off-by: Arthur Heymans 
---
M src/device/resource_allocator_common.c
M src/device/resource_allocator_v4.c
2 files changed, 35 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/62967/1

diff --git a/src/device/resource_allocator_common.c b/src/device/resource_allocator_common.c
index 1cb1895..bf5ceef 100644
--- a/src/device/resource_allocator_common.c
+++ b/src/device/resource_allocator_common.c
@@ -40,7 +40,7 @@
    }
 }

-const struct device *largest_resource(struct bus *bus,
+const struct device *largest_resource(struct bus *bus_list,
    			      struct resource **result_res,
    			      unsigned long type_mask,
    			      unsigned long type)
@@ -52,8 +52,9 @@
    state.result = NULL;
    state.seen_last = 0;

-	search_bus_resources(bus, type_mask, type, pick_largest_resource,
-			     &state);
+	for (struct bus *bus = bus_list; bus; bus = bus->next)
+		search_bus_resources(bus, type_mask, type, pick_largest_resource,
+				     &state);

*result_res = state.result;
    return state.result_dev;
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index 6f8159e..2a21e0b 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -55,7 +55,7 @@
    resource_t base;
    bool first_child_res = true;
    const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH;
-	struct bus *bus = bridge->link_list;
+	struct bus *bus_list = bridge->link_list;

child_res = NULL;

@@ -73,7 +73,7 @@
           dev_path(bridge), resource2str(bridge_res), bridge_res->size,
           bridge_res->align, bridge_res->gran, bridge_res->limit);

-	while ((child = largest_resource(bus, &child_res, type_mask, type_match))) {
+	while ((child = largest_resource(bus_list, &child_res, type_mask, type_match))) {

/* Size 0 resources can be skipped. */
    	if (!child_res->size)
@@ -153,7 +153,7 @@
 {
    const struct device *child;
    struct resource *res;
-	struct bus *bus = bridge->link_list;
+	struct bus *bus;
    const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH;

for (res = bridge->resource_list; res; res = res->next) {
@@ -167,10 +167,12 @@
    	 * Ensure that the resource requirements for all downstream bridges are
    	 * gathered before updating the window for current bridge resource.
    	 */
-		for (child = bus->children; child; child = child->sibling) {
-			if (!dev_has_children(child))
-				continue;
-			compute_bridge_resources(child, type_match, print_depth + 1);
+		for (bus = bridge->link_list; bus; bus = bus->next) {
+			for (child = bus->children; child; child = child->sibling) {
+				if (!dev_has_children(child))
+					continue;
+				compute_bridge_resources(child, type_match, print_depth + 1);
+			}
    	}

/*
@@ -202,16 +204,17 @@
    if (domain->link_list == NULL)
    	return;

-	for (child = domain->link_list->children; child; child = child->sibling) {
+	for (struct bus *bus = domain->link_list; bus; bus = bus->next) {
+		for (child = bus->children; child; child = child->sibling) {
+			/* Skip if this is not a bridge or has no children under it. */
+			if (!dev_has_children(child))
+				continue;

-		/* Skip if this is not a bridge or has no children under it. */
-		if (!dev_has_children(child))
-			continue;
-
-		compute_bridge_resources(child, IORESOURCE_IO, print_depth);
-		compute_bridge_resources(child, IORESOURCE_MEM, print_depth);
-		compute_bridge_resources(child, IORESOURCE_MEM | IORESOURCE_PREFETCH,
+			compute_bridge_resources(child, IORESOURCE_IO, print_depth);
+			compute_bridge_resources(child, IORESOURCE_MEM, print_depth);
+			compute_bridge_resources(child, IORESOURCE_MEM | IORESOURCE_PREFETCH,
    				 print_depth);
+		}
    }
 }

@@ -362,13 +365,13 @@
  * order until all resources of given type are allocated address space within the current
  * resource window.
  */
-static void allocate_child_resources(struct bus *bus, struct memranges *ranges,
+static void allocate_child_resources(struct bus *bus_list, struct memranges *ranges,
    			     unsigned long type_mask, unsigned long type_match)
 {
    struct resource *resource = NULL;
    const struct device *dev;

-	while ((dev = largest_resource(bus, &resource, type_mask, type_match))) {
+	while ((dev = largest_resource(bus_list, &resource, type_mask, type_match))) {

if (!resource->size)
    		continue;
@@ -427,12 +430,9 @@
    	update_constraints(ranges, dev, res);
    }

-	bus = dev->link_list;
-	if (bus == NULL)
-		return;
-
-	for (child = bus->children; child != NULL; child = child->sibling)
-		avoid_fixed_resources(ranges, child, mask_match);
+	for (bus = dev->link_list; bus; bus = bus->next)
+		for (child = bus->children; child != NULL; child = child->sibling)
+			avoid_fixed_resources(ranges, child, mask_match);
 }

static void constrain_domain_resources(const struct device *domain, struct memranges *ranges,
@@ -512,7 +512,7 @@
 {
    struct memranges ranges;
    const struct resource *res;
-	struct bus *bus = bridge->link_list;
+	struct bus *bus;
    unsigned long type_match;
    struct device *child;
    const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH;
@@ -527,15 +527,17 @@
    	type_match = res->flags & type_mask;

setup_resource_ranges(bridge, res, type_match, &ranges);
-		allocate_child_resources(bus, &ranges, type_mask, type_match);
+		allocate_child_resources(bridge->link_list, &ranges, type_mask, type_match);
    	cleanup_resource_ranges(bridge, &ranges, res);
    }

-	for (child = bus->children; child; child = child->sibling) {
-		if (!dev_has_children(child))
-			continue;
+	for (bus = bridge->link_list; bus; bus = bus->next) {
+		for (child = bus->children; child; child = child->sibling) {
+			if (!dev_has_children(child))
+				continue;

-		allocate_bridge_resources(child);
+			allocate_bridge_resources(child);
+		}
    }
 }