That looks like something I missed and looks good, do you need me to make a v4 or will you just add that in. 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. > -----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 (#81047): https://edk2.groups.io/g/devel/message/81047 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] -=-=-=-=-=-=-=-=-=-=-=-
