Re: Commit Partitioning - Re: [edk2-devel] [edk2] [PATCH V2] UefiPayloadPkg: Enhance UEFI payload for coreboot and Slim Bootloader

2019-04-15 Thread Guo Dong 

Thanks Jordan for the comments.
This patch adds a new package instead of updating existing code.
I thought it was difficult to separate this patch into multiple sub-patches 
while keeping each chunk to be able to compile and function.
Will consider this in the future.

Thanks,
Guo

> -Original Message-
> From: Justen, Jordan L
> Sent: Monday, April 15, 2019 3:28 PM
> To: Dong, Guo ; Ma, Maurice
> ; devel@edk2.groups.io
> Cc: Agyeman, Prince ; You, Benjamin
> ; Dong, Guo 
> Subject: Commit Partitioning - Re: [edk2-devel] [edk2] [PATCH V2]
> UefiPayloadPkg: Enhance UEFI payload for coreboot and Slim Bootloader
> 
> On 2019-04-11 08:51:22, Guo Dong wrote:
> > CorebootModulePkg and CorebootPayloadPkg originally supports coreboot
> only.
> > In order to support other bootloaders, such as Slim Bootloader, they
> > need be updated to be more generic.
> > UEFI Payload (UefiPayloadPkg) a converged package from
> > CorebootModulePkg and CorebootPayloadPkg with following updates:
> > a. Support both coreboot and Slim Bootloader b. Removed
> > SataControllerDxe and BaseSerialPortLib16550 to use EDK2 modules c.
> > Support passing bootloader parameter to UEFI payload, e.g. coreboot
> >table from coreboot or HOB list from Slim Bootloader d. Using
> > GraphicsOutputDxe from EDK2 with minor change instead of FbGop e.
> > Remove the dependency to IntelFrameworkPkg and
> IntelFrameworkModulePkg
> >and QuarkSocPkg
> > f. Use BaseDebugLibSerialPort library as DebugLib g. Use HPET timer,
> > drop legacy 8254 timer support h. Use BaseXApicX2ApicLib instead of
> > BaseXApicLib i. Remove HOB gUefiFrameBufferInfoGuid to use EDK2
> > graphics HOBs.
> > j. Other clean ups
> 
> Why this wasn't split into *at least* 10 patches given the 10 major bullet
> points listed here?
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Commit-Partitioning
> 
> >
> > On how UefiPayloadPkg could work with coreboot/Slim Bootloader, please
> > refer UefiPayloadPkg/BuildAndIntegrationInstructions.txt
> >
> > Once UefiPayloadPkg is checked-in, CorebootModulePkg and
> > CorebootPayloadPkg could be retired.
> >
> > Signed-off-by: Guo Dong 
> > Reviewed-by: Maurice Ma 
> 
> Same question to Maurice.
> 
> Maybe something to consider in the future.
> 
> -Jordan
> 
> > ---
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c   | 
> > 158
> ++
> ++
> ++
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h   | 
> >  30
> ++
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 
> >  58
> ++
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.c   | 
> > 566
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> 
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.h   | 
> >  39
> +++
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.inf | 
> >  73
> ++
> +++
> >  UefiPayloadPkg/BuildAndIntegrationInstructions.txt   | 
> >  82
> ++
> ++

Re: Commit Partitioning - Re: [edk2-devel] [edk2] [PATCH V2] UefiPayloadPkg: Enhance UEFI payload for coreboot and Slim Bootloader

2019-04-15 Thread Ma, Maurice 
Hi, Jordan

That is a good point.   Smaller patch will always help the review easier.  
Thank you for pointing it out.   

Thanks
Maurice
> -Original Message-
> From: Justen, Jordan L
> Sent: Monday, April 15, 2019 15:28
> To: Dong, Guo ; Ma, Maurice
> ; devel@edk2.groups.io
> Cc: Agyeman, Prince ; You, Benjamin
> ; Dong, Guo 
> Subject: Commit Partitioning - Re: [edk2-devel] [edk2] [PATCH V2]
> UefiPayloadPkg: Enhance UEFI payload for coreboot and Slim Bootloader
> 
> On 2019-04-11 08:51:22, Guo Dong wrote:
> > CorebootModulePkg and CorebootPayloadPkg originally supports coreboot
> only.
> > In order to support other bootloaders, such as Slim Bootloader, they
> > need be updated to be more generic.
> > UEFI Payload (UefiPayloadPkg) a converged package from
> > CorebootModulePkg and CorebootPayloadPkg with following updates:
> > a. Support both coreboot and Slim Bootloader b. Removed
> > SataControllerDxe and BaseSerialPortLib16550 to use EDK2 modules c.
> > Support passing bootloader parameter to UEFI payload, e.g. coreboot
> >table from coreboot or HOB list from Slim Bootloader d. Using
> > GraphicsOutputDxe from EDK2 with minor change instead of FbGop e.
> > Remove the dependency to IntelFrameworkPkg and
> IntelFrameworkModulePkg
> >and QuarkSocPkg
> > f. Use BaseDebugLibSerialPort library as DebugLib g. Use HPET timer,
> > drop legacy 8254 timer support h. Use BaseXApicX2ApicLib instead of
> > BaseXApicLib i. Remove HOB gUefiFrameBufferInfoGuid to use EDK2
> > graphics HOBs.
> > j. Other clean ups
> 
> Why this wasn't split into *at least* 10 patches given the 10 major bullet
> points listed here?
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Commit-Partitioning
> 
> >
> > On how UefiPayloadPkg could work with coreboot/Slim Bootloader, please
> > refer UefiPayloadPkg/BuildAndIntegrationInstructions.txt
> >
> > Once UefiPayloadPkg is checked-in, CorebootModulePkg and
> > CorebootPayloadPkg could be retired.
> >
> > Signed-off-by: Guo Dong 
> > Reviewed-by: Maurice Ma 
> 
> Same question to Maurice.
> 
> Maybe something to consider in the future.
> 
> -Jordan
> 
> > ---
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c   | 
> > 158
> ++
> ++
> ++
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h   | 
> >  30
> ++
> >  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 
> >  58
> ++
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.c   | 
> > 566
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> ++
> 
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.h   | 
> >  39
> +++
> >  UefiPayloadPkg/BlSupportPei/BlSupportPei.inf | 
> >  73
> ++
> +++
> >  UefiPayloadPkg/BuildAndIntegrationInstructions.txt   | 
> >  82
> ++
> 
> >  UefiPayloadPkg/GraphicsOutputDxe/ComponentName.c | 
> > 184
> ++
> ++
> ++
> ++
> >  UefiPayloadPkg/GraphicsOutputDxe/GraphicsOutput.c| 
> > 739
> ++
> ++
> ++
> ++
> ++
>