> Am 19.08.2016 um 14:14 schrieb Tom Rini <tr...@konsulko.com>:
>
>> On Fri, Aug 12, 2016 at 08:19:42PM +0200, Alexander Graf wrote:
>>
>>
>>> 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.
>
> OK, that is a lot of boards and such. And yes, I see both of these
> features / projects as important to the long-term health of U-Boot.
>
> So, Alex, when we've got storage converted fully to DM, you're willing
> to do further clean-ups to make it DM-better, yes? Thanks!
I don't understand the question. Eventually I agree with Simon that we should
have only a single object list and type, but with the state dm is in, I
definitely will not require any of the efi code on dm.
Having said that, I am definitely making sure things work with dm and as soon
as a non-dm subsystem drops, I will happily integrate the efi onject hierarchy
into the dm one for that class of objects.
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
