Hi Alex, On 10 August 2016 at 13:01, Alexander Graf <ag...@suse.de> wrote: > >> On 10 Aug 2016, at 18:25, Tom Rini <tr...@konsulko.com> wrote: >> >> 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? > > I want to make EFI the default boot path in openSUSE, which means any board > that anyone out there wants to run openSUSE on would be on the list. Anything > that keeps them from running EFI on a random board is a road block that I’d > rather not have if I can avoid it.
Of course, I fully understand that. However as mentioned in this patch, you are creating future problems. Can you address Tom's question? I take it that it boots on Raspberry Pi (which I'd like to try actually - are there instructions somewhere?). We can easily convert that over. Anything else? > > Again, I strongly object any dependency on BLK for EFI. If you want to push > people to using CONFIG_BLK, make CONFIG_ARM depend on CONFIG_BLK. :-) That won't work. If we could wave a magic wand and convert everything in one day, we would. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot