Re: [PATCH 1/1] block: fix blk_get_devnum_by_typename()

[Simon Glass <sjg@chromium.org>]

Hi Heinrich,

On Tue, 2 Aug 2022 at 03:50, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> Both the 'host' and the 'efiloader' block devices use the same parent
> uclass root. Thus the parent uclass is not an indicator the interface type.
>
> Currently the following fails:
>
>     setenv efi_selftest block device
>     bootefi selftest
>     part list efiloader 0
>
> Struct blk_desc contains the interface type. So we can check it directly
> without caring about the parent uclass.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  drivers/block/blk-uclass.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)

We've had this discussion before, but this patch will make it
difficult to migrate away from IF_TYPE.

Instead we should fix EFI. Having the root as a parent of a block
device seems wrong to me. What is the actual device that provides the
block device?

>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 21c5209bb6..779cda7834 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -122,15 +122,11 @@ struct blk_desc *blk_get_devnum_by_typename(const char *if_typename, int devnum)
>
>                 debug("%s: if_type=%d, devnum=%d: %s, %d, %d\n", __func__,
>                       if_type, devnum, dev->name, desc->if_type, desc->devnum);
> -               if (desc->devnum != devnum)
> -                       continue;
>
> -               /* Find out the parent device uclass */
> -               if (device_get_uclass_id(dev->parent) != uclass_id) {
> -                       debug("%s: parent uclass %d, this dev %d\n", __func__,
> -                             device_get_uclass_id(dev->parent), uclass_id);
> +               if (desc->if_type != if_type)
> +                       continue;
> +               if (desc->devnum != devnum)
>                         continue;
> -               }
>
>                 if (device_probe(dev))
>                         return NULL;
> --
> 2.36.1
>

Regards,
Simon