Re: [PATCH v3 0/4] AndroidBootImgLib improvements

[Jun Nie <jun.nie@linaro.org>]

Hi Jeff,

I do not ever work on EDK soon after this patch set was merged.  It is
long time since then.
I am sorry that I have no comments other than no objections on your patch.

Regards,
Jun

Jeff Brasen <jbrasen@nvidia.com> 于2021年9月22日周三 上午12:33写道：
>
> Jun/Others,
>
>   Any additional comments on this patch series?
>
> Thanks,
>
> Jeff
>
> ________________________________
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Tuesday, September 14, 2021 10:57 AM
> To: Leif Lindholm <leif@nuviainc.com>
> Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.chang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Jun Nie <jun.nie@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 <leif@nuviainc.com>
> Sent: Tuesday, September 14, 2021 9:00 AM
> To: Jeff Brasen <jbrasen@nvidia.com>
> Cc: devel@edk2.groups.io <devel@edk2.groups.io>; daniel.schaefer@hpe.com <daniel.schaefer@hpe.com>; abner.chang@hpe.com <abner.chang@hpe.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Jun Nie <jun.nie@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 <leif@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
> >