bootstd: Switch bootdev scanning to use labels

At present we set up the bootdev order at the start, then scan the
bootdevs one by one.

However this approach cannot be used with hunters, since the bootdevs may
not exist until the hunter is used. Nor can we just run all the hunters at
the start, since that violate's U-Boot's 'lazy init' requirement. It also
increases boot time.

So we need to adjust the algorithm to scan by labels instead. As a first
step, drop the dev_order[] array in favour of a list of labels. Update the
name of bootdev_setup_iter_order() to better reflect what it does.

Update some related comments and log messages. Also disable a few tests
until a later commit where we can use them.

Signed-off-by: Simon Glass <sjg@chromium.org>

6 files changed, 147 insertions, 152 deletions

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c
index 63005140061..334be7662a1 100644
--- a/boot/bootdev-uclass.c
+++ b/boot/bootdev-uclass.c
@@ -649,96 +649,12 @@ int bootdev_next_prio(struct bootflow_iter *iter, struct udevice **devp)
 	return 0;
 }
 
-/**
- * h_cmp_bootdev() - Compare two bootdevs to find out which should go first
- *
- * @v1: struct udevice * of first bootdev device
- * @v2: struct udevice * of second bootdev device
- * Return: sort order (<0 if dev1 < dev2, ==0 if equal, >0 if dev1 > dev2)
- */
-static int h_cmp_bootdev(const void *v1, const void *v2)
-{
-	const struct udevice *dev1 = *(struct udevice **)v1;
-	const struct udevice *dev2 = *(struct udevice **)v2;
-	const struct bootdev_uc_plat *ucp1 = dev_get_uclass_plat(dev1);
-	const struct bootdev_uc_plat *ucp2 = dev_get_uclass_plat(dev2);
-	int diff;
-
-	/* Use priority first */
-	diff = ucp1->prio - ucp2->prio;
-	if (diff)
-		return diff;
-
-	/* Fall back to seq for devices of the same priority */
-	diff = dev_seq(dev1) - dev_seq(dev2);
-
-	return diff;
-}
-
-/**
- * build_order() - Build the ordered list of bootdevs to use
- *
- * This builds an ordered list of devices by one of three methods:
- * - using the boot_targets environment variable, if non-empty
- * - using the bootdev-order devicetree property, if present
- * - sorted by priority and sequence number
- *
- * @bootstd: BOOTSTD device to use
- * @order: Bootdevs listed in default order
- * @max_count: Number of entries in @order
- * Return: number of bootdevs found in the ordering, or -E2BIG if the
- * boot_targets string is too long, or -EXDEV if the ordering produced 0 results
- */
-static int build_order(struct udevice *bootstd, struct udevice **order,
-		       int max_count)
-{
-	const char *overflow_target = NULL;
-	const char *const *labels;
-	struct udevice *dev;
-	int i, ret, count;
-	bool ok;
-
-	labels = bootstd_get_bootdev_order(bootstd, &ok);
-	if (!ok)
-		return log_msg_ret("ord", -ENOMEM);
-	if (labels) {
-		int upto;
-
-		upto = 0;
-		for (i = 0; labels[i]; i++) {
-			ret = bootdev_find_by_label(labels[i], &dev, NULL);
-			if (!ret) {
-				if (upto == max_count) {
-					overflow_target = labels[i];
-					break;
-				}
-				order[upto++] = dev;
-			}
-		}
-		count = upto;
-	} else {
-		/* sort them into priority order */
-		count = max_count;
-		qsort(order, count, sizeof(struct udevice *), h_cmp_bootdev);
-	}
-
-	if (overflow_target) {
-		log_warning("Expected at most %d bootdevs, but overflowed with boot_target '%s'\n",
-			    max_count, overflow_target);
-	}
-
-	if (!count)
-		return log_msg_ret("targ", -EXDEV);
-
-	return count;
-}
-
-int bootdev_setup_iter_order(struct bootflow_iter *iter, struct udevice **devp)
+int bootdev_setup_iter(struct bootflow_iter *iter, struct udevice **devp,
+		       int *method_flagsp)
 {
-	struct udevice *bootstd, *dev = *devp, **order;
+	struct udevice *bootstd, *dev = *devp;
 	bool show = iter->flags & BOOTFLOWF_SHOW;
-	struct uclass *uc;
-	int count, upto;
+	int method_flags;
 	int ret;
 
 	ret = uclass_first_device_err(UCLASS_BOOTSTD, &bootstd);
@@ -757,39 +673,33 @@ int bootdev_setup_iter_order(struct bootflow_iter *iter, struct udevice **devp)
 	/* Handle scanning a single device */
 	if (dev) {
 		iter->flags |= BOOTFLOWF_SINGLE_DEV;
-		return 0;
-	}
-
-	count = uclass_id_count(UCLASS_BOOTDEV);
-	if (!count)
-		return log_msg_ret("count", -ENOENT);
-
-	order = calloc(count, sizeof(struct udevice *));
-	if (!order)
-		return log_msg_ret("order", -ENOMEM);
-
-	/* Get the list of bootdevs */
-	uclass_id_foreach_dev(UCLASS_BOOTDEV, dev, uc)
-		order[upto++] = dev;
-	log_debug("Found %d bootdevs\n", count);
-	if (upto != count)
-		log_debug("Expected %d bootdevs, found %d using aliases\n",
-			  count, upto);
-
-	ret = build_order(bootstd, order, upto);
-	if (ret < 0) {
-		free(order);
-		return log_msg_ret("build", ret);
+		log_debug("Selected boodev: %s\n", dev->name);
+		method_flags = 0;
+	} else {
+		bool ok;
+
+		/* This either returns a non-empty list or NULL */
+		iter->labels = bootstd_get_bootdev_order(bootstd, &ok);
+		if (!ok)
+			return log_msg_ret("ord", -ENOMEM);
+		log_debug("setup labels %p\n", iter->labels);
+		if (iter->labels) {
+			iter->cur_label = -1;
+			ret = bootdev_next_label(iter, &dev, &method_flags);
+		} else {
+			ret = bootdev_next_prio(iter, &dev);
+			method_flags = 0;
+		}
+		if (!dev)
+			return log_msg_ret("fin", -ENOENT);
+		log_debug("Selected bootdev: %s\n", dev->name);
 	}
 
-	iter->num_devs = ret;
-	iter->dev_order = order;
-	iter->cur_dev = 0;
-
-	dev = *order;
 	ret = device_probe(dev);
 	if (ret)
 		return log_msg_ret("probe", ret);
+	if (method_flagsp)
+		*method_flagsp = method_flags;
 	*devp = dev;
 
 	return 0;
diff --git a/boot/bootflow.c b/boot/bootflow.c
index d2dbc9d4450..32e2aad470d 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -92,7 +92,6 @@ void bootflow_iter_init(struct bootflow_iter *iter, int flags)
 
 void bootflow_iter_uninit(struct bootflow_iter *iter)
 {
-	free(iter->dev_order);
 	free(iter->method_order);
 }
 
@@ -113,12 +112,25 @@ int bootflow_iter_drop_bootmeth(struct bootflow_iter *iter,
 	return 0;
 }
 
+/**
+ * bootflow_iter_set_dev() - switch to the next bootdev when iterating
+ *
+ * This sets iter->dev, records the device in the dev_used[] list and shows a
+ * message if required
+ *
+ * @iter: Iterator to update
+ * @dev: Bootdev to use, or NULL if there are no more
+ */
 static void bootflow_iter_set_dev(struct bootflow_iter *iter,
-				  struct udevice *dev)
+				  struct udevice *dev, int method_flags)
 {
 	struct bootmeth_uc_plat *ucp = dev_get_uclass_plat(iter->method);
 
+	log_debug("iter: Setting dev to %s, flags %x\n",
+		  dev ? dev->name : "(none)", method_flags);
 	iter->dev = dev;
+	iter->method_flags = method_flags;
+
 	if ((iter->flags & (BOOTFLOWF_SHOW | BOOTFLOWF_SINGLE_DEV)) ==
 	    BOOTFLOWF_SHOW) {
 		if (dev)
@@ -144,6 +156,7 @@ static int iter_incr(struct bootflow_iter *iter)
 	bool global;
 	int ret;
 
+	log_debug("entry: err=%d\n", iter->err);
 	global = iter->doing_global;
 
 	if (iter->err == BF_NO_MORE_DEVICES)
@@ -182,7 +195,7 @@ static int iter_incr(struct bootflow_iter *iter)
 			return 0;
 	}
 
-	/* No more partitions; start at the first one and...*/
+	/* No more partitions; start at the first one and... */
 	iter->part = 0;
 
 	/*
@@ -196,16 +209,32 @@ static int iter_incr(struct bootflow_iter *iter)
 	if (iter->flags & BOOTFLOWF_SINGLE_DEV) {
 		ret = -ENOENT;
 	} else {
-		if (inc_dev)
-			iter->cur_dev++;
-		if (iter->cur_dev == iter->num_devs) {
-			ret = -ENOENT;
-			bootflow_iter_set_dev(iter, NULL);
+		int method_flags;
+
+		ret = 0;
+		dev = iter->dev;
+		log_debug("inc_dev=%d\n", inc_dev);
+		if (!inc_dev) {
+			ret = bootdev_setup_iter(iter, &dev, &method_flags);
+		} else {
+			log_debug("labels %p\n", iter->labels);
+			if (iter->labels) {
+				ret = bootdev_next_label(iter, &dev,
+							 &method_flags);
+			} else {
+				ret = bootdev_next_prio(iter, &dev);
+				method_flags = 0;
+			}
+		}
+		log_debug("ret=%d, dev=%p %s\n", ret, dev,
+			  dev ? dev->name : "none");
+		if (ret) {
+			bootflow_iter_set_dev(iter, NULL, 0);
 		} else {
-			dev = iter->dev_order[iter->cur_dev];
 			ret = device_probe(dev);
+			log_debug("probe %s %d\n", dev->name, ret);
 			if (!log_msg_ret("probe", ret))
-				bootflow_iter_set_dev(iter, dev);
+				bootflow_iter_set_dev(iter, dev, method_flags);
 		}
 	}
 
@@ -230,7 +259,7 @@ static int bootflow_check(struct bootflow_iter *iter, struct bootflow *bflow)
 	int ret;
 
 	if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && iter->doing_global) {
-		bootflow_iter_set_dev(iter, NULL);
+		bootflow_iter_set_dev(iter, NULL, 0);
 		ret = bootmeth_get_bootflow(iter->method, bflow);
 		if (ret)
 			return log_msg_ret("glob", ret);
@@ -274,18 +303,27 @@ int bootflow_scan_bootdev(struct udevice *dev, struct bootflow_iter *iter,
 		flags |= BOOTFLOWF_SKIP_GLOBAL;
 	bootflow_iter_init(iter, flags);
 
-	ret = bootdev_setup_iter_order(iter, &dev);
-	if (ret)
-		return log_msg_ret("obdev", -ENODEV);
-
+	/*
+	 * Set up the ordering of bootmeths. This sets iter->doing_global and
+	 * iter->first_glob_method if we are starting with the global bootmeths
+	 */
 	ret = bootmeth_setup_iter_order(iter, !(flags & BOOTFLOWF_SKIP_GLOBAL));
 	if (ret)
 		return log_msg_ret("obmeth", -ENODEV);
 
 	/* Find the first bootmeth (there must be at least one!) */
 	iter->method = iter->method_order[iter->cur_method];
-	if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) || !iter->doing_global)
-		bootflow_iter_set_dev(iter, dev);
+
+	if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) || !iter->doing_global) {
+		struct udevice *dev = NULL;
+		int method_flags;
+
+		ret = bootdev_setup_iter(iter, &dev, &method_flags);
+		if (ret)
+			return log_msg_ret("obdev", -ENODEV);
+
+		bootflow_iter_set_dev(iter, dev, method_flags);
+	}
 
 	ret = bootflow_check(iter, bflow);
 	if (ret) {
diff --git a/include/bootdev.h b/include/bootdev.h
index 4b6a8eb8d8f..8fa67487c63 100644
--- a/include/bootdev.h
+++ b/include/bootdev.h
@@ -265,21 +265,22 @@ int bootdev_find_by_any(const char *name, struct udevice **devp,
 			int *method_flagsp);
 
 /**
- * bootdev_setup_iter_order() - Set up the ordering of bootdevs to scan
+ * bootdev_setup_iter() - Set up iteration through bootdevs
  *
- * This sets up the ordering information in @iter, based on the priority of each
- * bootdev and the bootdev-order property in the bootstd node
- *
- * If a single device is requested, no ordering is needed
+ * This sets up the an interation, based on the priority of each bootdev, the
+ * bootdev-order property in the bootstd node (or the boot_targets env var).
  *
  * @iter: Iterator to update with the order
  * @devp: On entry, *devp is NULL to scan all, otherwise this is the (single)
  *	device to scan. Returns the first device to use, which is the passed-in
  *	@devp if it was non-NULL
+ * @method_flagsp: If non-NULL, returns any flags implied by the label
+ * (enum bootflow_meth_flags_t), 0 if none
  * Return: 0 if OK, -ENOENT if no bootdevs, -ENOMEM if out of memory, other -ve
  *	on other error
  */
-int bootdev_setup_iter_order(struct bootflow_iter *iter, struct udevice **devp);
+int bootdev_setup_iter(struct bootflow_iter *iter, struct udevice **devp,
+		       int *method_flagsp);
 
 /**
  * bootdev_list_hunters() - List the available bootdev hunters
diff --git a/include/bootflow.h b/include/bootflow.h
index 69ac90483c6..bdb37352ab9 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -163,7 +163,8 @@ enum bootflow_meth_flags_t {
  * @flags: Flags to use (see enum bootflow_flags_t). If BOOTFLOWF_GLOBAL_FIRST is
  *	enabled then the global bootmeths are being scanned, otherwise we have
  *	moved onto the bootdevs
- * @dev: Current bootdev, NULL if none
+ * @dev: Current bootdev, NULL if none. This is only ever updated in
+ * bootflow_iter_set_dev()
  * @part: Current partition number (0 for whole device)
  * @method: Current bootmeth
  * @max_part: Maximum hardware partition number in @dev, 0 if there is no
@@ -173,9 +174,6 @@ enum bootflow_meth_flags_t {
  *	forward (e.g. to skip the current partition because it is not valid)
  *	-ESHUTDOWN: try next bootdev
  * @num_devs: Number of bootdevs in @dev_order
- * @cur_dev: Current bootdev number, an index into @dev_order[]
- * @dev_order: List of bootdevs to scan, in order of priority. The scan starts
- *	with the first one on the list
  * @labels: List of labels to scan for bootdevs
  * @cur_label: Current label being processed
  * @num_methods: Number of bootmeth devices in @method_order
@@ -198,8 +196,6 @@ struct bootflow_iter {
 	int first_bootable;
 	int err;
 	int num_devs;
-	int cur_dev;
-	struct udevice **dev_order;
 	const char *const *labels;
 	int cur_label;
 	int num_methods;
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 8ebc27a6435..679ffc4d8df 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -186,6 +186,7 @@ static int bootdev_test_any(struct unit_test_state *uts)
 BOOTSTD_TEST(bootdev_test_any, UT_TESTF_DM | UT_TESTF_SCAN_FDT |
 	     UT_TESTF_ETH_BOOTDEV);
 
+#if 0 /* disable for now */
 /* Check bootdev ordering with the bootdev-order property */
 static int bootdev_test_order(struct unit_test_state *uts)
 {
@@ -290,6 +291,7 @@ static int bootdev_test_prio(struct unit_test_state *uts)
 	return 0;
 }
 BOOTSTD_TEST(bootdev_test_prio, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+#endif
 
 /* Check listing hunters */
 static int bootdev_test_hunter(struct unit_test_state *uts)
@@ -402,6 +404,25 @@ static int bootdev_test_cmd_hunt(struct unit_test_state *uts)
 BOOTSTD_TEST(bootdev_test_cmd_hunt, UT_TESTF_DM | UT_TESTF_SCAN_FDT |
 	     UT_TESTF_ETH_BOOTDEV);
 
+/* Check searching for bootdevs using the hunters */
+static int bootdev_test_hunt_scan(struct unit_test_state *uts)
+{
+	struct bootflow_iter iter;
+	struct bootstd_priv *std;
+	struct bootflow bflow;
+
+	/* get access to the used hunters */
+	ut_assertok(bootstd_get_priv(&std));
+
+	ut_assertok(bootstd_test_drop_bootdev_order(uts));
+	ut_assertok(bootflow_scan_first(&iter, BOOTFLOWF_SHOW | BOOTFLOWF_HUNT |
+					BOOTFLOWF_SKIP_GLOBAL, &bflow));
+	ut_asserteq(BIT(MMC_HUNTER) | BIT(1), std->hunters_used);
+
+	return 0;
+}
+BOOTSTD_TEST(bootdev_test_hunt_scan, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
 /* Check that only bootable partitions are processed */
 static int bootdev_test_bootable(struct unit_test_state *uts)
 {
@@ -640,7 +661,6 @@ static int bootdev_test_next_prio(struct unit_test_state *uts)
 	ut_assert_nextline("Hunting with: mmc");
 	ut_assert_console_end();
 
-	/* extension in second in the list , so bit 1 */
 	ut_asserteq(BIT(MMC_HUNTER) | BIT(1), std->hunters_used);
 
 	ut_assertok(bootdev_next_prio(&iter, &dev));
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 3a65d06696b..1a2c54c1119 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -55,7 +55,10 @@ static int bootflow_cmd(struct unit_test_state *uts)
 	ut_assert_nextline("Scanning for bootflows in bootdev 'mmc1.bootdev'");
 	ut_assert_nextline("Seq  Method       State   Uclass    Part  Name                      Filename");
 	ut_assert_nextlinen("---");
+	ut_assert_nextline("Scanning bootdev 'mmc2.bootdev':");
+	ut_assert_nextline("Scanning bootdev 'mmc1.bootdev':");
 	ut_assert_nextline("  0  syslinux     ready   mmc          1  mmc1.bootdev.part_1       /extlinux/extlinux.conf");
+	ut_assert_nextline("No more bootdevs");
 	ut_assert_nextlinen("---");
 	ut_assert_nextline("(1 bootflow, 1 valid)");
 	ut_assert_console_end();
@@ -73,28 +76,55 @@ static int bootflow_cmd(struct unit_test_state *uts)
 }
 BOOTSTD_TEST(bootflow_cmd, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 
-/* Check 'bootflow scan' with a name / label / seq */
+#if 0 /* disable for now */
+/* Check 'bootflow scan' with a label / seq */
 static int bootflow_cmd_label(struct unit_test_state *uts)
 {
+	test_set_eth_enable(false);
+
 	console_record_reset_enable();
 	ut_assertok(run_command("bootflow scan -lH mmc1", 0));
-	ut_assert_nextline("Scanning for bootflows in bootdev 'mmc1.bootdev'");
+	ut_assert_nextline("Scanning for bootflows with label 'mmc1'");
 	ut_assert_skip_to_line("(1 bootflow, 1 valid)");
 	ut_assert_console_end();
 
-	ut_assertok(run_command("bootflow scan -lH mmc0.bootdev", 0));
-	ut_assert_nextline("Scanning for bootflows in bootdev 'mmc0.bootdev'");
+	ut_assertok(run_command("bootflow scan -lH 0", 0));
+	ut_assert_nextline("Scanning for bootflows with label '0'");
 	ut_assert_skip_to_line("(0 bootflows, 0 valid)");
 	ut_assert_console_end();
 
+	/*
+	 * with ethernet enabled we have 8 devices ahead of the mmc ones:
+	 *
+	 * ut_assertok(run_command("bootdev list", 0));
+	 * Seq  Probed  Status  Uclass    Name
+	 * ---  ------  ------  --------  ------------------
+	 * 0   [ + ]      OK  ethernet  eth@10002000.bootdev
+	 * 1   [   ]      OK  ethernet  eth@10003000.bootdev
+	 * 2   [   ]      OK  ethernet  sbe5.bootdev
+	 * 3   [   ]      OK  ethernet  eth@10004000.bootdev
+	 * 4   [   ]      OK  ethernet  phy-test-eth.bootdev
+	 * 5   [   ]      OK  ethernet  dsa-test-eth.bootdev
+	 * 6   [   ]      OK  ethernet  dsa-test@0.bootdev
+	 * 7   [   ]      OK  ethernet  dsa-test@1.bootdev
+	 * 8   [   ]      OK  mmc       mmc2.bootdev
+	 * 9   [ + ]      OK  mmc       mmc1.bootdev
+	 * a   [   ]      OK  mmc       mmc0.bootdev
+	 */
+	ut_assertok(run_command("bootflow scan -lH 9", 0));
+	ut_assert_nextline("Scanning for bootflows with label '9'");
+	ut_assert_skip_to_line("(1 bootflow, 1 valid)");
+
 	ut_assertok(run_command("bootflow scan -lH 0", 0));
-	ut_assert_nextline("Scanning for bootflows in bootdev 'mmc2.bootdev'");
+	ut_assert_nextline("Scanning for bootflows with label '0'");
 	ut_assert_skip_to_line("(0 bootflows, 0 valid)");
 	ut_assert_console_end();
 
 	return 0;
 }
-BOOTSTD_TEST(bootflow_cmd_label, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+BOOTSTD_TEST(bootflow_cmd_label, UT_TESTF_DM | UT_TESTF_SCAN_FDT |
+	     UT_TESTF_ETH_BOOTDEV);
+#endif
 
 /* Check 'bootflow scan/list' commands using all bootdevs */
 static int bootflow_cmd_glob(struct unit_test_state *uts)