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

[Tom Rini]

On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
> 
> 
> > Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg at chromium.org>:
> > 
> > Hi Alex,
> > 
> >> On 10 August 2016 at 07:02, Alexander Graf <agraf at suse.de> wrote:
> >>> 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?
> > 
> > No it just means that these platforms need to move to BLK before they
> > can use the EFI loader. Given the embryonic nature of this feature,
> > that seems reasonable, and the impact would be small. It will also
> > encourage conversion and keep the code cleaner.
> 
> No, it will simply make my life harder because I would have to sit
> down and vonvert every single board to BLK that I need EFI enabled.

Seems like as good a place as any to jump in, of the boards that you
need EFI enabled on, what isn't converted today?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160810/15233a41/attachment.sig>