On Thu, Sep 23, 2021 at 15:20:41 +0000, Jeff Brasen wrote:
> That looks like something I missed and looks good, do you need me to
> make a v4 or will you just add that in.
Nah, I just folded it in.
Pushed as 79019c7a4228..7ea7f9c07757.
Thanks!
> When I built I just used our
> application that has the guid in our inf so missed this.
>
> From my looking at it AndroidFastBootApp doesn't seem to use this lib.
(That ought to have been obvious even to me, oh well, thanks :)
/
Leif
> > -----Original Message-----
> > From: Leif Lindholm <l...@nuviainc.com>
> > Sent: Thursday, September 23, 2021 5:06 AM
> > To: Jeff Brasen <jbra...@nvidia.com>
> > Cc: devel@edk2.groups.io; daniel.schae...@hpe.com;
> > abner.ch...@hpe.com; ardb+tianoc...@kernel.org; Jun Nie
> > <jun....@linaro.org>
> > Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Hi Jeff,
> >
> > I was about to say "no more issues", and then I went to build EmbeddedPkg,
> > and it turns out this fails in Applications/AndroidBootApp due to the
> > missing
> > dependency on gEfiLoadFile2ProtocolGuid in AndroidBootImgLib.inf.
> >
> > (Why this doesn't break AndroidFastbootApp build as well is not immediately
> > obvious to me.)
> >
> > Would you like to figure out why, or would you prefer me to just fold in
> >
> > diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> > b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> > index affde50f617a..8eefeef4f915 100644
> > --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> > +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> > @@ -39,6 +39,7 @@ [Packages]
> > [Protocols]
> > gAndroidBootImgProtocolGuid
> > gEfiLoadedImageProtocolGuid
> > + gEfiLoadFile2ProtocolGuid
> >
> > [Guids]
> > gEfiAcpiTableGuid
> >
> > ?
> >
> > /
> > Leif
> >
> > On Tue, Sep 21, 2021 at 16:32:58 +0000, Jeff Brasen wrote:
> > > Jun/Others,
> > >
> > > Any additional comments on this patch series?
> > >
> > >
> > > Thanks,
> > >
> > > Jeff
> > >
> > > ________________________________
> > > From: Jeff Brasen <jbra...@nvidia.com>
> > > Sent: Tuesday, September 14, 2021 10:57 AM
> > > To: Leif Lindholm <l...@nuviainc.com>
> > > Cc: devel@edk2.groups.io <devel@edk2.groups.io>;
> > > daniel.schae...@hpe.com <daniel.schae...@hpe.com>;
> > abner.ch...@hpe.com
> > > <abner.ch...@hpe.com>; ardb+tianoc...@kernel.org
> > > <ardb+tianoc...@kernel.org>; Jun Nie <jun....@linaro.org>
> > > Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements
> > >
> > > So for patch 3: This is only a change if mAndroidBootImg->UpdateDtb ==
> > NULL.
> > >
> > > This seemed like a bug as we would not add the initrd values nor would we
> > use the fdt from the BootImg if that is where the device tree was sourced
> > from.
> > >
> > > It seems like either we should require UpdateDtb to be implemented
> > (which seems to cause greater compatibility issues) or we install the device
> > tree we have if that function isn't implemented.
> > >
> > > As far as merging goes I am fine either way. Our particular flow won't hit
> > this path as we don't have a device tree in the bootimg (use the system
> > config table) and we will have the new pcd set, but this seemed like a bug
> > while I looking at this code.
> > >
> > >
> > > Thanks,
> > >
> > > Jeff
> > >
> > > ________________________________
> > > From: Leif Lindholm <l...@nuviainc.com>
> > > Sent: Tuesday, September 14, 2021 9:00 AM
> > > To: Jeff Brasen <jbra...@nvidia.com>
> > > Cc: devel@edk2.groups.io <devel@edk2.groups.io>;
> > > daniel.schae...@hpe.com <daniel.schae...@hpe.com>;
> > abner.ch...@hpe.com
> > > <abner.ch...@hpe.com>; ardb+tianoc...@kernel.org
> > > <ardb+tianoc...@kernel.org>; Jun Nie <jun....@linaro.org>
> > > Subject: Re: [PATCH v3 0/4] AndroidBootImgLib improvements
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > Hi Jeff,
> > >
> > > Thanks for this.
> > > This set looks good to me, with a slight question mark wrt behaviour
> > > compatibility with previous versions for 3/4.
> > > (I think it's fine, but I'm a bear of very little brain, and it's been
> > > several years since I reviewed this code, and even longer since I
> > > really interacted with Android.
> > > ^
> > > | shameless plug for more EmbeddedPkg reviewer volunteers.)
> > >
> > > I've added Jun Nie, who wrote the original version of this code, to
> > > see if he has any comments.
> > >
> > > 1-2/4 are obviously unproblematic, and I could merge those ahead of
> > > time if preferred. You can add
> > > Reviewed-by: Leif Lindholm <l...@nuviainc.com> for those if there are
> > > any further revisions of the set.
> > >
> > > Best Regards,
> > >
> > > Leif
> > >
> > > On Mon, Sep 13, 2021 at 23:18:47 +0000, Jeff Brasen wrote:
> > > > Added support for using loadfile2 approach for passing ramdisk to linux.
> > > > Created patch series for general error handling improvments based on
> > > > review feedback.
> > > > If ACPI tables are in system table or PCD is defined the LoadFile2
> > > > method of passing initrd will be used.
> > > >
> > > > [v3]
> > > > -Code review cleanup
> > > > -Removed duplicate header file
> > > > -Added change to allow FDT to install if UpdateDtb function is not
> > > > defined -Added specific ACPI check -Moved install functions to
> > > > subfunctions
> > > >
> > > > [v2]
> > > > -Added review feedback
> > > > -General improvements to error handling
> > > >
> > > > [v1]
> > > > - Intial revision
> > > >
> > > >
> > > > Jeff Brasen (4):
> > > > EmbeddedPkg: Remove duplicate libfdt.h include
> > > > EmbeddedPkg: AndroidBootImgBoot error handling updates
> > > > EmbeddedPkg: Install FDT if UpdateDtb is not present
> > > > EmbeddedPkg: Add LoadFile2 for linux initrd
> > > >
> > > > EmbeddedPkg/EmbeddedPkg.dec | 1 +
> > > > .../AndroidBootImgLib/AndroidBootImgLib.inf | 4 +
> > > > .../AndroidBootImgLib/AndroidBootImgLib.c | 275 +++++++++++++++-
> > --
> > > > 3 files changed, 233 insertions(+), 47 deletions(-)
> > > >
> > > > --
> > > > 2.17.1
> > > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81053): https://edk2.groups.io/g/devel/message/81053
Mute This Topic: https://groups.io/mt/85589861/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
