[U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage

[Simon Glass]

+Tom

Hi Alex,

On 10 August 2016 at 01:47, Alexander Graf <agraf at suse.de> wrote:
>
>> On 08 Aug 2016, at 23:44, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Alexander,
>>
>> On 5 August 2016 at 06:49, Alexander Graf <agraf at suse.de> wrote:
>>> When using CONFIG_BLK, there were 2 issues:
>>>
>>>  1) The name we generate the device with has to match the
>>>     name we set in efi_set_bootdev()
>>>
>>>  2) The device we pass into our block functions was wrong,
>>>     we should not rediscover it but just use the already known
>>>     pointer.
>>>
>>> This patch fixes both issues.
>>>
>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>> ---
>>> cmd/bootefi.c             | 23 ++++++++++++++++++-----
>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>
[...]

>>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>>> index c434c92..e00a747 100644
>>> --- a/lib/efi_loader/efi_disk.c
>>> +++ b/lib/efi_loader/efi_disk.c
>>> @@ -31,6 +31,8 @@ struct efi_disk_obj {
>>>        struct efi_device_path_file_path *dp;
>>>        /* Offset into disk for simple partitions */
>>>        lbaint_t offset;
>>> +       /* Internal block device */
>>> +       const struct blk_desc *desc;
>>
>> Rather than storing this, can you store the udevice?
>
> I could, but then I would diverge between the CONFIG_BLK and non-CONFIG_BLK path again, which would turn the code into an #ifdef mess (read: hard to maintain), because the whole device creation path relies on struct blk_desc * today and doesn’t pass the udevice anywhere.
>
> Do you feel strongly about this? To give you an idea how messy it gets, the diff is below.

Actually I'd like to make this feature depend on CONFIG_BLK. If we add
new features that don't use driver model, and then use the legacy data
structures such that converting to driver model becomes harder, we'll
never be done.

I did mention this at the beginning and it seems to have come to pass.

In order of preference from my side:

1. Make EFI_LOADER depend on BLK
2. Apply your messy diff (so that we can remove the old code one day
and end up with something nice)

>
>

[...messy diff...]

Regards,
Simon