On Mon, Aug 22, 2016 at 12:22:58AM -0400, Alexander Graf wrote:
>
>
> > 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.OK, that's what I wanted. To me the greatest problem (and why we've pushed back on some other things that came in but didn't implement DM) is non-DM code that won't have someone active to convert it later. That won't be the case here, so we'll address things here more later. Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
