[edk2-platforms PATCH 0/6] Marvell ACS fixes

[edk2-platforms PATCH 0/6] Marvell ACS fixes

[Marcin Wojtas]

Hi,

This series addresses a couple of issues found during
ACS3.0 test suit runs. The SMBIOS Type17 table
extension and RTC fixes/improvements.
Moreover the SPCR console subtype is switched to
a different value and Cn913xDb ACPI tables are
fixed, so that to achieve proper Windows 10 PE boot.

More details can be found in the commit logs.
The patchest is publicly available in the github:
https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/marvell-acs-r20210524

Best regards,
Marcin

Marcin Wojtas (6):
  Marvell/Drivers: SmbiosPlatformDxe: Align Type17 to SMBIOS v3.2
  Marvell: Armada7k8k/OcteonTx: Fix RT debug prints
  Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1
  Marvell/Cn913xDbA: AcpiTables: Use unique UID's
  Marvell: RealTimeClockLib: Fix daylight and timezone handling
  Marvell: RealTimeClockLib: Rework LibGetWakeupTime/LibSetWakeupTime

 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                          |  2 -
 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h |  2 +-
 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 32 ++++++++--------
 Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c          | 39 ++++++++++++++++++--
 Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc                        |  2 +-
 Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl             |  6 +--
 Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc                      |  2 +-
 7 files changed, 56 insertions(+), 29 deletions(-)

-- 
2.29.0

[edk2-platforms PATCH 1/6] Marvell/Drivers: SmbiosPlatformDxe: Align Type17 to SMBIOS v3.2

[Marcin Wojtas]

This patch adds missing entries required for SMBIOS v3.2 compliance
of the Type17 table. On the occasion improve Type4 table contents.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 39 ++++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 2ecaec2af5..a99291e902 100644
--- a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -181,7 +181,7 @@ STATIC SMBIOS_TABLE_TYPE4 mArmadaDefaultType4 = {
   3,             //version
   {0,0,0,0,0,1}, //voltage
   0,             //external clock
-  2000,          //max speed
+  2200,          //max speed
   0,             //current speed - requires update
   0x41,          //status
   ProcessorUpgradeOther,
@@ -196,6 +196,9 @@ STATIC SMBIOS_TABLE_TYPE4 mArmadaDefaultType4 = {
   4,             //threads per socket
   0xEC,          //processor characteristics
   ProcessorFamilyARM, //ARM core
+  0,             // CoreCount2;
+  0,             // EnabledCoreCount2;
+  0,             // ThreadCount2;
 };
 
 STATIC CHAR8 CONST *mArmadaDefaultType4Strings[] = {
@@ -457,7 +460,7 @@ STATIC SMBIOS_TABLE_TYPE17 mArmadaDefaultType17 = {
   0,  //Memory size obtained dynamically
   MemoryFormFactorRowOfChips,      //Memory factor
   0,                               //Not part of a set
-  1,                               //Right side of board
+  1,                               //Location
   2,                               //Bank 0
   MemoryTypeDdr4,                  //DDR4
   {0,0,0,0,0,0,0,0,0,0,0,0,0,0,1}, //unbuffered
@@ -467,10 +470,36 @@ STATIC SMBIOS_TABLE_TYPE17 mArmadaDefaultType17 = {
   0, //asset tag
   0, //part number
   0, //rank
+  0, // ExtendedSize; (since Size < 32GB-1)
+  0, // ConfiguredMemoryClockSpeed - initialized at runtime
+  0, // MinimumVoltage; (unknown)
+  0, // MaximumVoltage; (unknown)
+  0, // ConfiguredVoltage; (unknown)
+  MemoryTechnologyDram, // MemoryTechnology
+  {{                    // MemoryOperatingModeCapability
+    0,  // Reserved                        :1;
+    0,  // Other                           :1;
+    0,  // Unknown                         :1;
+    1,  // VolatileMemory                  :1;
+    0,  // ByteAccessiblePersistentMemory  :1;
+    0,  // BlockAccessiblePersistentMemory :1;
+    0   // Reserved                        :10;
+  }},
+  0, // FirwareVersion
+  0, // ModuleManufacturerID (unknown)
+  0, // ModuleProductID (unknown)
+  0, // MemorySubsystemControllerManufacturerID (unknown)
+  0, // MemorySubsystemControllerProductID (unknown)
+  0, // NonVolatileSize
+  0, // VolatileSize - initialized at runtime
+  0, // CacheSize
+  0, // LogicalSize
+  0, // ExtendedSpeed,
+  0  // ExtendedConfiguredMemorySpeed
 };
 
 STATIC CHAR8 CONST *mArmadaDefaultType17Strings[] = {
-  "RIGHT SIDE\0",                     /* location */
+  "DIMM SLOT\0",                      /* location */
   "BANK 0\0",                         /* bank description */
   NULL
 };
@@ -735,9 +764,10 @@ SmbiosMemoryInstall (
   }
 
   //
-  // Update TYPE17 memory size field
+  // Update TYPE17 memory size fields
   //
   mArmadaDefaultType17.Size = (UINT16)(MemorySize >> 20);
+  mArmadaDefaultType17.VolatileSize = MemorySize;
 
   return EFI_SUCCESS;
 }
@@ -767,6 +797,7 @@ SmbiosInstallAllStructures (
   mArmadaDefaultType0.SystemBiosMinorRelease = FirmwareMinorRevisionNumber;
   mArmadaDefaultType4.CurrentSpeed = SampleAtResetGetCpuFrequency ();
   mArmadaDefaultType17.Speed = SampleAtResetGetDramFrequency ();
+  mArmadaDefaultType17.ConfiguredMemoryClockSpeed = SampleAtResetGetDramFrequency ();
 
   //
   // Generate memory descriptors.
-- 
2.29.0

[edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT debug prints

[Marcin Wojtas]

Resolution of the DebugLib for the DXE_RUNTIME_DRIVER library
class was limited to non-RELEASE builds. This caused crashes
during FWTS in case the RT attempted to use UART. Fix that
by allowing to use DxeRuntimeDebugLibSerialPort in all kind
of builds.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 4cdafe8b1f..939fbf14d9 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -195,9 +195,7 @@
 !else
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
 !endif
-!if $(TARGET) != RELEASE
   DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
-!endif
   VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
 
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
-- 
2.29.0

[edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1

[Marcin Wojtas]

DBG2 ACPI table description [1] specifies three subtypes
related to 16550 UART:
0x0 - 16550 compatible
0x1 - 16550 subset
0x12 - 16550 compatible with parameters defined in
       Generic Address Structure (GAS)

It turned out however, that the Windows OS treats 0x0 subtype as
legacy x86 UART with 8-bit access. ARM SoCs can use types 0x1 (16550 with
fixed mmio32 access) or 0x12 (16550 with fully respected GAS contents).

Switch Marvell SoCs ACPI UART subtype to 0x1 - thanks to that the
same firmware can run properly with UART output in Windows 10, Linux
and ESXI hypervisor.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc   | 2 +-
 Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
index 438cf7880e..6efc175bdf 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
@@ -22,7 +22,7 @@ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
   ),
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550,  // InterfaceType
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16450,  // InterfaceType
   { EFI_ACPI_RESERVED_BYTE,
     EFI_ACPI_RESERVED_BYTE,
     EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
index f663d8ade8..2a3415f0a6 100644
--- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
+++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
@@ -22,7 +22,7 @@ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
   ),
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550,  // InterfaceType
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16450,  // InterfaceType
   { EFI_ACPI_RESERVED_BYTE,
     EFI_ACPI_RESERVED_BYTE,
     EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
-- 
2.29.0

[edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use unique UID's

[Marcin Wojtas]

The CN9131 variant's SSDT comprised UID's, whose values
overlapped the ones used in the main DSDT file. Fix that.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
index dc32fe836a..691a709c18 100644
--- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
+++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
@@ -18,7 +18,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
         Device (AHC1)
         {
             Name (_HID, "LNRO001E")     // _HID: Hardware ID
-            Name (_UID, 0x00)           // _UID: Unique ID
+            Name (_UID, 0x01)           // _UID: Unique ID
             Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
             Name (_CLS, Package (0x03)  // _CLS: Class Code
             {
@@ -43,7 +43,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
         Device (XHC2)
         {
             Name (_HID, "PNP0D10")      // _HID: Hardware ID
-            Name (_UID, 0x01)           // _UID: Unique ID
+            Name (_UID, 0x02)           // _UID: Unique ID
             Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
 
             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
@@ -62,7 +62,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2, "MVEBU ", "CN9131", 3)
         {
             Name (_HID, "MRVL0110")                             // _HID: Hardware ID
             Name (_CCA, 0x01)                                   // Cache-coherent controller
-            Name (_UID, 0x00)                                   // _UID: Unique ID
+            Name (_UID, 0x01)                                   // _UID: Unique ID
             Name (_CRS, ResourceTemplate ()
             {
                 Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
-- 
2.29.0

[edk2-platforms PATCH 5/6] Marvell: RealTimeClockLib: Fix daylight and timezone handling

[Marcin Wojtas]

The Marvell implementation of the RealTimeClockLib was unnecessarily
overriding the daylight and timezone values, which are handled
by non-volatile variables in the generic code. Fix that.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
index 40ab01ed41..a48d44ed83 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
@@ -79,9 +79,6 @@ LibGetTime (
   // Convert from internal 32-bit time to UEFI time
   EpochToEfiTime (RegVal, Time);
 
-  Time->TimeZone = EFI_UNSPECIFIED_TIMEZONE;
-  Time->Daylight = 0;
-
   return Status;
 }
 
-- 
2.29.0

[edk2-platforms PATCH 6/6] Marvell: RealTimeClockLib: Rework LibGetWakeupTime/LibSetWakeupTime

[Marcin Wojtas]

Apply multiple fixes to the Marvell RealTimeClockLib wakeup
library callbacks.

LibGetWakeupTime:
* Add input parameters validation
* Fix 'Pending' value check

LibSetWakeupTime:
* Allow disabling the wakeup timer regardless the input 'Time' value
* Use more generic 'Time' value verification, which is more strict
  than the replaced custom one.
* Use proper alarm mask for 'Pending' signalling

With above the ACS3.0 FWTS and SCT timer tests pass cleanly.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h |  2 +-
 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 29 ++++++++++----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
index 7fa1d092e4..c33e63d107 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
@@ -17,7 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define RTC_BRIDGE_TIMING_CTRL0_REG_OFFS        0x80
 #define RTC_BRIDGE_TIMING_CTRL1_REG_OFFS        0x84
 #define RTC_IRQ_STATUS_REG                      0x90
-#define RTC_IRQ_ALARM_MASK                      0x1
+#define RTC_IRQ_ALARM_MASK                      0x2
 #define RTC_WRITE_PERIOD_DELAY_MASK             0xFFFF
 #define RTC_WRITE_PERIOD_DELAY_DEFAULT          0x3FF
 #define RTC_WRITE_SETUP_DELAY_MASK              (0xFFFF << 16)
diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
index a48d44ed83..49c9385d53 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
@@ -140,11 +140,15 @@ LibGetWakeupTime (
 {
   UINT32 WakeupSeconds;
 
+  if (Time == NULL || Enabled == NULL || Pending == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   *Enabled = MmioRead32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG) & RTC_IRQ_ALARM_EN;
 
   *Pending = MmioRead32 (mArmadaRtcBase + RTC_IRQ_STATUS_REG) & RTC_IRQ_ALARM_MASK;
   // Ack pending alarm
-  if (Pending) {
+  if (*Pending) {
     MmioWrite32 (mArmadaRtcBase + RTC_IRQ_STATUS_REG, RTC_IRQ_ALARM_MASK);
   }
 
@@ -176,14 +180,14 @@ LibSetWakeupTime (
 {
   UINTN       WakeupSeconds;
 
-  //
-  // Because the Armada RTC uses a 32-bit counter for seconds,
-  // the maximum time span is just over 136 years.
-  // Time is stored in Unix Epoch format, so it starts in 1970,
-  // Therefore it can not exceed the year 2106.
-  //
-  if ((Time->Year < 1970) || (Time->Year >= 2106)) {
-    return EFI_UNSUPPORTED;
+  // Handle timer disabling case
+  if (!Enabled) {
+    RtcDelayedWrite (RTC_IRQ_2_CONFIG_REG, 0);
+    return EFI_SUCCESS;
+  }
+
+  if (Time == NULL || !IsTimeValid (Time)) {
+    return EFI_INVALID_PARAMETER;
   }
 
   // Convert time to raw seconds
@@ -195,11 +199,8 @@ LibSetWakeupTime (
   // Issue delayed write to alarm register
   RtcDelayedWrite (RTC_ALARM_2_REG, (UINT32)WakeupSeconds);
 
-  if (Enabled) {
-    MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
-  } else {
-    MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, 0);
-  }
+  // Enable wakeup timer
+  RtcDelayedWrite (RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
 
   return EFI_SUCCESS;
 }
-- 
2.29.0

Re: [edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT debug prints

[Samer El-Haj-Mahmoud]

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT
> debug prints
>
> Resolution of the DebugLib for the DXE_RUNTIME_DRIVER library
> class was limited to non-RELEASE builds. This caused crashes
> during FWTS in case the RT attempted to use UART. Fix that
> by allowing to use DxeRuntimeDebugLibSerialPort in all kind
> of builds.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 4cdafe8b1f..939fbf14d9 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -195,9 +195,7 @@
>  !else
>
>
> CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.i
> nf
>
>  !endif
>
> -!if $(TARGET) != RELEASE
>
>
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDeb
> ugLibSerialPort.inf
>
> -!endif
>
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
> bRuntimeDxe.inf
>
>
>
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>
> --
> 2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use unique UID's

[Samer El-Haj-Mahmoud]

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use
> unique UID's
>
> The CN9131 variant's SSDT comprised UID's, whose values
> overlapped the ones used in the main DSDT file. Fix that.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> index dc32fe836a..691a709c18 100644
> --- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> +++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> @@ -18,7 +18,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          Device (AHC1)
>
>          {
>
>              Name (_HID, "LNRO001E")     // _HID: Hardware ID
>
> -            Name (_UID, 0x00)           // _UID: Unique ID
>
> +            Name (_UID, 0x01)           // _UID: Unique ID
>
>              Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
>
>              Name (_CLS, Package (0x03)  // _CLS: Class Code
>
>              {
>
> @@ -43,7 +43,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          Device (XHC2)
>
>          {
>
>              Name (_HID, "PNP0D10")      // _HID: Hardware ID
>
> -            Name (_UID, 0x01)           // _UID: Unique ID
>
> +            Name (_UID, 0x02)           // _UID: Unique ID
>
>              Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
>
>
>
>              Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
>
> @@ -62,7 +62,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          {
>
>              Name (_HID, "MRVL0110")                             // _HID: Hardware ID
>
>              Name (_CCA, 0x01)                                   // Cache-coherent controller
>
> -            Name (_UID, 0x00)                                   // _UID: Unique ID
>
> +            Name (_UID, 0x01)                                   // _UID: Unique ID
>
>              Name (_CRS, ResourceTemplate ()
>
>              {
>
>                  Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
>
> --
> 2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1

[Samer El-Haj-Mahmoud]

Thanks for the patch.

Not an issue with this patch specifically, but it seems this #define EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16450 should probably be renamed to reflect the latest spec (at https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table), which says:

0x0001  :  16550 subset compatible with DBGP Revision 1

Maybe you could send another patch to do this in SerialPortConsoleRedirectionTable.h ? Hopefully before this value is used by other platforms (this patch introduces the first usage of this value in edk2 and edk2-platforms)



> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch
> SPCR UART subtype to 0x1
>
> DBG2 ACPI table description [1] specifies three subtypes
> related to 16550 UART:
> 0x0 - 16550 compatible
> 0x1 - 16550 subset
> 0x12 - 16550 compatible with parameters defined in
>        Generic Address Structure (GAS)
>
> It turned out however, that the Windows OS treats 0x0 subtype as
> legacy x86 UART with 8-bit access. ARM SoCs can use types 0x1 (16550 with
> fixed mmio32 access) or 0x12 (16550 with fully respected GAS contents).
>
> Switch Marvell SoCs ACPI UART subtype to 0x1 - thanks to that the
> same firmware can run properly with UART output in Windows 10, Linux
> and ESXI hypervisor.
>
> [1] https://docs.microsoft.com/en-us/windows-
> hardware/drivers/bringup/acpi-debug-port-table
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc   | 2 +-
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> index 438cf7880e..6efc175bdf 100644
> --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> @@ -22,7 +22,7 @@
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
>
>                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
>
>    ),
>
> -
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> 16550,  // InterfaceType
>
> +
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> 16450,  // InterfaceType
>
>    { EFI_ACPI_RESERVED_BYTE,
>
>      EFI_ACPI_RESERVED_BYTE,
>
>      EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
>
> diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> index f663d8ade8..2a3415f0a6 100644
> --- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> +++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> @@ -22,7 +22,7 @@
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
>
>                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
>
>    ),
>
> -
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> 16550,  // InterfaceType
>
> +
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> 16450,  // InterfaceType
>
>    { EFI_ACPI_RESERVED_BYTE,
>
>      EFI_ACPI_RESERVED_BYTE,
>
>      EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
>
> --
> 2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 5/6] Marvell: RealTimeClockLib: Fix daylight and timezone handling

[Samer El-Haj-Mahmoud]

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 5/6] Marvell: RealTimeClockLib: Fix daylight
> and timezone handling
>
> The Marvell implementation of the RealTimeClockLib was unnecessarily
> overriding the daylight and timezone values, which are handled
> by non-volatile variables in the generic code. Fix that.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c |
> 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git
> a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> index 40ab01ed41..a48d44ed83 100644
> ---
> a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> +++
> b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> @@ -79,9 +79,6 @@ LibGetTime (
>    // Convert from internal 32-bit time to UEFI time
>
>    EpochToEfiTime (RegVal, Time);
>
>
>
> -  Time->TimeZone = EFI_UNSPECIFIED_TIMEZONE;
>
> -  Time->Daylight = 0;
>
> -
>
>    return Status;
>
>  }
>
>
>
> --
> 2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 1/6] Marvell/Drivers: SmbiosPlatformDxe: Align Type17 to SMBIOS v3.2

[Samer El-Haj-Mahmoud]

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 1/6] Marvell/Drivers: SmbiosPlatformDxe:
> Align Type17 to SMBIOS v3.2
>
> This patch adds missing entries required for SMBIOS v3.2 compliance
> of the Type17 table. On the occasion improve Type4 table contents.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 39
> ++++++++++++++++++--
>  1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> index 2ecaec2af5..a99291e902 100644
> --- a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> +++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> @@ -181,7 +181,7 @@ STATIC SMBIOS_TABLE_TYPE4 mArmadaDefaultType4
> = {
>    3,             //version
>
>    {0,0,0,0,0,1}, //voltage
>
>    0,             //external clock
>
> -  2000,          //max speed
>
> +  2200,          //max speed
>
>    0,             //current speed - requires update
>
>    0x41,          //status
>
>    ProcessorUpgradeOther,
>
> @@ -196,6 +196,9 @@ STATIC SMBIOS_TABLE_TYPE4 mArmadaDefaultType4
> = {
>    4,             //threads per socket
>
>    0xEC,          //processor characteristics
>
>    ProcessorFamilyARM, //ARM core
>
> +  0,             // CoreCount2;
>
> +  0,             // EnabledCoreCount2;
>
> +  0,             // ThreadCount2;
>
>  };
>
>
>
>  STATIC CHAR8 CONST *mArmadaDefaultType4Strings[] = {
>
> @@ -457,7 +460,7 @@ STATIC SMBIOS_TABLE_TYPE17
> mArmadaDefaultType17 = {
>    0,  //Memory size obtained dynamically
>
>    MemoryFormFactorRowOfChips,      //Memory factor
>
>    0,                               //Not part of a set
>
> -  1,                               //Right side of board
>
> +  1,                               //Location
>
>    2,                               //Bank 0
>
>    MemoryTypeDdr4,                  //DDR4
>
>    {0,0,0,0,0,0,0,0,0,0,0,0,0,0,1}, //unbuffered
>
> @@ -467,10 +470,36 @@ STATIC SMBIOS_TABLE_TYPE17
> mArmadaDefaultType17 = {
>    0, //asset tag
>
>    0, //part number
>
>    0, //rank
>
> +  0, // ExtendedSize; (since Size < 32GB-1)
>
> +  0, // ConfiguredMemoryClockSpeed - initialized at runtime
>
> +  0, // MinimumVoltage; (unknown)
>
> +  0, // MaximumVoltage; (unknown)
>
> +  0, // ConfiguredVoltage; (unknown)
>
> +  MemoryTechnologyDram, // MemoryTechnology
>
> +  {{                    // MemoryOperatingModeCapability
>
> +    0,  // Reserved                        :1;
>
> +    0,  // Other                           :1;
>
> +    0,  // Unknown                         :1;
>
> +    1,  // VolatileMemory                  :1;
>
> +    0,  // ByteAccessiblePersistentMemory  :1;
>
> +    0,  // BlockAccessiblePersistentMemory :1;
>
> +    0   // Reserved                        :10;
>
> +  }},
>
> +  0, // FirwareVersion
>
> +  0, // ModuleManufacturerID (unknown)
>
> +  0, // ModuleProductID (unknown)
>
> +  0, // MemorySubsystemControllerManufacturerID (unknown)
>
> +  0, // MemorySubsystemControllerProductID (unknown)
>
> +  0, // NonVolatileSize
>
> +  0, // VolatileSize - initialized at runtime
>
> +  0, // CacheSize
>
> +  0, // LogicalSize
>
> +  0, // ExtendedSpeed,
>
> +  0  // ExtendedConfiguredMemorySpeed
>
>  };
>
>
>
>  STATIC CHAR8 CONST *mArmadaDefaultType17Strings[] = {
>
> -  "RIGHT SIDE\0",                     /* location */
>
> +  "DIMM SLOT\0",                      /* location */
>
>    "BANK 0\0",                         /* bank description */
>
>    NULL
>
>  };
>
> @@ -735,9 +764,10 @@ SmbiosMemoryInstall (
>    }
>
>
>
>    //
>
> -  // Update TYPE17 memory size field
>
> +  // Update TYPE17 memory size fields
>
>    //
>
>    mArmadaDefaultType17.Size = (UINT16)(MemorySize >> 20);
>
> +  mArmadaDefaultType17.VolatileSize = MemorySize;
>
>
>
>    return EFI_SUCCESS;
>
>  }
>
> @@ -767,6 +797,7 @@ SmbiosInstallAllStructures (
>    mArmadaDefaultType0.SystemBiosMinorRelease =
> FirmwareMinorRevisionNumber;
>
>    mArmadaDefaultType4.CurrentSpeed = SampleAtResetGetCpuFrequency
> ();
>
>    mArmadaDefaultType17.Speed = SampleAtResetGetDramFrequency ();
>
> +  mArmadaDefaultType17.ConfiguredMemoryClockSpeed =
> SampleAtResetGetDramFrequency ();
>
>
>
>    //
>
>    // Generate memory descriptors.
>
> --
> 2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-devel] [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1

[Samer El-Haj-Mahmoud]

That being said, for this particular patch

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer
> El-Haj-Mahmoud via groups.io
> Sent: Monday, May 24, 2021 7:07 AM
> To: Marcin Wojtas <mw@semihalf.com>; devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Subject: Re: [edk2-devel] [edk2-platforms PATCH 3/6] Marvell:
> Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1
>
> Thanks for the patch.
>
> Not an issue with this patch specifically, but it seems this #define
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> 16450 should probably be renamed to reflect the latest spec (at
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-
> debug-port-table), which says:
>
> 0x0001  :  16550 subset compatible with DBGP Revision 1
>
> Maybe you could send another patch to do this in
> SerialPortConsoleRedirectionTable.h ? Hopefully before this value is used by
> other platforms (this patch introduces the first usage of this value in edk2
> and edk2-platforms)
>
>
>
> > -----Original Message-----
> > From: Marcin Wojtas <mw@semihalf.com>
> > Sent: Monday, May 24, 2021 1:29 AM
> > To: devel@edk2.groups.io
> > Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-
> Mahmoud
> > <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> > <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> > Marcin Wojtas <mw@semihalf.com>
> > Subject: [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx:
> Switch
> > SPCR UART subtype to 0x1
> >
> > DBG2 ACPI table description [1] specifies three subtypes
> > related to 16550 UART:
> > 0x0 - 16550 compatible
> > 0x1 - 16550 subset
> > 0x12 - 16550 compatible with parameters defined in
> >        Generic Address Structure (GAS)
> >
> > It turned out however, that the Windows OS treats 0x0 subtype as
> > legacy x86 UART with 8-bit access. ARM SoCs can use types 0x1 (16550 with
> > fixed mmio32 access) or 0x12 (16550 with fully respected GAS contents).
> >
> > Switch Marvell SoCs ACPI UART subtype to 0x1 - thanks to that the
> > same firmware can run properly with UART output in Windows 10, Linux
> > and ESXI hypervisor.
> >
> > [1] https://docs.microsoft.com/en-us/windows-
> > hardware/drivers/bringup/acpi-debug-port-table
> >
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> >  Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc   | 2 +-
> >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> > b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> > index 438cf7880e..6efc175bdf 100644
> > --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> > +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
> > @@ -22,7 +22,7 @@
> > EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> >                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> >
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> >
> >    ),
> >
> > -
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> > 16550,  // InterfaceType
> >
> > +
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> > 16450,  // InterfaceType
> >
> >    { EFI_ACPI_RESERVED_BYTE,
> >
> >      EFI_ACPI_RESERVED_BYTE,
> >
> >      EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
> >
> > diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> > b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> > index f663d8ade8..2a3415f0a6 100644
> > --- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> > +++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
> > @@ -22,7 +22,7 @@
> > EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> >                  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> >
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> >
> >    ),
> >
> > -
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> > 16550,  // InterfaceType
> >
> > +
> >
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_
> > 16450,  // InterfaceType
> >
> >    { EFI_ACPI_RESERVED_BYTE,
> >
> >      EFI_ACPI_RESERVED_BYTE,
> >
> >      EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]
> >
> > --
> > 2.29.0
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
>
>
> 
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT debug prints

[Sunny Wang]

Looks good to me as well.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Sent: Monday, May 24, 2021 7:00 PM
To: Marcin Wojtas <mw@semihalf.com>; devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Sunny Wang <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: RE: [edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT debug prints

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 2/6] Marvell: Armada7k8k/OcteonTx: Fix RT
> debug prints
>
> Resolution of the DebugLib for the DXE_RUNTIME_DRIVER library
> class was limited to non-RELEASE builds. This caused crashes
> during FWTS in case the RT attempted to use UART. Fix that
> by allowing to use DxeRuntimeDebugLibSerialPort in all kind
> of builds.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 4cdafe8b1f..939fbf14d9 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -195,9 +195,7 @@
>  !else
>
>
> CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.i
> nf
>
>  !endif
>
> -!if $(TARGET) != RELEASE
>
>
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDeb
> ugLibSerialPort.inf
>
> -!endif
>
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
> bRuntimeDxe.inf
>
>
>
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>
> --
> 2.29.0


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1

[Sunny Wang]

Looks good to me.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: Marcin Wojtas <mw@semihalf.com>
Sent: Monday, May 24, 2021 1:29 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com; Marcin Wojtas <mw@semihalf.com>
Subject: [edk2-platforms PATCH 3/6] Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1

DBG2 ACPI table description [1] specifies three subtypes
related to 16550 UART:
0x0 - 16550 compatible
0x1 - 16550 subset
0x12 - 16550 compatible with parameters defined in
       Generic Address Structure (GAS)

It turned out however, that the Windows OS treats 0x0 subtype as
legacy x86 UART with 8-bit access. ARM SoCs can use types 0x1 (16550 with
fixed mmio32 access) or 0x12 (16550 with fully respected GAS contents).

Switch Marvell SoCs ACPI UART subtype to 0x1 - thanks to that the
same firmware can run properly with UART output in Windows 10, Linux
and ESXI hypervisor.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc   | 2 +-
 Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
index 438cf7880e..6efc175bdf 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc
@@ -22,7 +22,7 @@ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,

                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION

   ),

-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550,  // InterfaceType

+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16450,  // InterfaceType

   { EFI_ACPI_RESERVED_BYTE,

     EFI_ACPI_RESERVED_BYTE,

     EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]

diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
index f663d8ade8..2a3415f0a6 100644
--- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
+++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc
@@ -22,7 +22,7 @@ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,

                 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION

   ),

-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550,  // InterfaceType

+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16450,  // InterfaceType

   { EFI_ACPI_RESERVED_BYTE,

     EFI_ACPI_RESERVED_BYTE,

     EFI_ACPI_RESERVED_BYTE },                                           // Reserved1[3]

--
2.29.0

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use unique UID's

[Sunny Wang]

Looks good to me as well. By the way, this fixed Windows PE blue screen issue.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Sent: Monday, May 24, 2021 7:00 PM
To: Marcin Wojtas <mw@semihalf.com>; devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Sunny Wang <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: RE: [edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use unique UID's

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Marcin Wojtas <mw@semihalf.com>
> Sent: Monday, May 24, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> <Sunny.Wang@arm.com>; gjb@semihalf.com; upstream@semihalf.com;
> Marcin Wojtas <mw@semihalf.com>
> Subject: [edk2-platforms PATCH 4/6] Marvell/Cn913xDbA: AcpiTables: Use
> unique UID's
>
> The CN9131 variant's SSDT comprised UID's, whose values
> overlapped the ones used in the main DSDT file. Fix that.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> index dc32fe836a..691a709c18 100644
> --- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> +++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl
> @@ -18,7 +18,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          Device (AHC1)
>
>          {
>
>              Name (_HID, "LNRO001E")     // _HID: Hardware ID
>
> -            Name (_UID, 0x00)           // _UID: Unique ID
>
> +            Name (_UID, 0x01)           // _UID: Unique ID
>
>              Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
>
>              Name (_CLS, Package (0x03)  // _CLS: Class Code
>
>              {
>
> @@ -43,7 +43,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          Device (XHC2)
>
>          {
>
>              Name (_HID, "PNP0D10")      // _HID: Hardware ID
>
> -            Name (_UID, 0x01)           // _UID: Unique ID
>
> +            Name (_UID, 0x02)           // _UID: Unique ID
>
>              Name (_CCA, 0x01)           // _CCA: Cache Coherency Attribute
>
>
>
>              Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
>
> @@ -62,7 +62,7 @@ DefinitionBlock ("Cn9131DbASsdt.aml", "SSDT", 2,
> "MVEBU ", "CN9131", 3)
>          {
>
>              Name (_HID, "MRVL0110")                             // _HID: Hardware ID
>
>              Name (_CCA, 0x01)                                   // Cache-coherent controller
>
> -            Name (_UID, 0x00)                                   // _UID: Unique ID
>
> +            Name (_UID, 0x01)                                   // _UID: Unique ID
>
>              Name (_CRS, ResourceTemplate ()
>
>              {
>
>                  Memory32Fixed (ReadWrite, 0xf4000000 , 0x100000)
>
> --
> 2.29.0


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-platforms PATCH 6/6] Marvell: RealTimeClockLib: Rework LibGetWakeupTime/LibSetWakeupTime

[Ard Biesheuvel]

On Mon, 24 May 2021 at 07:29, Marcin Wojtas <mw@semihalf.com> wrote:
>
> Apply multiple fixes to the Marvell RealTimeClockLib wakeup
> library callbacks.
>
> LibGetWakeupTime:
> * Add input parameters validation
> * Fix 'Pending' value check
>
> LibSetWakeupTime:
> * Allow disabling the wakeup timer regardless the input 'Time' value
> * Use more generic 'Time' value verification, which is more strict
>   than the replaced custom one.
> * Use proper alarm mask for 'Pending' signalling
>
> With above the ACS3.0 FWTS and SCT timer tests pass cleanly.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h |  2 +-
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 29 ++++++++++----------
>  2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
> index 7fa1d092e4..c33e63d107 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
> @@ -17,7 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define RTC_BRIDGE_TIMING_CTRL0_REG_OFFS        0x80
>  #define RTC_BRIDGE_TIMING_CTRL1_REG_OFFS        0x84
>  #define RTC_IRQ_STATUS_REG                      0x90
> -#define RTC_IRQ_ALARM_MASK                      0x1
> +#define RTC_IRQ_ALARM_MASK                      0x2
>  #define RTC_WRITE_PERIOD_DELAY_MASK             0xFFFF
>  #define RTC_WRITE_PERIOD_DELAY_DEFAULT          0x3FF
>  #define RTC_WRITE_SETUP_DELAY_MASK              (0xFFFF << 16)
> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> index a48d44ed83..49c9385d53 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> @@ -140,11 +140,15 @@ LibGetWakeupTime (
>  {
>    UINT32 WakeupSeconds;
>
> +  if (Time == NULL || Enabled == NULL || Pending == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    *Enabled = MmioRead32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG) & RTC_IRQ_ALARM_EN;
>
>    *Pending = MmioRead32 (mArmadaRtcBase + RTC_IRQ_STATUS_REG) & RTC_IRQ_ALARM_MASK;
>    // Ack pending alarm
> -  if (Pending) {
> +  if (*Pending) {
>      MmioWrite32 (mArmadaRtcBase + RTC_IRQ_STATUS_REG, RTC_IRQ_ALARM_MASK);
>    }
>
> @@ -176,14 +180,14 @@ LibSetWakeupTime (
>  {
>    UINTN       WakeupSeconds;
>
> -  //
> -  // Because the Armada RTC uses a 32-bit counter for seconds,
> -  // the maximum time span is just over 136 years.
> -  // Time is stored in Unix Epoch format, so it starts in 1970,
> -  // Therefore it can not exceed the year 2106.
> -  //
> -  if ((Time->Year < 1970) || (Time->Year >= 2106)) {
> -    return EFI_UNSUPPORTED;
> +  // Handle timer disabling case
> +  if (!Enabled) {
> +    RtcDelayedWrite (RTC_IRQ_2_CONFIG_REG, 0);
> +    return EFI_SUCCESS;
> +  }
> +
> +  if (Time == NULL || !IsTimeValid (Time)) {
> +    return EFI_INVALID_PARAMETER;
>    }
>
>    // Convert time to raw seconds
> @@ -195,11 +199,8 @@ LibSetWakeupTime (
>    // Issue delayed write to alarm register
>    RtcDelayedWrite (RTC_ALARM_2_REG, (UINT32)WakeupSeconds);
>
> -  if (Enabled) {
> -    MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
> -  } else {
> -    MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, 0);
> -  }
> +  // Enable wakeup timer
> +  RtcDelayedWrite (RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
>
>    return EFI_SUCCESS;
>  }
> --
> 2.29.0
>

Re: [edk2-platforms PATCH 0/6] Marvell ACS fixes

[Ard Biesheuvel]

On Mon, 24 May 2021 at 07:29, Marcin Wojtas <mw@semihalf.com> wrote:
>
> Hi,
>
> This series addresses a couple of issues found during
> ACS3.0 test suit runs. The SMBIOS Type17 table
> extension and RTC fixes/improvements.
> Moreover the SPCR console subtype is switched to
> a different value and Cn913xDb ACPI tables are
> fixed, so that to achieve proper Windows 10 PE boot.
>
> More details can be found in the commit logs.
> The patchest is publicly available in the github:
> https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/marvell-acs-r20210524
>
> Best regards,
> Marcin
>
> Marcin Wojtas (6):
>   Marvell/Drivers: SmbiosPlatformDxe: Align Type17 to SMBIOS v3.2
>   Marvell: Armada7k8k/OcteonTx: Fix RT debug prints
>   Marvell: Armada7k8k/OcteonTx: Switch SPCR UART subtype to 0x1
>   Marvell/Cn913xDbA: AcpiTables: Use unique UID's
>   Marvell: RealTimeClockLib: Fix daylight and timezone handling
>   Marvell: RealTimeClockLib: Rework LibGetWakeupTime/LibSetWakeupTime
>

Series pushed as f7c08b9bae56..64f3cf4cc84d

Thanks!

>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                          |  2 -
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h |  2 +-
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 32 ++++++++--------
>  Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c          | 39 ++++++++++++++++++--
>  Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc                        |  2 +-
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl             |  6 +--
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Spcr.aslc                      |  2 +-
>  7 files changed, 56 insertions(+), 29 deletions(-)
>
> --
> 2.29.0
>