[PATCH v4 04/21] dm: blk: Add probe in blk_first_device/blk_next_device

[Simon Glass]

Hi Michal,

On Sun, 25 Sept 2022 at 02:28, Michal Suchanek <msuchanek at suse.de> wrote:
>
> The description claims that the device is probed but it isn't.
>
> Add the device_probe() call.
>
> Also consolidate the iteration into one function.
>
> Fixes: 8a5cbc065d ("dm: blk: Use uclass_find_first/next_device() in blk_first/next_device()")
> Signed-off-by: Michal Suchanek <msuchanek at suse.de>
> ---
>  drivers/block/blk-uclass.c | 46 ++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 21c5209bb6..992f8ad3da 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -361,45 +361,43 @@ int blk_dselect_hwpart(struct blk_desc *desc, int hwpart)
>         return blk_select_hwpart(desc->bdev, hwpart);
>  }
>
> -int blk_first_device(int if_type, struct udevice **devp)
> +static int _blk_next_device(int if_type, struct udevice **devp)
>  {
>         struct blk_desc *desc;
> -       int ret;
> +       int ret = 0;
> +
> +       for (; *devp; uclass_find_next_device(devp)) {
> +               desc = dev_get_uclass_plat(*devp);
> +               if (desc->if_type == if_type) {
> +                       ret = device_probe(*devp);
> +                       if (!ret)
> +                               return 0;
> +               }
> +       }
>
> -       ret = uclass_find_first_device(UCLASS_BLK, devp);
>         if (ret)
>                 return ret;
> -       if (!*devp)
> -               return -ENODEV;
> -       do {
> -               desc = dev_get_uclass_plat(*devp);
> -               if (desc->if_type == if_type)
> -                       return 0;
> -               ret = uclass_find_next_device(devp);
> -               if (ret)
> -                       return ret;
> -       } while (*devp);

This looks wrong since a media device may have other devices under it,
e.g. UCLASS_BOOTDEV so I think you should keep the existing code and
just call uclass_probe() at the end.

You could add a test for this by checking that only the BLK device is probed.

>
>         return -ENODEV;
>  }
>
> +int blk_first_device(int if_type, struct udevice **devp)
> +{
> +       uclass_find_first_device(UCLASS_BLK, devp);
> +
> +       return _blk_next_device(if_type, devp);
> +}
> +
>  int blk_next_device(struct udevice **devp)
>  {
>         struct blk_desc *desc;
> -       int ret, if_type;
> +       int if_type;
>
>         desc = dev_get_uclass_plat(*devp);
>         if_type = desc->if_type;
> -       do {
> -               ret = uclass_find_next_device(devp);
> -               if (ret)
> -                       return ret;
> -               if (!*devp)
> -                       return -ENODEV;
> -               desc = dev_get_uclass_plat(*devp);
> -               if (desc->if_type == if_type)
> -                       return 0;
> -       } while (1);
> +       uclass_find_next_device(devp);
> +
> +       return _blk_next_device(if_type, devp);
>  }
>
>  int blk_find_device(int if_type, int devnum, struct udevice **devp)
> --
> 2.37.3
>

Regards,
Simon