public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib
@ 2017-07-04 17:11 Leif Lindholm
  2017-07-04 17:27 ` Ard Biesheuvel
  0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2017-07-04 17:11 UTC (permalink / raw)
  To: edk2-devel; +Cc: ard.biesheuvel, ryan.harkin

Since we're in the process of migrating all of the VExpress platforms
to MdeModulePkg ResetSystemRuntimeDxe, convert VExpress ResetSystemLib
from EfiResetSystemLib interface to the ResetSystemLib one.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 .../Library/ResetSystemLib/ResetSystemLib.c        | 117 ++++++++++++---------
 .../Library/ResetSystemLib/ResetSystemLib.inf      |   6 +-
 2 files changed, 73 insertions(+), 50 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
index bafb6f8093..d2bc4a88fa 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
@@ -5,6 +5,7 @@
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
   Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -16,73 +17,95 @@
 
 **/
 
-#include <PiDxe.h>
+#include <Base.h>
 
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-#include <Library/EfiResetSystemLib.h>
 #include <Library/ArmPlatformSysConfigLib.h>
+#include <Library/DebugLib.h>
+#include <Library/ResetSystemLib.h>
 
 #include <ArmPlatform.h>
 
 /**
-  Resets the entire platform.
+  This function causes a system-wide reset (cold reset), in which
+  all circuitry within the system returns to its initial state. This type of
+  reset is asynchronous to system operation and operates without regard to
+  cycle boundaries.
 
-  @param  ResetType             The type of reset to perform.
-  @param  ResetStatus           The status code for the reset.
-  @param  DataSize              The size, in bytes, of WatchdogData.
-  @param  ResetData             For a ResetType of EfiResetCold, EfiResetWarm, or
-                                EfiResetShutdown the data buffer starts with a Null-terminated
-                                Unicode string, optionally followed by additional binary data.
+  If this function returns, it means that the system does not support cold
+  reset.
+**/
+VOID
+EFIAPI
+ResetCold (
+  VOID
+  )
+{
+  ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
+}
+
+/**
+  This function causes a system-wide initialization (warm reset), in which all
+  processors are set to their initial state. Pending cycles are not corrupted.
 
+  If this function returns, it means that the system does not support warm
+  reset.
 **/
-EFI_STATUS
+VOID
 EFIAPI
-LibResetSystem (
-  IN EFI_RESET_TYPE   ResetType,
-  IN EFI_STATUS       ResetStatus,
-  IN UINTN            DataSize,
-  IN CHAR16           *ResetData OPTIONAL
+ResetWarm (
+  VOID
   )
 {
-  switch (ResetType) {
-  case EfiResetPlatformSpecific:
-    // Map the platform specific reset as reboot
-  case EfiResetWarm:
-    // Map a warm reset into a cold reset
-  case EfiResetCold:
-    // Send the REBOOT function to the platform microcontroller
-    ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
-
-    // We should never be here
-    while(1);
-  case EfiResetShutdown:
-    // Send the SHUTDOWN function to the platform microcontroller
-    ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
-
-    // We should never be here
-    while(1);
-  }
-
-  ASSERT(FALSE);
-  return EFI_UNSUPPORTED;
+  ResetCold ();
 }
 
 /**
-  Initialize any infrastructure required for LibResetSystem () to function.
+  This function causes the system to enter a power state equivalent
+  to the ACPI G2/S5 or G3 states.
+
+  If this function returns, it means that the system does not support shut down reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
+}
 
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
+/**
+  This function causes the system to enter S3 and then wake up immediately.
 
-  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+  If this function returns, it means that the system does not support S3
+  feature.
+**/
+VOID
+EFIAPI
+EnterS3WithImmediateWake (
+  VOID
+  )
+{
+  // not implemented
+}
 
+/**
+  This function causes a systemwide reset. The exact type of the reset is
+  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
+  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
+  the platform must pick a supported reset type to perform.The platform may
+  optionally log the parameters from any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
+                         followed by the EFI_GUID.
 **/
-EFI_STATUS
+VOID
 EFIAPI
-LibInitializeResetSystem (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID    *ResetData
   )
 {
-  return EFI_SUCCESS;
+  ResetCold ();
 }
diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
index 8c17ea8cdb..780636c1de 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
@@ -2,6 +2,7 @@
 # Reset System lib to make it easy to port new platforms
 #
 # Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -19,17 +20,16 @@
   FILE_GUID                      = 36885202-0854-4373-bfd2-95d229b44d44
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = EfiResetSystemLib
+  LIBRARY_CLASS                  = ResetSystemLib
 
 [Sources.common]
   ResetSystemLib.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
 
 [LibraryClasses]
   DebugLib
-  BaseLib
   ArmPlatformSysConfigLib
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib
  2017-07-04 17:11 [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib Leif Lindholm
@ 2017-07-04 17:27 ` Ard Biesheuvel
  2017-07-05 11:20   ` Ryan Harkin
  0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2017-07-04 17:27 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Ryan Harkin

On 4 July 2017 at 18:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Since we're in the process of migrating all of the VExpress platforms
> to MdeModulePkg ResetSystemRuntimeDxe, convert VExpress ResetSystemLib
> from EfiResetSystemLib interface to the ResetSystemLib one.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  .../Library/ResetSystemLib/ResetSystemLib.c        | 117 ++++++++++++---------
>  .../Library/ResetSystemLib/ResetSystemLib.inf      |   6 +-
>  2 files changed, 73 insertions(+), 50 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> index bafb6f8093..d2bc4a88fa 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> @@ -5,6 +5,7 @@
>
>    Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>    Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -16,73 +17,95 @@
>
>  **/
>
> -#include <PiDxe.h>
> +#include <Base.h>
>
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/EfiResetSystemLib.h>
>  #include <Library/ArmPlatformSysConfigLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/ResetSystemLib.h>
>
>  #include <ArmPlatform.h>
>
>  /**
> -  Resets the entire platform.
> +  This function causes a system-wide reset (cold reset), in which
> +  all circuitry within the system returns to its initial state. This type of
> +  reset is asynchronous to system operation and operates without regard to
> +  cycle boundaries.
>
> -  @param  ResetType             The type of reset to perform.
> -  @param  ResetStatus           The status code for the reset.
> -  @param  DataSize              The size, in bytes, of WatchdogData.
> -  @param  ResetData             For a ResetType of EfiResetCold, EfiResetWarm, or
> -                                EfiResetShutdown the data buffer starts with a Null-terminated
> -                                Unicode string, optionally followed by additional binary data.
> +  If this function returns, it means that the system does not support cold
> +  reset.
> +**/
> +VOID
> +EFIAPI
> +ResetCold (
> +  VOID
> +  )
> +{
> +  ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
> +}
> +
> +/**
> +  This function causes a system-wide initialization (warm reset), in which all
> +  processors are set to their initial state. Pending cycles are not corrupted.
>
> +  If this function returns, it means that the system does not support warm
> +  reset.
>  **/
> -EFI_STATUS
> +VOID
>  EFIAPI
> -LibResetSystem (
> -  IN EFI_RESET_TYPE   ResetType,
> -  IN EFI_STATUS       ResetStatus,
> -  IN UINTN            DataSize,
> -  IN CHAR16           *ResetData OPTIONAL
> +ResetWarm (
> +  VOID
>    )
>  {
> -  switch (ResetType) {
> -  case EfiResetPlatformSpecific:
> -    // Map the platform specific reset as reboot
> -  case EfiResetWarm:
> -    // Map a warm reset into a cold reset
> -  case EfiResetCold:
> -    // Send the REBOOT function to the platform microcontroller
> -    ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
> -
> -    // We should never be here
> -    while(1);
> -  case EfiResetShutdown:
> -    // Send the SHUTDOWN function to the platform microcontroller
> -    ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
> -
> -    // We should never be here
> -    while(1);
> -  }
> -
> -  ASSERT(FALSE);
> -  return EFI_UNSUPPORTED;
> +  ResetCold ();
>  }
>
>  /**
> -  Initialize any infrastructure required for LibResetSystem () to function.
> +  This function causes the system to enter a power state equivalent
> +  to the ACPI G2/S5 or G3 states.
> +
> +  If this function returns, it means that the system does not support shut down reset.
> +**/
> +VOID
> +EFIAPI
> +ResetShutdown (
> +  VOID
> +  )
> +{
> +  ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
> +}
>
> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
> -  @param  SystemTable   A pointer to the EFI System Table.
> +/**
> +  This function causes the system to enter S3 and then wake up immediately.
>
> -  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> +  If this function returns, it means that the system does not support S3
> +  feature.
> +**/
> +VOID
> +EFIAPI
> +EnterS3WithImmediateWake (
> +  VOID
> +  )
> +{
> +  // not implemented
> +}
>
> +/**
> +  This function causes a systemwide reset. The exact type of the reset is
> +  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
> +  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
> +  the platform must pick a supported reset type to perform.The platform may
> +  optionally log the parameters from any non-normal reset that occurs.
> +
> +  @param[in]  DataSize   The size, in bytes, of ResetData.
> +  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
> +                         followed by the EFI_GUID.
>  **/
> -EFI_STATUS
> +VOID
>  EFIAPI
> -LibInitializeResetSystem (
> -  IN EFI_HANDLE        ImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> +ResetPlatformSpecific (
> +  IN UINTN   DataSize,
> +  IN VOID    *ResetData
>    )
>  {
> -  return EFI_SUCCESS;
> +  ResetCold ();
>  }
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> index 8c17ea8cdb..780636c1de 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> @@ -2,6 +2,7 @@
>  # Reset System lib to make it easy to port new platforms
>  #
>  # Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
> +# Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
> @@ -19,17 +20,16 @@
>    FILE_GUID                      = 36885202-0854-4373-bfd2-95d229b44d44
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = EfiResetSystemLib
> +  LIBRARY_CLASS                  = ResetSystemLib
>
>  [Sources.common]
>    ResetSystemLib.c
>
>  [Packages]
> +  MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
>
>  [LibraryClasses]
>    DebugLib
> -  BaseLib
>    ArmPlatformSysConfigLib
> --
> 2.11.0
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib
  2017-07-04 17:27 ` Ard Biesheuvel
@ 2017-07-05 11:20   ` Ryan Harkin
  2017-07-05 15:30     ` Leif Lindholm
  0 siblings, 1 reply; 4+ messages in thread
From: Ryan Harkin @ 2017-07-05 11:20 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Leif Lindholm, edk2-devel@lists.01.org

On 4 July 2017 at 18:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 4 July 2017 at 18:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> Since we're in the process of migrating all of the VExpress platforms
>> to MdeModulePkg ResetSystemRuntimeDxe, convert VExpress ResetSystemLib
>> from EfiResetSystemLib interface to the ResetSystemLib one.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

After popping commit e4129b0e5897d76885170bec9da996b266f185f9 off the
top of edk2, and adding this, I also applied patch "Platforms/ARM:
move ARM platforms to generic ResetSystemRuntimeDxe" to
OpenPlatformPkg and tested on TC2, FVP Foundation and AEMv8 models and
Juno R0/1/2.

I checked that reset and shutdown both still work as expected. Reset
was triggered from BDS, grub and EFI Shell. Shutdown (reset -s) was
trigger from EFI Shell.

Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>
>> ---
>>  .../Library/ResetSystemLib/ResetSystemLib.c        | 117 ++++++++++++---------
>>  .../Library/ResetSystemLib/ResetSystemLib.inf      |   6 +-
>>  2 files changed, 73 insertions(+), 50 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
>> index bafb6f8093..d2bc4a88fa 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
>> @@ -5,6 +5,7 @@
>>
>>    Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>>    Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
>> +  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
>>
>>    This program and the accompanying materials
>>    are licensed and made available under the terms and conditions of the BSD License
>> @@ -16,73 +17,95 @@
>>
>>  **/
>>
>> -#include <PiDxe.h>
>> +#include <Base.h>
>>
>> -#include <Library/BaseLib.h>
>> -#include <Library/DebugLib.h>
>> -#include <Library/EfiResetSystemLib.h>
>>  #include <Library/ArmPlatformSysConfigLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/ResetSystemLib.h>
>>
>>  #include <ArmPlatform.h>
>>
>>  /**
>> -  Resets the entire platform.
>> +  This function causes a system-wide reset (cold reset), in which
>> +  all circuitry within the system returns to its initial state. This type of
>> +  reset is asynchronous to system operation and operates without regard to
>> +  cycle boundaries.
>>
>> -  @param  ResetType             The type of reset to perform.
>> -  @param  ResetStatus           The status code for the reset.
>> -  @param  DataSize              The size, in bytes, of WatchdogData.
>> -  @param  ResetData             For a ResetType of EfiResetCold, EfiResetWarm, or
>> -                                EfiResetShutdown the data buffer starts with a Null-terminated
>> -                                Unicode string, optionally followed by additional binary data.
>> +  If this function returns, it means that the system does not support cold
>> +  reset.
>> +**/
>> +VOID
>> +EFIAPI
>> +ResetCold (
>> +  VOID
>> +  )
>> +{
>> +  ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
>> +}
>> +
>> +/**
>> +  This function causes a system-wide initialization (warm reset), in which all
>> +  processors are set to their initial state. Pending cycles are not corrupted.
>>
>> +  If this function returns, it means that the system does not support warm
>> +  reset.
>>  **/
>> -EFI_STATUS
>> +VOID
>>  EFIAPI
>> -LibResetSystem (
>> -  IN EFI_RESET_TYPE   ResetType,
>> -  IN EFI_STATUS       ResetStatus,
>> -  IN UINTN            DataSize,
>> -  IN CHAR16           *ResetData OPTIONAL
>> +ResetWarm (
>> +  VOID
>>    )
>>  {
>> -  switch (ResetType) {
>> -  case EfiResetPlatformSpecific:
>> -    // Map the platform specific reset as reboot
>> -  case EfiResetWarm:
>> -    // Map a warm reset into a cold reset
>> -  case EfiResetCold:
>> -    // Send the REBOOT function to the platform microcontroller
>> -    ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
>> -
>> -    // We should never be here
>> -    while(1);
>> -  case EfiResetShutdown:
>> -    // Send the SHUTDOWN function to the platform microcontroller
>> -    ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
>> -
>> -    // We should never be here
>> -    while(1);
>> -  }
>> -
>> -  ASSERT(FALSE);
>> -  return EFI_UNSUPPORTED;
>> +  ResetCold ();
>>  }
>>
>>  /**
>> -  Initialize any infrastructure required for LibResetSystem () to function.
>> +  This function causes the system to enter a power state equivalent
>> +  to the ACPI G2/S5 or G3 states.
>> +
>> +  If this function returns, it means that the system does not support shut down reset.
>> +**/
>> +VOID
>> +EFIAPI
>> +ResetShutdown (
>> +  VOID
>> +  )
>> +{
>> +  ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
>> +}
>>
>> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
>> -  @param  SystemTable   A pointer to the EFI System Table.
>> +/**
>> +  This function causes the system to enter S3 and then wake up immediately.
>>
>> -  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
>> +  If this function returns, it means that the system does not support S3
>> +  feature.
>> +**/
>> +VOID
>> +EFIAPI
>> +EnterS3WithImmediateWake (
>> +  VOID
>> +  )
>> +{
>> +  // not implemented
>> +}
>>
>> +/**
>> +  This function causes a systemwide reset. The exact type of the reset is
>> +  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
>> +  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
>> +  the platform must pick a supported reset type to perform.The platform may
>> +  optionally log the parameters from any non-normal reset that occurs.
>> +
>> +  @param[in]  DataSize   The size, in bytes, of ResetData.
>> +  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
>> +                         followed by the EFI_GUID.
>>  **/
>> -EFI_STATUS
>> +VOID
>>  EFIAPI
>> -LibInitializeResetSystem (
>> -  IN EFI_HANDLE        ImageHandle,
>> -  IN EFI_SYSTEM_TABLE  *SystemTable
>> +ResetPlatformSpecific (
>> +  IN UINTN   DataSize,
>> +  IN VOID    *ResetData
>>    )
>>  {
>> -  return EFI_SUCCESS;
>> +  ResetCold ();
>>  }
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
>> index 8c17ea8cdb..780636c1de 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
>> @@ -2,6 +2,7 @@
>>  # Reset System lib to make it easy to port new platforms
>>  #
>>  # Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
>> +# Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
>>  #
>>  #  This program and the accompanying materials
>>  #  are licensed and made available under the terms and conditions of the BSD License
>> @@ -19,17 +20,16 @@
>>    FILE_GUID                      = 36885202-0854-4373-bfd2-95d229b44d44
>>    MODULE_TYPE                    = BASE
>>    VERSION_STRING                 = 1.0
>> -  LIBRARY_CLASS                  = EfiResetSystemLib
>> +  LIBRARY_CLASS                  = ResetSystemLib
>>
>>  [Sources.common]
>>    ResetSystemLib.c
>>
>>  [Packages]
>> +  MdeModulePkg/MdeModulePkg.dec
>>    MdePkg/MdePkg.dec
>> -  EmbeddedPkg/EmbeddedPkg.dec
>>    ArmPlatformPkg/ArmPlatformPkg.dec
>>
>>  [LibraryClasses]
>>    DebugLib
>> -  BaseLib
>>    ArmPlatformSysConfigLib
>> --
>> 2.11.0
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib
  2017-07-05 11:20   ` Ryan Harkin
@ 2017-07-05 15:30     ` Leif Lindholm
  0 siblings, 0 replies; 4+ messages in thread
From: Leif Lindholm @ 2017-07-05 15:30 UTC (permalink / raw)
  To: Ryan Harkin; +Cc: Ard Biesheuvel, edk2-devel@lists.01.org

On Wed, Jul 05, 2017 at 12:20:14PM +0100, Ryan Harkin wrote:
> On 4 July 2017 at 18:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 4 July 2017 at 18:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> Since we're in the process of migrating all of the VExpress platforms
> >> to MdeModulePkg ResetSystemRuntimeDxe, convert VExpress ResetSystemLib
> >> from EfiResetSystemLib interface to the ResetSystemLib one.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> After popping commit e4129b0e5897d76885170bec9da996b266f185f9 off the
> top of edk2, and adding this, I also applied patch "Platforms/ARM:
> move ARM platforms to generic ResetSystemRuntimeDxe" to
> OpenPlatformPkg and tested on TC2, FVP Foundation and AEMv8 models and
> Juno R0/1/2.
> 
> I checked that reset and shutdown both still work as expected. Reset
> was triggered from BDS, grub and EFI Shell. Shutdown (reset -s) was
> trigger from EFI Shell.
> 
> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

Thanks guys.
This end pushed as 729ddffda0.

> >
> >> ---
> >>  .../Library/ResetSystemLib/ResetSystemLib.c        | 117 ++++++++++++---------
> >>  .../Library/ResetSystemLib/ResetSystemLib.inf      |   6 +-
> >>  2 files changed, 73 insertions(+), 50 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> >> index bafb6f8093..d2bc4a88fa 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.c
> >> @@ -5,6 +5,7 @@
> >>
> >>    Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> >>    Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
> >> +  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> >>
> >>    This program and the accompanying materials
> >>    are licensed and made available under the terms and conditions of the BSD License
> >> @@ -16,73 +17,95 @@
> >>
> >>  **/
> >>
> >> -#include <PiDxe.h>
> >> +#include <Base.h>
> >>
> >> -#include <Library/BaseLib.h>
> >> -#include <Library/DebugLib.h>
> >> -#include <Library/EfiResetSystemLib.h>
> >>  #include <Library/ArmPlatformSysConfigLib.h>
> >> +#include <Library/DebugLib.h>
> >> +#include <Library/ResetSystemLib.h>
> >>
> >>  #include <ArmPlatform.h>
> >>
> >>  /**
> >> -  Resets the entire platform.
> >> +  This function causes a system-wide reset (cold reset), in which
> >> +  all circuitry within the system returns to its initial state. This type of
> >> +  reset is asynchronous to system operation and operates without regard to
> >> +  cycle boundaries.
> >>
> >> -  @param  ResetType             The type of reset to perform.
> >> -  @param  ResetStatus           The status code for the reset.
> >> -  @param  DataSize              The size, in bytes, of WatchdogData.
> >> -  @param  ResetData             For a ResetType of EfiResetCold, EfiResetWarm, or
> >> -                                EfiResetShutdown the data buffer starts with a Null-terminated
> >> -                                Unicode string, optionally followed by additional binary data.
> >> +  If this function returns, it means that the system does not support cold
> >> +  reset.
> >> +**/
> >> +VOID
> >> +EFIAPI
> >> +ResetCold (
> >> +  VOID
> >> +  )
> >> +{
> >> +  ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
> >> +}
> >> +
> >> +/**
> >> +  This function causes a system-wide initialization (warm reset), in which all
> >> +  processors are set to their initial state. Pending cycles are not corrupted.
> >>
> >> +  If this function returns, it means that the system does not support warm
> >> +  reset.
> >>  **/
> >> -EFI_STATUS
> >> +VOID
> >>  EFIAPI
> >> -LibResetSystem (
> >> -  IN EFI_RESET_TYPE   ResetType,
> >> -  IN EFI_STATUS       ResetStatus,
> >> -  IN UINTN            DataSize,
> >> -  IN CHAR16           *ResetData OPTIONAL
> >> +ResetWarm (
> >> +  VOID
> >>    )
> >>  {
> >> -  switch (ResetType) {
> >> -  case EfiResetPlatformSpecific:
> >> -    // Map the platform specific reset as reboot
> >> -  case EfiResetWarm:
> >> -    // Map a warm reset into a cold reset
> >> -  case EfiResetCold:
> >> -    // Send the REBOOT function to the platform microcontroller
> >> -    ArmPlatformSysConfigSet (SYS_CFG_REBOOT, 0);
> >> -
> >> -    // We should never be here
> >> -    while(1);
> >> -  case EfiResetShutdown:
> >> -    // Send the SHUTDOWN function to the platform microcontroller
> >> -    ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
> >> -
> >> -    // We should never be here
> >> -    while(1);
> >> -  }
> >> -
> >> -  ASSERT(FALSE);
> >> -  return EFI_UNSUPPORTED;
> >> +  ResetCold ();
> >>  }
> >>
> >>  /**
> >> -  Initialize any infrastructure required for LibResetSystem () to function.
> >> +  This function causes the system to enter a power state equivalent
> >> +  to the ACPI G2/S5 or G3 states.
> >> +
> >> +  If this function returns, it means that the system does not support shut down reset.
> >> +**/
> >> +VOID
> >> +EFIAPI
> >> +ResetShutdown (
> >> +  VOID
> >> +  )
> >> +{
> >> +  ArmPlatformSysConfigSet (SYS_CFG_SHUTDOWN, 0);
> >> +}
> >>
> >> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
> >> -  @param  SystemTable   A pointer to the EFI System Table.
> >> +/**
> >> +  This function causes the system to enter S3 and then wake up immediately.
> >>
> >> -  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> >> +  If this function returns, it means that the system does not support S3
> >> +  feature.
> >> +**/
> >> +VOID
> >> +EFIAPI
> >> +EnterS3WithImmediateWake (
> >> +  VOID
> >> +  )
> >> +{
> >> +  // not implemented
> >> +}
> >>
> >> +/**
> >> +  This function causes a systemwide reset. The exact type of the reset is
> >> +  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
> >> +  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
> >> +  the platform must pick a supported reset type to perform.The platform may
> >> +  optionally log the parameters from any non-normal reset that occurs.
> >> +
> >> +  @param[in]  DataSize   The size, in bytes, of ResetData.
> >> +  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
> >> +                         followed by the EFI_GUID.
> >>  **/
> >> -EFI_STATUS
> >> +VOID
> >>  EFIAPI
> >> -LibInitializeResetSystem (
> >> -  IN EFI_HANDLE        ImageHandle,
> >> -  IN EFI_SYSTEM_TABLE  *SystemTable
> >> +ResetPlatformSpecific (
> >> +  IN UINTN   DataSize,
> >> +  IN VOID    *ResetData
> >>    )
> >>  {
> >> -  return EFI_SUCCESS;
> >> +  ResetCold ();
> >>  }
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >> index 8c17ea8cdb..780636c1de 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ResetSystemLib/ResetSystemLib.inf
> >> @@ -2,6 +2,7 @@
> >>  # Reset System lib to make it easy to port new platforms
> >>  #
> >>  # Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
> >> +# Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> >>  #
> >>  #  This program and the accompanying materials
> >>  #  are licensed and made available under the terms and conditions of the BSD License
> >> @@ -19,17 +20,16 @@
> >>    FILE_GUID                      = 36885202-0854-4373-bfd2-95d229b44d44
> >>    MODULE_TYPE                    = BASE
> >>    VERSION_STRING                 = 1.0
> >> -  LIBRARY_CLASS                  = EfiResetSystemLib
> >> +  LIBRARY_CLASS                  = ResetSystemLib
> >>
> >>  [Sources.common]
> >>    ResetSystemLib.c
> >>
> >>  [Packages]
> >> +  MdeModulePkg/MdeModulePkg.dec
> >>    MdePkg/MdePkg.dec
> >> -  EmbeddedPkg/EmbeddedPkg.dec
> >>    ArmPlatformPkg/ArmPlatformPkg.dec
> >>
> >>  [LibraryClasses]
> >>    DebugLib
> >> -  BaseLib
> >>    ArmPlatformSysConfigLib
> >> --
> >> 2.11.0
> >>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-05 15:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-04 17:11 [PATCH v2] ArmPlatformPkg: convert VExpress ResetSystemLib to ResetSystemLib Leif Lindholm
2017-07-04 17:27 ` Ard Biesheuvel
2017-07-05 11:20   ` Ryan Harkin
2017-07-05 15:30     ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox