Re: [PATCH] dm: core: Do not stop uclass iteration on error.

[Simon Glass <sjg@chromium.org>]

Hi Michal,

On Thu, 4 Aug 2022 at 11:59, Michal Suchanek <msuchanek@suse.de> wrote:
>
> When probing a device fails NULL pointer is returned, and other devices
> cannot be iterated. Skip to next device on error instead.
>
> Fixes: 6494d708bf ("dm: Add base driver model support")
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  drivers/core/uclass.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)

Are you able to create a test for this, e.g. in test/dm/core.c or test-fdt.c ?

Some sandbox devices can be made to give an error, so the test can
check the logic.

Strangely enough, this is actually a very big change, so we need to be careful.

>
> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> index 08d9ed82de..ccf7d59141 100644
> --- a/drivers/core/uclass.c
> +++ b/drivers/core/uclass.c
> @@ -574,16 +574,31 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent,
>  }
>  #endif
>
> +/* Starting from the given device return first device in the uclass that probes successfully */
> +static int __uclass_next_device(struct udevice *dev, int ret, struct udevice **devp)
> +{
> +       if (!dev) {
> +               *devp = dev;
> +               return 0;
> +       }
> +       while ((ret = uclass_get_device_tail(dev, ret, devp))) {
> +               ret = uclass_find_next_device(&dev);
> +               if (!dev) {
> +                       *devp = dev;
> +                       return 0;
> +               }
> +       }
> +
> +       return ret;
> +}
> +
>  int uclass_first_device(enum uclass_id id, struct udevice **devp)
>  {
> -       struct udevice *dev;
> +       struct udevice *dev = NULL;
>         int ret;
>
> -       *devp = NULL;
>         ret = uclass_find_first_device(id, &dev);
> -       if (!dev)
> -               return 0;
> -       return uclass_get_device_tail(dev, ret, devp);
> +       return __uclass_next_device(dev, ret, devp);
>  }
>
>  int uclass_first_device_err(enum uclass_id id, struct udevice **devp)
> @@ -604,11 +619,8 @@ int uclass_next_device(struct udevice **devp)
>         struct udevice *dev = *devp;
>         int ret;
>
> -       *devp = NULL;
>         ret = uclass_find_next_device(&dev);
> -       if (!dev)
> -               return 0;
> -       return uclass_get_device_tail(dev, ret, devp);
> +       return __uclass_next_device(dev, ret, devp);
>  }
>
>  int uclass_next_device_err(struct udevice **devp)
> --
> 2.37.1
>

Regards,
Simon