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

[Heinrich Schuchardt]

On 8/3/22 20:14, Simon Glass wrote:
> Hi Heinrich,
> 
> On Tue, 2 Aug 2022 at 10:22, Heinrich Schuchardt
> <heinrich.schuchardt at 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 at 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 at 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.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.

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