util/sconfig: Compare probe conditions for override device match
When the override functionality looks for device match, check that
the probe list for both the devices matches exactly if probe list
exists for the base device. This ensures that if there are two devices
with same identity (e.g. I2C address or USB port #) but using
different properties (registers) controlled by different probe
statements, then the two devices are not incorrectly matched as the
same device.
The check for base device having a probe list is performed before
comparing the probe lists because a base device might not really have
any probe requirements at all. So, when overriding such a device,
there is no need to check for the probe list match.
BUG=b:187193527
TEST=Verified by adding two I2C devices in the override tree with the
same I2C address and chip but different probe statements and confirmed
that both the devices are present in generated static.c file.
Change-Id: Ib18868b336cf4ffc9aa38aee7c6f333a35d32fce
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57111
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Weimin Wu <wuweimin@huaqin.corp-partner.google.com>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
diff --git a/util/sconfig/main.c b/util/sconfig/main.c
index 91c079a..f7d1d10 100644
--- a/util/sconfig/main.c
+++ b/util/sconfig/main.c
@@ -552,17 +552,26 @@
}
}
+static int check_probe_exists(struct fw_config_probe *probe, const char *field,
+ const char *option)
+{
+ while (probe) {
+ if (!strcmp(probe->field, field) && !strcmp(probe->option, option)) {
+ return 1;
+ }
+ probe = probe->next;
+ }
+
+ return 0;
+}
+
void add_fw_config_probe(struct bus *bus, const char *field, const char *option)
{
struct fw_config_probe *probe;
- probe = bus->dev->probe;
- while (probe) {
- if (!strcmp(probe->field, field) && !strcmp(probe->option, option)) {
- printf("ERROR: fw_config probe %s:%s already exists\n", field, option);
- exit(1);
- }
- probe = probe->next;
+ if (check_probe_exists(bus->dev->probe, field, option)) {
+ printf("ERROR: fw_config probe %s:%s already exists\n", field, option);
+ exit(1);
}
probe = S_ALLOC(sizeof(*probe));
@@ -1479,6 +1488,56 @@
fclose(filec);
}
+static int device_probe_count(struct fw_config_probe *probe)
+{
+ int count = 0;
+ while (probe) {
+ probe = probe->next;
+ count++;
+ }
+
+ return count;
+}
+
+/*
+ * When overriding devices, use the following rules:
+ * 1. If probe count matches and:
+ * a. Entire probe list matches for both devices -> Same device, override.
+ * b. No probe entries match -> Different devices, do not override.
+ * c. Partial list matches -> Bad device tree entries, fail build.
+ *
+ * 2. If probe counts do not match and:
+ * a. No probe entries match -> Different devices, do not override.
+ * b. Partial list matches -> Bad device tree entries, fail build.
+ */
+static int device_probes_match(struct device *a, struct device *b)
+{
+ struct fw_config_probe *a_probe = a->probe;
+ struct fw_config_probe *b_probe = b->probe;
+ int a_probe_count = device_probe_count(a_probe);
+ int b_probe_count = device_probe_count(b_probe);
+ int match_count = 0;
+
+ while (a_probe) {
+ if (check_probe_exists(b_probe, a_probe->field, a_probe->option))
+ match_count++;
+ a_probe = a_probe->next;
+ }
+
+ if ((a_probe_count == b_probe_count) && (a_probe_count == match_count))
+ return 1;
+
+ if (match_count) {
+ printf("ERROR: devices with overlapping probes: ");
+ printf(a->path, a->path_a, a->path_b);
+ printf(b->path, b->path_a, b->path_b);
+ printf("\n");
+ exit(1);
+ }
+
+ return 0;
+}
+
/*
* Match device nodes from base and override tree to see if they are the same
* node.
@@ -1803,7 +1862,16 @@
/* Look for a matching device in base tree. */
for (base_child = base_parent->children;
base_child; base_child = base_child->sibling) {
- if (device_match(base_child, override_child))
+ if (!device_match(base_child, override_child))
+ continue;
+ /* If base device has no probe statement, nothing else to compare. */
+ if (base_child->probe == NULL)
+ break;
+ /*
+ * If base device has probe statements, ensure that all probe conditions
+ * match for base and override device.
+ */
+ if (device_probes_match(base_child, override_child))
break;
}