Re: [PATCH v2 31/39] Revert "bootdev: avoid infinite probe loop"

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 06.08.24 14:58, Simon Glass wrote:
> 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.

Please, describe how the original problem will be addressed.

@Caleb:

Do you have instruction how to reproduce the problem?

>
> 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 virtual of staying in the loop looking for

%s/virtual/virtue/

Best regards

Heinrich

> a bootdev, so let's go back to that and try to fix this another way.
>
> This reverts commit 9d92c418acfb7576e12e2bd53fed294bb9543724.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>   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 7c7bba088c9..15a8a3555c6 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;
>   			}
>   		}