On 8/3/22 20:14, Simon Glass wrote:
Hi Heinrich,

On Tue, 2 Aug 2022 at 10:22, Heinrich Schuchardt
<heinrich.schucha...@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.schucha...@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.schucha...@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.



In
"[1/1] drivers: add memory disk support"
https://patchwork.ozlabs.org/project/uboot/patch/20220419211641.316935-1-heinrich.schucha...@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.

To me the device driver of the actual device would be the most natural indicator of the device type. Looking forward to your suggestion.

Best regards

Heinrich


[..]

Regards,
Simon

Reply via email to