Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-23 Thread Leif Lindholm
On Thu, Sep 23, 2021 at 15:20:41 +, 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 
> > Sent: Thursday, September 23, 2021 5:06 AM
> > To: Jeff Brasen 
> > Cc: devel@edk2.groups.io; daniel.schae...@hpe.com;
> > abner.ch...@hpe.com; ardb+tianoc...@kernel.org; Jun Nie
> > 
> > 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 +, Jeff Brasen wrote:
> > > Jun/Others,
> > >
> > >   Any additional comments on this patch series?
> > >
> > >
> > > Thanks,
> > >
> > > Jeff
> > >
> > > 
> > > From: Jeff Brasen 
> > > Sent: Tuesday, September 14, 2021 10:57 AM
> > > To: Leif Lindholm 
> > > Cc: devel@edk2.groups.io ;
> > > daniel.schae...@hpe.com ;
> > abner.ch...@hpe.com
> > > ; ardb+tianoc...@kernel.org
> > > ; Jun Nie 
> > > 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 
> > > Sent: Tuesday, September 14, 2021 9:00 AM
> > > To: Jeff Brasen 
> > > Cc: devel@edk2.groups.io ;
> > > daniel.schae...@hpe.com ;
> > abner.ch...@hpe.com
> > > ; ardb+tianoc...@kernel.org
> > > ; Jun Nie 
> > > 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  for those if there are
> > > any further revisions of the set.
> > >
> > > Best Regards,
> > >
> > > Leif
> > >
> > > On Mon, Sep 13, 2021 at 23:18:47 +, 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
> > > > 

Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-23 Thread Jeff Brasen via groups.io
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 
> Sent: Thursday, September 23, 2021 5:06 AM
> To: Jeff Brasen 
> Cc: devel@edk2.groups.io; daniel.schae...@hpe.com;
> abner.ch...@hpe.com; ardb+tianoc...@kernel.org; Jun Nie
> 
> 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 +, Jeff Brasen wrote:
> > Jun/Others,
> >
> >   Any additional comments on this patch series?
> >
> >
> > Thanks,
> >
> > Jeff
> >
> > 
> > From: Jeff Brasen 
> > Sent: Tuesday, September 14, 2021 10:57 AM
> > To: Leif Lindholm 
> > Cc: devel@edk2.groups.io ;
> > daniel.schae...@hpe.com ;
> abner.ch...@hpe.com
> > ; ardb+tianoc...@kernel.org
> > ; Jun Nie 
> > 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 
> > Sent: Tuesday, September 14, 2021 9:00 AM
> > To: Jeff Brasen 
> > Cc: devel@edk2.groups.io ;
> > daniel.schae...@hpe.com ;
> abner.ch...@hpe.com
> > ; ardb+tianoc...@kernel.org
> > ; Jun Nie 
> > 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  for those if there are
> > any further revisions of the set.
> >
> > Best Regards,
> >
> > Leif
> >
> > On Mon, Sep 13, 2021 at 23:18:47 +, 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
> > >
> > >  

Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-23 Thread Leif Lindholm
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 +, Jeff Brasen wrote:
> Jun/Others,
> 
>   Any additional comments on this patch series?
> 
> 
> Thanks,
> 
> Jeff
> 
> 
> From: Jeff Brasen 
> Sent: Tuesday, September 14, 2021 10:57 AM
> To: Leif Lindholm 
> Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
> ; abner.ch...@hpe.com ; 
> ardb+tianoc...@kernel.org ; Jun Nie 
> 
> 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 
> Sent: Tuesday, September 14, 2021 9:00 AM
> To: Jeff Brasen 
> Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
> ; abner.ch...@hpe.com ; 
> ardb+tianoc...@kernel.org ; Jun Nie 
> 
> 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 
> for those if there are any further revisions of the set.
> 
> Best Regards,
> 
> Leif
> 
> On Mon, Sep 13, 2021 at 23:18:47 +, 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 (#81028): https://edk2.groups.io/g/devel/message/81028
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]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-22 Thread Jun Nie
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  于2021年9月22日周三 上午12:33写道:
>
> Jun/Others,
>
>   Any additional comments on this patch series?
>
> Thanks,
>
> Jeff
>
> 
> From: Jeff Brasen 
> Sent: Tuesday, September 14, 2021 10:57 AM
> To: Leif Lindholm 
> Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
> ; abner.ch...@hpe.com ; 
> ardb+tianoc...@kernel.org ; Jun Nie 
> 
> 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 
> Sent: Tuesday, September 14, 2021 9:00 AM
> To: Jeff Brasen 
> Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
> ; abner.ch...@hpe.com ; 
> ardb+tianoc...@kernel.org ; Jun Nie 
> 
> 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 
> for those if there are any further revisions of the set.
>
> Best Regards,
>
> Leif
>
> On Mon, Sep 13, 2021 at 23:18:47 +, 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 (#80948): https://edk2.groups.io/g/devel/message/80948
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]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-21 Thread Jeff Brasen via groups.io
Jun/Others,

  Any additional comments on this patch series?


Thanks,

Jeff


From: Jeff Brasen 
Sent: Tuesday, September 14, 2021 10:57 AM
To: Leif Lindholm 
Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
; abner.ch...@hpe.com ; 
ardb+tianoc...@kernel.org ; Jun Nie 

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 
Sent: Tuesday, September 14, 2021 9:00 AM
To: Jeff Brasen 
Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
; abner.ch...@hpe.com ; 
ardb+tianoc...@kernel.org ; Jun Nie 

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 
for those if there are any further revisions of the set.

Best Regards,

Leif

On Mon, Sep 13, 2021 at 23:18:47 +, 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 (#80935): https://edk2.groups.io/g/devel/message/80935
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]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-14 Thread Jeff Brasen via groups.io
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 
Sent: Tuesday, September 14, 2021 9:00 AM
To: Jeff Brasen 
Cc: devel@edk2.groups.io ; daniel.schae...@hpe.com 
; abner.ch...@hpe.com ; 
ardb+tianoc...@kernel.org ; Jun Nie 

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 
for those if there are any further revisions of the set.

Best Regards,

Leif

On Mon, Sep 13, 2021 at 23:18:47 +, 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 (#80666): https://edk2.groups.io/g/devel/message/80666
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]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3 0/4] AndroidBootImgLib improvements

2021-09-14 Thread Leif Lindholm
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 
for those if there are any further revisions of the set.

Best Regards,

Leif

On Mon, Sep 13, 2021 at 23:18:47 +, 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 (#80663): https://edk2.groups.io/g/devel/message/80663
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]
-=-=-=-=-=-=-=-=-=-=-=-