Re: [PATCH 05/15] ArmVirtPkg/PrePi: remove dependency on ArmPlatformLib

[Laszlo Ersek <lersek@redhat.com>]

On 11/21/17 16:46, Laszlo Ersek wrote:
> On 11/17/17 17:09, Ard Biesheuvel wrote:
>> Remove the pointless dependency on ArmPlatformLib: none of the code we
>> call from it actually does anything useful.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 1 -
>>  ArmVirtPkg/PrePi/PrePi.c                            | 6 ++----
>>  ArmVirtPkg/PrePi/PrePi.h                            | 1 -
>>  3 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> index 5e706934f69f..1d79b1360c22 100755
>> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> @@ -52,7 +52,6 @@ [LibraryClasses]
>>    LzmaDecompressLib
>>    PeCoffGetEntryPointLib
>>    PrePiLib
>> -  ArmPlatformLib
>>    ArmPlatformStackLib
>>    MemoryAllocationLib
>>    HobLib
>> diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c
>> index c4fa979c43ef..fce4ab9428a5 100755
>> --- a/ArmVirtPkg/PrePi/PrePi.c
>> +++ b/ArmVirtPkg/PrePi/PrePi.c
>> @@ -13,6 +13,7 @@
>>  **/
>>  
>>  #include <PiPei.h>
>> +#include <Pi/PiBootMode.h>
>>  
>>  #include <Library/PrePiLib.h>
>>  #include <Library/PrintLib.h>
>> @@ -85,7 +86,7 @@ PrePiMain (
>>    BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
>>  
>>    // Set the Boot Mode
>> -  SetBootMode (ArmPlatformGetBootMode ());
>> +  SetBootMode (BOOT_WITH_FULL_CONFIGURATION);
>>  
>>    // Initialize Platform HOBs (CpuHob and FvHob)
>>    Status = PlatformPeim ();
>> @@ -123,9 +124,6 @@ CEntryPoint (
>>  {
>>    UINT64   StartTimeStamp;
>>  
>> -  // Initialize the platform specific controllers
>> -  ArmPlatformInitialize (MpId);
>> -
>>    if (PerformanceMeasurementEnabled ()) {
>>      // Initialize the Timer Library to setup the Timer HW controller
>>      TimerConstructor ();
>> diff --git a/ArmVirtPkg/PrePi/PrePi.h b/ArmVirtPkg/PrePi/PrePi.h
>> index d3189c0b8a6f..1ba88e0506cb 100644
>> --- a/ArmVirtPkg/PrePi/PrePi.h
>> +++ b/ArmVirtPkg/PrePi/PrePi.h
>> @@ -25,7 +25,6 @@
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/HobLib.h>
>>  #include <Library/SerialPortLib.h>
>> -#include <Library/ArmPlatformLib.h>
>>  
>>  #define SerialPrint(txt)  SerialPortWrite (txt, AsciiStrLen(txt)+1);
>>  
>>
> 
> ArmPlatformGetBootMode() and ArmPlatformInitialize() have identical
> implementations between the ArmQemuRelocatablePlatformLib and the
> ArmXenRelocatablePlatformLib instances, so I agree common handling is
> justified here.
> 
> The ArmPlatformInitialize() call is not replaced by the function's contents:
> 
>   ASSERT (!FeaturePcdGet (PcdSystemMemoryInitializeInSec));
> 
> but I guess this assertion is satisfied simply by the PrePi nature of
> the platform -- once we stop sharing the code like before, the assert
> becomes useless (there's no possible mis-build to catch). I think,
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 

heh, that was supposed to be: "I think.". Nothing else to add :)