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

Ard Biesheuvel posted 15 patches 7 years, 5 months ago
There is a newer version of this series
[edk2] [PATCH 05/15] ArmVirtPkg/PrePi: remove dependency on ArmPlatformLib
Posted by Ard Biesheuvel 7 years, 5 months ago
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);
 
-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 05/15] ArmVirtPkg/PrePi: remove dependency on ArmPlatformLib
Posted by Laszlo Ersek 7 years, 5 months ago
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>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 05/15] ArmVirtPkg/PrePi: remove dependency on ArmPlatformLib
Posted by Laszlo Ersek 7 years, 5 months ago
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 :)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel