Change in coreboot[master]: device: Add helper function to find matching device on bus

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

device: Add helper function to find matching device on bus

This change adds a helper function dev_find_matching_device_on_bus()
which scans all the child devices on the given bus and calls a
function provided by the caller to return the device that matches the
caller's criteria.

Change-Id: I2e3332c0a175ab995c523f078f29a9f498f17931
Signed-off-by: Furquan Shaikh 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 22 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/40543/1

diff --git a/src/device/device_const.c b/src/device/device_const.c
index c59c5e9..a38cb7e 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -69,6 +69,19 @@
    return result;
 }

+DEVTREE_CONST struct device *dev_find_matching_device_on_bus(const struct bus *bus,
+							match_device_fn fn)
+{
+	struct device *child = NULL;
+
+	while ((child = dev_bus_each_child(bus, child)) != NULL) {
+		if (fn(child))
+			break;
+	}
+
+	return child;
+}
+
 /**
  * Given a device pointer, find the next PCI device.
  *
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 4e9c594..2cefecc 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -207,6 +207,15 @@
 struct device *dev_find_lapic(unsigned int apic_id);
 int dev_count_cpu(void);

+/*
+ * Signature for matching function that is used by dev_find_matching_device_on_bus() to decide
+ * if the device being considered is the one that matches the caller's criteria. This function
+ * is supposed to return true if the provided device matches the criteria, else false.
+ */
+typedef bool (*match_device_fn)(DEVTREE_CONST struct device *dev);
+DEVTREE_CONST struct device *dev_find_matching_device_on_bus(const struct bus *bus,
+							match_device_fn fn);
+
 struct device *add_cpu_device(struct bus *cpu_bus, unsigned int apic_id,
    			int enabled);
 void set_cpu_topology(struct device *cpu, unsigned int node,

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG@10 
PS2, Line 10: and calls a
            : function provided by the caller to return the device that matches the
            : caller's criteria.
WDYT: "and returns the first device that the function returns true for?"

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG@10 
PS2, Line 10: and calls a
            : function provided by the caller to return the device that matches the
            : caller's criteria.
> WDYT: "and returns the first device that the function returns true for?"

Makes sense. I will update the commit message and comment.

[Furquan Shaikh (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: device: Add helper function to find matching device on bus
......................................................................

device: Add helper function to find matching device on bus

This change adds a helper function dev_find_matching_device_on_bus()
which scans all the child devices on the given bus and calls a
match function provided by the caller. It returns the first device
that the match function returns true for, else NULL if no such device
is found.

Change-Id: I2e3332c0a175ab995c523f078f29a9f498f17931
Signed-off-by: Furquan Shaikh 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/40543/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40543/2//COMMIT_MSG@10 
PS2, Line 10: and calls a
            : function provided by the caller to return the device that matches the
            : caller's criteria.
> Makes sense. I will update the commit message and comment.

Done

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/40543/3/src/device/device_const.c 
File src/device/device_const.c:

https://review.coreboot.org/c/coreboot/+/40543/3/src/device/device_const.c@7... 
PS3, Line 75: 	struct device *child = NULL;
DEVTREE_CONST

src/device/device_const.c: In function 'dev_find_matching_device_on_bus':
src/device/device_const.c:77:16: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  while ((child = dev_bus_each_child(bus, child)) != NULL) {
                ^

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40543/3/src/device/device_const.c 
File src/device/device_const.c:

https://review.coreboot.org/c/coreboot/+/40543/3/src/device/device_const.c@7... 
PS3, Line 75: 	struct device *child = NULL;
> DEVTREE_CONST […]

Done

[Furquan Shaikh (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: device: Add helper function to find matching device on bus
......................................................................

device: Add helper function to find matching device on bus

This change adds a helper function dev_find_matching_device_on_bus()
which scans all the child devices on the given bus and calls a
match function provided by the caller. It returns the first device
that the match function returns true for, else NULL if no such device
is found.

Change-Id: I2e3332c0a175ab995c523f078f29a9f498f17931
Signed-off-by: Furquan Shaikh 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/40543/4

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 4: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 5: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

device: Add helper function to find matching device on bus

This change adds a helper function dev_find_matching_device_on_bus()
which scans all the child devices on the given bus and calls a
match function provided by the caller. It returns the first device
that the match function returns true for, else NULL if no such device
is found.

Change-Id: I2e3332c0a175ab995c523f078f29a9f498f17931
Signed-off-by: Furquan Shaikh 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40543
Reviewed-by: Aaron Durbin 
Reviewed-by: Tim Wawrzynczak 
Tested-by: build bot (Jenkins) 
---
M src/device/device_const.c
M src/include/device/device.h
2 files changed, 26 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/device/device_const.c b/src/device/device_const.c
index de404dc..add253b 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -70,6 +70,19 @@
    return result;
 }

+DEVTREE_CONST struct device *dev_find_matching_device_on_bus(const struct bus *bus,
+							match_device_fn fn)
+{
+	DEVTREE_CONST struct device *child = NULL;
+
+	while ((child = dev_bus_each_child(bus, child)) != NULL) {
+		if (fn(child))
+			break;
+	}
+
+	return child;
+}
+
 /**
  * Given a device pointer, find the next PCI device.
  *
diff --git a/src/include/device/device.h b/src/include/device/device.h
index a33702a..4983b48 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -207,6 +207,19 @@
 struct device *dev_find_lapic(unsigned int apic_id);
 int dev_count_cpu(void);

+/*
+ * Signature for matching function that is used by dev_find_matching_device_on_bus() to decide
+ * if the device being considered is the one that matches the caller's criteria. This function
+ * is supposed to return true if the provided device matches the criteria, else false.
+ */
+typedef bool (*match_device_fn)(DEVTREE_CONST struct device *dev);
+/*
+ * Returns the first device on the bus that the match_device_fn returns true for. If no such
+ * device is found, it returns NULL.
+ */
+DEVTREE_CONST struct device *dev_find_matching_device_on_bus(const struct bus *bus,
+							match_device_fn fn);
+
 struct device *add_cpu_device(struct bus *cpu_bus, unsigned int apic_id,
    			int enabled);
 void set_cpu_topology(struct device *cpu, unsigned int node,

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40543 )

Change subject: device: Add helper function to find matching device on bus
......................................................................

Patch Set 6:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2543
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2542
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2541
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/2540

Please note: This test is under development and might not be accurate at all!