On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote: > > > > Am 10.08.2016 um 15:16 schrieb Simon Glass <s...@chromium.org>: > > > > Hi Alex, > > > >> On 10 August 2016 at 07:02, Alexander Graf <ag...@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 <ag...@suse.de> wrote: > >>>>> > >>>>> On 08 Aug 2016, at 23:44, Simon Glass <s...@chromium.org> wrote: > >>>>> > >>>>> Hi Alexander, > >>>>> > >>>>>> On 5 August 2016 at 06:49, Alexander Graf <ag...@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 <ag...@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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot