> On 10 Aug 2016, at 15:33, Simon Glass <s...@chromium.org> wrote:
>
> 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.
That approach sounds terribly wrong tbh. If it’s so simple, why not just mark a
flag day where CONFIG_BLK becomes mandatory and fix all fallout? Then you’ll
have much more help at your disposal than a few percent of my overloaded days
;).
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
