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

[Simon Glass <sjg@chromium.org>]

Hi Heinrich,

On Tue, 2 Aug 2022 at 10:22, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 8/2/22 14:41, Simon Glass wrote:
> > 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
>
> Yes, you blocked the obvious solution.

Yes, I explained the problem with that at the time.

>
> > difficult to migrate away from IF_TYPE.
>
> My patch does not have any impact on the migration as function
> blk_get_devnum_by_typename() will simply vanish together with IF_TYPE.
>
> Migrating away from IF_TYPE could follow the following path if you
> wanted to keep struct blk_desc:
>
> Just replace devnum by the udevice in struct blk_desc and add the GUI
> representation of the device type (e.g. "mmc") as field to struct blk_ops.
>
> The field devnum only made sense in the world of legacy drivers.
> By the way why do I still find CONFIG_IS_ENABLED(BLK) in block drivers?
>
> A better solution would be to completely do away with struct blk_desc
> and instead always use the udevice.
>
> >
> > 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?
>
> There is no actual parent device. In
> lib/efi_selftest/efi_selftest_block_device.c the block device is a RAM
> disk. This is the same situation as with the sandbox host device where
> you have chosen root as the dummy parent for good reason.

Is it a RAM disk on disk, or an in-memory one?

>
> In
> "[1/1] drivers: add memory disk support"
> https://patchwork.ozlabs.org/project/uboot/patch/20220419211641.316935-1-heinrich.schuchardt@canonical.com/
> I have proposed a further block device type that has no actual parent.
>
> The idea of using a parent device to match a block device was always a
> dead end. Let's bury it now.

Possibly, but it is how we can drop the IF_TYPE stuff. Let me spend a
bit of time looking at this and see what can be done.

[..]

Regards,
Simon