Re: [edk2-platforms PATCH v4 1/4] SbsaQemu: Add FdtHelperLib

["Rebecca Cran" <rebecca@nuviainc.com>]

On 2/23/21 9:32 AM, Leif Lindholm wrote:
> On Tue, Feb 23, 2021 at 07:26:32 -0700, Rebecca Cran wrote:
>> The CountCpusFromFdt function is now used in two places. Create
>> FdtHelperLib for this and similar functions.
>>
>> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
>> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>> ---
>>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc                             |  2 +
>>   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c |  5 +-
>>   Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h            | 24 ++++++++
>>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c       | 62 ++++++++++++++++++++
>>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf     | 28 +++++++++
>>   5 files changed, 120 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
>> index f6af3f9111ee..8faad3eda217 100644
>> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
>> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
>> @@ -121,6 +121,8 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
>>     # ARM PL011 UART Driver
>>     PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>   
>> +  FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>> +
>>     # Debug Support
>>     PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
>>     DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
>> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> index fb7c1835c3d7..7bf60cd2ded1 100644
>> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> @@ -487,9 +487,12 @@ InitializeSbsaQemuAcpiDxe (
>>   {
>>     EFI_STATUS                     Status;
>>     EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
>> +  UINT32                         NumCores;
>>   
>>     // Parse the device tree and get the number of CPUs
>> -  CountCpusFromFdt ();
>> +  NumCores = FdtHelperCountCpus ();
>> +  Status = PcdSet32S (PcdCoreCount, NumCores);
>> +  ASSERT_RETURN_ERROR (Status);
>>   
>>     // Check if ACPI Table Protocol has been installed
>>     Status = gBS->LocateProtocol (
> 
> Would this hunk make more sense in 2/4?
> (I think it breaks bisect here.)
> 
> No further comments on this set, I think is thise moves, the set can
> go in.

Umm, yes it should be in 2/4 - apologies. I'll work to improve verifying 
which patches changes are in.

I've just sent out a v5.

-- 
Rebecca Cran