On 12.08.16 19:20, Simon Glass wrote:
> 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.
I don't see how I am creating future problems, really. Passing a
udevice* instead of the struct blk_desc* internally doesn't improve the
code really at the end of the day.
> 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?
For a list of currently available "upstream" openSUSE images, see
https://build.opensuse.org/package/view_file/openSUSE:Factory:ARM/JeOS/pre_checkin.sh?expand=1
Every one of those is on the short-term list. Any other board that
people want to run on is potentially on the mid-term to long-term list.
>
>>
>> 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.
EFI isn't going to be that magic wand either though :). Most people
couldn't care less. It's hard enough to convince people that disabling
CONFIG_EFI is a bad idea. Having anyone put effort into it would
basically mean the end of the whole idea that uEFI support "just works"
with U-Boot (which is the main selling point).
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
