[PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Ard Biesheuvel]

Switch to the generic implementation of the ResetSystem() runtime
service call, which is preferred over the one in EmbeddedPkg. This
involves selecting another DXE runtime driver, and morphing the
existing EfiResetSystemLib implementation into a ResetSystemLib one
(which is what the generic driver depends on for platform glue)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
 Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
 Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
 Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
 4 files changed, 87 insertions(+), 36 deletions(-)

diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
index 475fc6d05379..32eaa2942564 100644
--- a/Platforms/Marvell/Armada/Armada.dsc.inc
+++ b/Platforms/Marvell/Armada/Armada.dsc.inc
@@ -94,7 +94,7 @@
   # Reset and Time libraries
   EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
   RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
-  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
+  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
 
   # Network support
   NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
@@ -416,7 +416,7 @@
   MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
 
   EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
-  EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
+  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
   EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
 
diff --git a/Platforms/Marvell/Armada/Armada70x0.fdf b/Platforms/Marvell/Armada/Armada70x0.fdf
index 280b40be1d39..8091ea260261 100644
--- a/Platforms/Marvell/Armada/Armada70x0.fdf
+++ b/Platforms/Marvell/Armada/Armada70x0.fdf
@@ -99,7 +99,7 @@ FvNameGuid         = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
   INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
   INF EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
-  INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
+  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
   INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
   INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
diff --git a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
index c0787aceac9c..f349c5c5387b 100644
--- a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
+++ b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
@@ -66,51 +66,99 @@ LibResetSystemVirtualNotifyEvent (
 }
 
 /**
-  Resets the entire platform.
-
-  @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.
+  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.
+
+  If this function returns, it means that the system does not support cold reset.
 **/
-EFI_STATUS
+VOID
 EFIAPI
-LibResetSystem (
-  IN EFI_RESET_TYPE   ResetType,
-  IN EFI_STATUS       ResetStatus,
-  IN UINTN            DataSize,
-  IN VOID             *ResetData OPTIONAL
+ResetCold (
+  VOID
   )
 {
   UINT32   Data;
 
-  switch (ResetType) {
-  case EfiResetCold:
-  case EfiResetWarm:
-    Data = MmioRead32 (mAddress);
-    Data &= ~PcdGet32 (PcdResetRegMask);
-    MmioWrite32 (mAddress, Data);
-    break;
-  case EfiResetShutdown:
+  Data = MmioRead32 (mAddress);
+  Data &= ~PcdGet32 (PcdResetRegMask);
+  MmioWrite32 (mAddress, Data);
+}
+
+/**
+  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.
+**/
+VOID
+EFIAPI
+ResetWarm (
+  VOID
+  )
+{
+  // Map a warm reset into a cold reset
+  ResetCold ();
+}
+
+/**
+  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 shutdown reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
   //
   // Currently there is no support for power-off platform
   //
-    break;
-  default:
-    break;
-  }
+}
+
+/**
+  This function causes the system to enter S3 and then wake up immediately.
+
+  If this function returns, it means that the system does not support S3 feature.
+**/
+VOID
+EFIAPI
+EnterS3WithImmediateWake (
+  VOID
+  )
+{
+  // not implemented
+}
 
-  return EFI_DEVICE_ERROR;
+/**
+  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.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID    *ResetData
+  )
+{
+  // Map the platform specific reset as reboot
+  ResetCold ();
 }
 
-EFI_STATUS
+RETURN_STATUS
 EFIAPI
 LibInitializeResetSystem (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+  VOID
   )
 {
   UINT64 Alignment;
diff --git a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
index 87fff5707909..0d5951d7e9a0 100644
--- a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
+++ b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
@@ -1,4 +1,5 @@
 # Copyright (C) 2016 Marvell International Ltd.
+# Copyright (C) 2017 Linaro Ltd.
 #
 # Marvell BSD License Option
 #
@@ -32,16 +33,18 @@
 
 [Defines]
   INF_VERSION                    = 0x00010019
-  BASE_NAME                      = Reset
+  BASE_NAME                      = MvResetSystemLib
   FILE_GUID                      = 9d1373c0-6fac-432c-88e7-818744dc45d9
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = EfiResetSystemLib
+  LIBRARY_CLASS                  = ResetSystemLib
+  CONSTRUCTOR                    = LibInitializeResetSystem
 
 [Sources.common]
   MvResetSystemLib.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OpenPlatformPkg/Platforms/Marvell/Marvell.dec
 
-- 
2.9.3

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Marcin Wojtas]

Hi,

2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> Switch to the generic implementation of the ResetSystem() runtime
> service call, which is preferred over the one in EmbeddedPkg. This
> involves selecting another DXE runtime driver, and morphing the
> existing EfiResetSystemLib implementation into a ResetSystemLib one
> (which is what the generic driver depends on for platform glue)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>  4 files changed, 87 insertions(+), 36 deletions(-)
>
> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
> index 475fc6d05379..32eaa2942564 100644
> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
> @@ -94,7 +94,7 @@
>    # Reset and Time libraries
>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf

Isn't it better to switch to generic PSCI:
EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
and remove MvResetSystemLib whatsoever?

Best regards,
Marcin

>
>    # Network support
>    NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> @@ -416,7 +416,7 @@
>    MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>
>    EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> -  EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
> +  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
>    EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>    EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>
> diff --git a/Platforms/Marvell/Armada/Armada70x0.fdf b/Platforms/Marvell/Armada/Armada70x0.fdf
> index 280b40be1d39..8091ea260261 100644
> --- a/Platforms/Marvell/Armada/Armada70x0.fdf
> +++ b/Platforms/Marvell/Armada/Armada70x0.fdf
> @@ -99,7 +99,7 @@ FvNameGuid         = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>    INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>    INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>    INF EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
> -  INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
> +  INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
>    INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>    INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>    INF OpenPlatformPkg/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
> diff --git a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
> index c0787aceac9c..f349c5c5387b 100644
> --- a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
> +++ b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c
> @@ -66,51 +66,99 @@ LibResetSystemVirtualNotifyEvent (
>  }
>
>  /**
> -  Resets the entire platform.
> -
> -  @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.
> +  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.
> +
> +  If this function returns, it means that the system does not support cold reset.
>  **/
> -EFI_STATUS
> +VOID
>  EFIAPI
> -LibResetSystem (
> -  IN EFI_RESET_TYPE   ResetType,
> -  IN EFI_STATUS       ResetStatus,
> -  IN UINTN            DataSize,
> -  IN VOID             *ResetData OPTIONAL
> +ResetCold (
> +  VOID
>    )
>  {
>    UINT32   Data;
>
> -  switch (ResetType) {
> -  case EfiResetCold:
> -  case EfiResetWarm:
> -    Data = MmioRead32 (mAddress);
> -    Data &= ~PcdGet32 (PcdResetRegMask);
> -    MmioWrite32 (mAddress, Data);
> -    break;
> -  case EfiResetShutdown:
> +  Data = MmioRead32 (mAddress);
> +  Data &= ~PcdGet32 (PcdResetRegMask);
> +  MmioWrite32 (mAddress, Data);
> +}
> +
> +/**
> +  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.
> +**/
> +VOID
> +EFIAPI
> +ResetWarm (
> +  VOID
> +  )
> +{
> +  // Map a warm reset into a cold reset
> +  ResetCold ();
> +}
> +
> +/**
> +  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 shutdown reset.
> +**/
> +VOID
> +EFIAPI
> +ResetShutdown (
> +  VOID
> +  )
> +{
>    //
>    // Currently there is no support for power-off platform
>    //
> -    break;
> -  default:
> -    break;
> -  }
> +}
> +
> +/**
> +  This function causes the system to enter S3 and then wake up immediately.
> +
> +  If this function returns, it means that the system does not support S3 feature.
> +**/
> +VOID
> +EFIAPI
> +EnterS3WithImmediateWake (
> +  VOID
> +  )
> +{
> +  // not implemented
> +}
>
> -  return EFI_DEVICE_ERROR;
> +/**
> +  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.
> +**/
> +VOID
> +EFIAPI
> +ResetPlatformSpecific (
> +  IN UINTN   DataSize,
> +  IN VOID    *ResetData
> +  )
> +{
> +  // Map the platform specific reset as reboot
> +  ResetCold ();
>  }
>
> -EFI_STATUS
> +RETURN_STATUS
>  EFIAPI
>  LibInitializeResetSystem (
> -  IN EFI_HANDLE        ImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> +  VOID
>    )
>  {
>    UINT64 Alignment;
> diff --git a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> index 87fff5707909..0d5951d7e9a0 100644
> --- a/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> +++ b/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> @@ -1,4 +1,5 @@
>  # Copyright (C) 2016 Marvell International Ltd.
> +# Copyright (C) 2017 Linaro Ltd.
>  #
>  # Marvell BSD License Option
>  #
> @@ -32,16 +33,18 @@
>
>  [Defines]
>    INF_VERSION                    = 0x00010019
> -  BASE_NAME                      = Reset
> +  BASE_NAME                      = MvResetSystemLib
>    FILE_GUID                      = 9d1373c0-6fac-432c-88e7-818744dc45d9
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = EfiResetSystemLib
> +  LIBRARY_CLASS                  = ResetSystemLib
> +  CONSTRUCTOR                    = LibInitializeResetSystem
>
>  [Sources.common]
>    MvResetSystemLib.c
>
>  [Packages]
> +  MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
>    OpenPlatformPkg/Platforms/Marvell/Marvell.dec
>
> --
> 2.9.3
>

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Ard Biesheuvel]

On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi,
>
> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> Switch to the generic implementation of the ResetSystem() runtime
>> service call, which is preferred over the one in EmbeddedPkg. This
>> involves selecting another DXE runtime driver, and morphing the
>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>> (which is what the generic driver depends on for platform glue)
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>>  4 files changed, 87 insertions(+), 36 deletions(-)
>>
>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>> index 475fc6d05379..32eaa2942564 100644
>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>> @@ -94,7 +94,7 @@
>>    # Reset and Time libraries
>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>
> Isn't it better to switch to generic PSCI:
> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
> and remove MvResetSystemLib whatsoever?
>

Yes. But Leif tells me that 7040 does not support PSCI?

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Marcin Wojtas]

Leif, where do you have this information from?

7040 differs from 8040 only with the numeber of peripherals. It has
same AP806 HW block (CPUs, GIC, DDR controller, etc.). And, what's
most important ArmPsciResetSystemLib.inf on the other branch works on
my 7040-db board as well.

Best regards,
Marcin

2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
>> Hi,
>>
>> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>> Switch to the generic implementation of the ResetSystem() runtime
>>> service call, which is preferred over the one in EmbeddedPkg. This
>>> involves selecting another DXE runtime driver, and morphing the
>>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>>> (which is what the generic driver depends on for platform glue)
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>>>  4 files changed, 87 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>>> index 475fc6d05379..32eaa2942564 100644
>>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>>> @@ -94,7 +94,7 @@
>>>    # Reset and Time libraries
>>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>
>> Isn't it better to switch to generic PSCI:
>> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>> and remove MvResetSystemLib whatsoever?
>>
>
> Yes. But Leif tells me that 7040 does not support PSCI?

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Leif Lindholm]

On Tue, Jul 04, 2017 at 01:19:10PM +0200, Marcin Wojtas wrote:
> 2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> > On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
> >> Hi,
> >>
> >> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> >>> Switch to the generic implementation of the ResetSystem() runtime
> >>> service call, which is preferred over the one in EmbeddedPkg. This
> >>> involves selecting another DXE runtime driver, and morphing the
> >>> existing EfiResetSystemLib implementation into a ResetSystemLib one
> >>> (which is what the generic driver depends on for platform glue)
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> ---
> >>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
> >>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
> >>>  4 files changed, 87 insertions(+), 36 deletions(-)
> >>>
> >>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
> >>> index 475fc6d05379..32eaa2942564 100644
> >>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
> >>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
> >>> @@ -94,7 +94,7 @@
> >>>    # Reset and Time libraries
> >>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
> >>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
> >>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> >>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
> >>
> >> Isn't it better to switch to generic PSCI:
> >> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
> >> and remove MvResetSystemLib whatsoever?
> >>
> >
> > Yes. But Leif tells me that 7040 does not support PSCI?
>
> Leif, where do you have this information from?

>From the source code in OpenPlatformPkg, which does not use it and the
documentation that does not mention it.

If this is incorrect, please submit patches to make the code in the
tree reflect reality.

/
    Leif

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Marcin Wojtas]

2017-07-04 13:41 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Tue, Jul 04, 2017 at 01:19:10PM +0200, Marcin Wojtas wrote:
>> 2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> > On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
>> >> Hi,
>> >>
>> >> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> >>> Switch to the generic implementation of the ResetSystem() runtime
>> >>> service call, which is preferred over the one in EmbeddedPkg. This
>> >>> involves selecting another DXE runtime driver, and morphing the
>> >>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>> >>> (which is what the generic driver depends on for platform glue)
>> >>>
>> >>> Contributed-under: TianoCore Contribution Agreement 1.0
>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>> ---
>> >>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>> >>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>> >>>  4 files changed, 87 insertions(+), 36 deletions(-)
>> >>>
>> >>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>> >>> index 475fc6d05379..32eaa2942564 100644
>> >>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>> >>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>> >>> @@ -94,7 +94,7 @@
>> >>>    # Reset and Time libraries
>> >>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>> >>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>> >>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>> >>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>> >>
>> >> Isn't it better to switch to generic PSCI:
>> >> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>> >> and remove MvResetSystemLib whatsoever?
>> >>
>> >
>> > Yes. But Leif tells me that 7040 does not support PSCI?
>>
>> Leif, where do you have this information from?
>
> From the source code in OpenPlatformPkg, which does not use it and the
> documentation that does not mention it.

Ok, long time ago in early SoC revision, we PSCI was indeed broken and
later, we were not aware of the ArmPsciResetSystemLib existence:)
On the WIP branch, we have a replacement commit:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commit/b3fe877fe7207475ba293a1fc9b0a5c8f873d2c9
which will be submitted of course.

Best regards,
Marcin

>
> If this is incorrect, please submit patches to make the code in the
> tree reflect reality.
>
> /
>     Leif

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Ard Biesheuvel]

On 4 July 2017 at 12:47, Marcin Wojtas <mw@semihalf.com> wrote:
> 2017-07-04 13:41 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> On Tue, Jul 04, 2017 at 01:19:10PM +0200, Marcin Wojtas wrote:
>>> 2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>> > On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
>>> >> Hi,
>>> >>
>>> >> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>> >>> Switch to the generic implementation of the ResetSystem() runtime
>>> >>> service call, which is preferred over the one in EmbeddedPkg. This
>>> >>> involves selecting another DXE runtime driver, and morphing the
>>> >>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>>> >>> (which is what the generic driver depends on for platform glue)
>>> >>>
>>> >>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> >>> ---
>>> >>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>>> >>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>>> >>>  4 files changed, 87 insertions(+), 36 deletions(-)
>>> >>>
>>> >>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>>> >>> index 475fc6d05379..32eaa2942564 100644
>>> >>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>>> >>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>>> >>> @@ -94,7 +94,7 @@
>>> >>>    # Reset and Time libraries
>>> >>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>>> >>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>>> >>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>> >>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>> >>
>>> >> Isn't it better to switch to generic PSCI:
>>> >> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>>> >> and remove MvResetSystemLib whatsoever?
>>> >>
>>> >
>>> > Yes. But Leif tells me that 7040 does not support PSCI?
>>>
>>> Leif, where do you have this information from?
>>
>> From the source code in OpenPlatformPkg, which does not use it and the
>> documentation that does not mention it.
>
> Ok, long time ago in early SoC revision, we PSCI was indeed broken and
> later, we were not aware of the ArmPsciResetSystemLib existence:)
> On the WIP branch, we have a replacement commit:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commit/b3fe877fe7207475ba293a1fc9b0a5c8f873d2c9
> which will be submitted of course.
>

OK. But EfiSystemResetLib and EmbeddedPkg/ResetRuntimeDxe are going
away, so this commit cannot be merged as-is.

But since you are saying 7040 should use PSCI, I can update the patch
and drop MvResetSystemLib entirely.

Re: [PATCH] Platforms/Marvell: switch to generic ResetSystemRuntimeDxe

[Marcin Wojtas]

2017-07-04 13:49 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 4 July 2017 at 12:47, Marcin Wojtas <mw@semihalf.com> wrote:
>> 2017-07-04 13:41 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>>> On Tue, Jul 04, 2017 at 01:19:10PM +0200, Marcin Wojtas wrote:
>>>> 2017-07-04 13:10 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>> > On 4 July 2017 at 12:04, Marcin Wojtas <mw@semihalf.com> wrote:
>>>> >> Hi,
>>>> >>
>>>> >> 2017-07-04 13:00 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>> >>> Switch to the generic implementation of the ResetSystem() runtime
>>>> >>> service call, which is preferred over the one in EmbeddedPkg. This
>>>> >>> involves selecting another DXE runtime driver, and morphing the
>>>> >>> existing EfiResetSystemLib implementation into a ResetSystemLib one
>>>> >>> (which is what the generic driver depends on for platform glue)
>>>> >>>
>>>> >>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> >>> ---
>>>> >>>  Platforms/Marvell/Armada/Armada.dsc.inc                       |   4 +-
>>>> >>>  Platforms/Marvell/Armada/Armada70x0.fdf                       |   2 +-
>>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.c   | 110 ++++++++++++++------
>>>> >>>  Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf |   7 +-
>>>> >>>  4 files changed, 87 insertions(+), 36 deletions(-)
>>>> >>>
>>>> >>> diff --git a/Platforms/Marvell/Armada/Armada.dsc.inc b/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> index 475fc6d05379..32eaa2942564 100644
>>>> >>> --- a/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> +++ b/Platforms/Marvell/Armada/Armada.dsc.inc
>>>> >>> @@ -94,7 +94,7 @@
>>>> >>>    # Reset and Time libraries
>>>> >>>    EfiTimeBaseLib|OpenPlatformPkg/Library/EfiTimeBaseLib/EfiTimeBaseLib.inf
>>>> >>>    RealTimeClockLib|OpenPlatformPkg/Platforms/Marvell/Armada/Library/RealTimeClockLib/RealTimeClockLib.inf
>>>> >>> -  EfiResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>>> >>> +  ResetSystemLib|OpenPlatformPkg/Platforms/Marvell/Library/ResetSystemLib/MvResetSystemLib.inf
>>>> >>
>>>> >> Isn't it better to switch to generic PSCI:
>>>> >> EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
>>>> >> and remove MvResetSystemLib whatsoever?
>>>> >>
>>>> >
>>>> > Yes. But Leif tells me that 7040 does not support PSCI?
>>>>
>>>> Leif, where do you have this information from?
>>>
>>> From the source code in OpenPlatformPkg, which does not use it and the
>>> documentation that does not mention it.
>>
>> Ok, long time ago in early SoC revision, we PSCI was indeed broken and
>> later, we were not aware of the ArmPsciResetSystemLib existence:)
>> On the WIP branch, we have a replacement commit:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commit/b3fe877fe7207475ba293a1fc9b0a5c8f873d2c9
>> which will be submitted of course.
>>
>
> OK. But EfiSystemResetLib and EmbeddedPkg/ResetRuntimeDxe are going
> away, so this commit cannot be merged as-is.

I understand, I wasn't aware of those plans.

Thanks,
Marcin

>
> But since you are saying 7040 should use PSCI, I can update the patch
> and drop MvResetSystemLib entirely.