Hi Alex, On 10 August 2016 at 07:25, Alexander Graf <ag...@suse.de> 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.
Yes that's right. But it is mostly just a simple case of enabling the option. For a few boards there might be an MMC driver that needs converting, but we can approach those one by one. Plus I could use the help in converting things. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot