[PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()

[Simon Glass]

Hi Heinrich,

On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 10/3/22 16:57, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> On the sandbox I run:
> >>
> >>      => setenv efi_selftest block device
> >>      => bootefi selftest
> >>
> >> and see the following output:
> >>
> >>      ** Bad device specification host 0 **
> >>      Couldn't find partition host 0:0
> >>      Cannot read EFI system partition
> >>
> >> Running
> >>
> >>      => lsblk
> >>
> >> yields
> >>
> >>      Block Driver          Devices
> >>      -----------------------------
> >>      efi_blk             : efiloader 0
> >>      ide_blk             : <none>
> >>      mmc_blk             : mmc 2, mmc 1, mmc 0
> >>      nvme-blk            : <none>
> >>      sandbox_host_blk    : <none>
> >>      scsi_blk            : <none>
> >>      usb_storage_blk     : <none>
> >>      virtio-blk          : <none>
> >>
> >> So a efi_blk device was mistaken for a host device.
> >>
> >> I continue with
> >>
> >>      => host bind 0 ../sandbox.img
> >>      => ls host 0:1
> >>
> >> and get the following output:
> >>
> >>             13   hello.txt
> >>              7   u-boot.txt
> >>
> >>      2 file(s), 0 dir(s)
> >>
> >> This is the content of efiblock 0:1 and not of host 0:1 (sic!).
> >>
> >> The uclass of the parent device is irrelevant for the determination of the
> >> uclass of the block device. We must use the uclass stored in the block
> >> device descriptor.
> >>
> >> This issue has been raised repeatedly:
> >>
> >> [PATCH 1/1] block: fix blk_get_devnum_by_typename()
> >> https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schuchardt@canonical.com/
> >> [PATCH 1/1] blk: simplify blk_get_devnum_by_typename()
> >> https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schuchardt@canonical.com/
> >
> > Yes and you were not able/willing to take on the required work, so
> > this carried on longer than it should have. I finally did this myself
> > and it is now in -next.
>
> The refactoring was orthogonal to the problem that I reported and which
> you unfortunately did not consider in the process.

Well it involved using if_type to work around a problem, just making
it harder to get rid of. Overall I am in favour of a faster pace of
migration that we have been following and it would help if people took
on some of this, instead of fixing their little issue.

>
> >
> > So we might finally be able to fix this problem properly, since
> > if_type is mostly just a work-around concept in -next, with just the
> > fake uclass_id being used at present.
> >
> > Can you use if_type_to_uclass_id() here, which is the work-around
> > function for now?
>
> This function does not exist in origin/next. We won't apply this patch
> in the 2022-10 cycle.

I think I mean conv_uclass_id() which is the new name.

>
> Let's fix the bug first before thinking about future refactoring.
>
> You may determine the uclass ID for field bdev in struct blk_desc using
> function device_get_uclass_id() when refactoring.

So if you call conv_uclass_id() (without any other refactoring) does
that fix the problem?

>
> >
> > Also, I wonder if we can require SPL_BLK and thus get rid of the
> > legacy block interface? Then we can drop drop uclass_id and a few
> > other fields from struct blk_desc.
>
> This is beyond the scope of this patch. Neither host nor efi_loader
> devices exist in SPL.

Yes I understand that. It was just a question...

Regards,
Simon