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

[Alexander Graf]

On 08/10/2016 02:56 PM, Simon Glass wrote:
> +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

If we make EFI_LOADER depend on BLK, doesn't that break all systems that 
need storage that isn't converted to device model today? Like the SATA 
breakage on Xilinx systems, just at a much bigger scale?


Alex