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

[Simon Glass <sjg@chromium.org>]

Hi Heinrich,

On Thu, 4 Aug 2022 at 00:06, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 8/3/22 20:14, Simon Glass wrote:
> > 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?
>
> With the patch it is just an memory area embedded the U-Boot binary. But
> in future you might also use it to declare a memory area in the rest of
> RAM as backing a RAM disk.

I sent a series to clean this up.

Regards,
Simon