From aad8b18a529d7413f35ba413a9c122dbb76d08bb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 15 Aug 2024 14:30:21 -0600 Subject: [PATCH 1/2] Revert "bootdev: avoid infinite probe loop" This turns out to be insufficient to fix the problem, since when bootdev_next_prio() exits, the caller has no idea that this really is the end. Nor is it, since there may be other devices which should be checked. The caller iterates which calls iter_incr() which calls bootdev_next_prio() again, which finds the same device and the loop continues. We never did create a test for this[1], which makes it hard to be sure which problem was fixed. The original code had the virtue of staying in the loop looking for a bootdev, so let's go back to that and try to fix this another way. A future patch will make bootdev_next_prio() continue after failure which should provide same effect. This reverts commit 9d92c418acfb7576e12e2bd53fed294bb9543724. Signed-off-by: Simon Glass --- boot/bootdev-uclass.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 7c7bba088c..15a8a3555c 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -632,7 +632,7 @@ int bootdev_next_label(struct bootflow_iter *iter, struct udevice **devp, int bootdev_next_prio(struct bootflow_iter *iter, struct udevice **devp) { - struct udevice *dev = *devp, *last_dev = NULL; + struct udevice *dev = *devp; bool found; int ret; @@ -682,19 +682,9 @@ int bootdev_next_prio(struct bootflow_iter *iter, struct udevice **devp) } } else { ret = device_probe(dev); - if (!ret) - last_dev = dev; if (ret) { - log_warning("Device '%s' failed to probe\n", + log_debug("Device '%s' failed to probe\n", dev->name); - if (last_dev == dev) { - /* - * We have already tried this device - * and it failed to probe. Give up. - */ - return log_msg_ret("probe", ret); - } - last_dev = dev; dev = NULL; } } From cae1ad02f7aa2ee67ecd0b18eb29c51dd8021267 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 15 Aug 2024 14:30:22 -0600 Subject: [PATCH 2/2] bootstd: Make bootdev_next_prio() continue after failure When a device fails to probe, the next device should be tried, until either we find a suitable device or run out of devices. A device should never be tried twice. When we run out of devices of a particular priority, the hunter should be used to generate devices of the next priority. Only if all attempts fail should this function return an error. Update the function to use the latent 'found' boolean to determine whether another loop iteration is warranted, rather than setting 'dev' to NULL, which creates confusion, suggesting that no devices have been scanned and the whole process is starting from the beginning. Note that the upcoming bootflow_efi() test is used to test this behaviour. Signed-off-by: Simon Glass Fixes: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/17 --- boot/bootdev-uclass.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 15a8a3555c..807f8dfb06 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -640,6 +640,7 @@ int bootdev_next_prio(struct bootflow_iter *iter, struct udevice **devp) *devp = NULL; log_debug("next prio %d: dev=%p/%s\n", iter->cur_prio, dev, dev ? dev->name : "none"); + found = false; do { /* * Don't probe devices here since they may not be of the @@ -682,13 +683,13 @@ int bootdev_next_prio(struct bootflow_iter *iter, struct udevice **devp) } } else { ret = device_probe(dev); - if (ret) { + if (ret) log_debug("Device '%s' failed to probe\n", dev->name); - dev = NULL; - } + else + found = true; } - } while (!dev); + } while (!found); *devp = dev;