public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI
@ 2021-03-24  9:45 Sunny Wang
  2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sunny Wang @ 2021-03-24  9:45 UTC (permalink / raw)
  To: devel
  Cc: Sunny Wang, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
	Pete Batard, Ard Biesheuvel, Sunny Wang

Changes:
  1. Add code to ConfigDxe driver and AcpiTables module to dynamically
     build either Mini UART or PL011 UART info in ACPI. This fixes the
     issue discussed in https://github.com/pftf/RPi4/issues/118.
  2. Cleanup by moving duplicate Debug Port 2 table related defines and
     structures to a newly created header file (RpiDebugPort2Table.h).

Testing Done:
  - Booted to UEFI shell and use acpiview command to check the result of
    the different UART settings in config.txt (enabling either Mini UART
    or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
    changed as expected.

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
 .../RaspberryPi/AcpiTables/AcpiTables.inf     |   9 +-
 .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 ++++++++
 .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 ++++++++---------
 .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +++++++++
 .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +++++++++---------
 Platform/RaspberryPi/AcpiTables/Uart.asl      |  10 +-
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +++++++++-
 .../IndustryStandard/RpiDebugPort2Table.h     |  34 ++++
 8 files changed, 497 insertions(+), 212 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
 create mode 100644 Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d3363a76a1..fc8e927138 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
 #
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
 #  Copyright (c) 2017, Andrey Warkentin <andrey.warkentin@gmail.com>
 #  Copyright (c) Microsoft Corporation. All rights reserved.
 #
@@ -28,12 +28,14 @@
   Emmc.asl
   Madt.aslc
   Fadt.aslc
-  Dbg2.aslc
+  Dbg2MiniUart.aslc
+  Dbg2Pl011.aslc
   Gtdt.aslc
   Iort.aslc
   Dsdt.asl
   Csrt.aslc
-  Spcr.aslc
+  SpcrMiniUart.aslc
+  SpcrPl011.aslc
   Pptt.aslc
   SsdtThermal.asl

@@ -71,3 +73,4 @@

 [BuildOptions]
   GCC:*_*_*_ASL_FLAGS       = -vw3133 -vw3150
+
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
new file mode 100644
index 0000000000..c83b978731
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,82 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
+    {                                                                                                                 \
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
+      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
+      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
+      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
+      0,                                                              /* UINT16    OemDataLength */                   \
+      0,                                                              /* UINT16    OemDataOffset */                   \
+      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
+      SubType,                                                        /* UINT16    Port Subtype */                    \
+      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
+    },                                                                                                                \
+    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
+    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
+    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
+  }
+
+
+STATIC DBG2_TABLE Dbg2 = {
+  {
+    ACPI_HEADER (
+      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+      DBG2_TABLE,
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
+    ),
+    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
+    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
+  },
+  {
+    /*
+     * Kernel Debug Port
+     */
+    DBG2_DEBUG_PORT_DDI (
+      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
+      RPI_UART_INTERFACE_TYPE,
+      RPI_UART_BASE_ADDRESS,
+      RPI_UART_LENGTH,
+      RPI_UART_STR
+    ),
+  }
+};
+
+#pragma pack()
+
+//
+// Reference the table being generated to prevent the optimizer from removing
+// the data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Dbg2;
+
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
similarity index 72%
rename from Platform/RaspberryPi/AcpiTables/Dbg2.aslc
rename to Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
index e3f2adae7e..dccfa24601 100644
--- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
@@ -1,105 +1,82 @@
-/** @file
- *
- *  Debug Port Table (DBG2)
- *
- *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
- *  Copyright (c) 2012-2020, ARM Limited. All rights reserved.
- *
- *  SPDX-License-Identifier: BSD-2-Clause-Patent
- *
- **/
-
-#include <IndustryStandard/Acpi.h>
-#include <IndustryStandard/Bcm2836.h>
-#include <IndustryStandard/DebugPort2Table.h>
-#include <Library/AcpiLib.h>
-#include <Library/PcdLib.h>
-
-#include "AcpiTables.h"
-
-#pragma pack(1)
-
-#define RPI_DBG2_NUM_DEBUG_PORTS                        1
-#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
-#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
-
-#if (RPI_MODEL == 4)
-#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
-#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
-#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
-#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
-#else
-#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
-#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
-#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
-//
-// RPI_UART_STR should match the value used Uart.asl
-//
-#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
-#endif
-
-typedef struct {
-  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
-  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
-  UINT32                                                AddressSize;
-  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
-} DBG2_DEBUG_DEVICE_INFORMATION;
-
-typedef struct {
-  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
-  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
-} DBG2_TABLE;
-
-
-#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
-    {                                                                                                                 \
-      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
-      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
-      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
-      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
-      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
-      0,                                                              /* UINT16    OemDataLength */                   \
-      0,                                                              /* UINT16    OemDataOffset */                   \
-      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
-      SubType,                                                        /* UINT16    Port Subtype */                    \
-      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
-      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
-      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
-    },                                                                                                                \
-    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
-    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
-    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
-  }
-
-
-STATIC DBG2_TABLE Dbg2 = {
-  {
-    ACPI_HEADER (
-      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
-      DBG2_TABLE,
-      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
-    ),
-    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
-    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
-  },
-  {
-    /*
-     * Kernel Debug Port
-     */
-    DBG2_DEBUG_PORT_DDI (
-      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
-      RPI_UART_INTERFACE_TYPE,
-      RPI_UART_BASE_ADDRESS,
-      RPI_UART_LENGTH,
-      RPI_UART_STR
-    ),
-  }
-};
-
-#pragma pack()
-
-//
-// Reference the table being generated to prevent the optimizer from removing
-// the data structure from the executable
-//
-VOID* CONST ReferenceAcpiTable = &Dbg2;
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
+#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
+    {                                                                                                                 \
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
+      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
+      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
+      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
+      0,                                                              /* UINT16    OemDataLength */                   \
+      0,                                                              /* UINT16    OemDataOffset */                   \
+      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
+      SubType,                                                        /* UINT16    Port Subtype */                    \
+      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
+    },                                                                                                                \
+    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
+    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
+    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
+  }
+
+
+STATIC DBG2_TABLE Dbg2 = {
+  {
+    ACPI_HEADER (
+      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+      DBG2_TABLE,
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
+    ),
+    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
+    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
+  },
+  {
+    /*
+     * Kernel Debug Port
+     */
+    DBG2_DEBUG_PORT_DDI (
+      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
+      RPI_UART_INTERFACE_TYPE,
+      RPI_UART_BASE_ADDRESS,
+      RPI_UART_LENGTH,
+      RPI_UART_STR
+    ),
+  }
+};
+
+#pragma pack()
+
+//
+// Reference the table being generated to prevent the optimizer from removing
+// the data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Dbg2;
+
diff --git a/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
new file mode 100644
index 0000000000..aaf5c5317e
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
@@ -0,0 +1,92 @@
+/** @file
+* SPCR Table
+*
+* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
+* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#define RPI_UART_FLOW_CONTROL_NONE           0
+
+#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
+
+STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
+  ACPI_HEADER (
+    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
+  ),
+  // UINT8                                   InterfaceType;
+  RPI_UART_INTERFACE_TYPE,
+  // UINT8                                   Reserved1[3];
+  {
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE
+  },
+  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
+  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
+  // UINT8                                   InterruptType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
+  // UINT8                                   Irq;
+  0,                                         // Not used on ARM
+  // UINT32                                  GlobalSystemInterrupt;
+  RPI_UART_INTERRUPT,
+  // UINT8                                   BaudRate;
+#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
+#else
+#error Unsupported SPCR Baud Rate
+#endif
+  // UINT8                                   Parity;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
+  // UINT8                                   StopBits;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
+  // UINT8                                   FlowControl;
+  RPI_UART_FLOW_CONTROL_NONE,
+  // UINT8                                   TerminalType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
+  // UINT8                                   Reserved2;
+  EFI_ACPI_RESERVED_BYTE,
+  // UINT16                                  PciDeviceId;
+  0xFFFF,
+  // UINT16                                  PciVendorId;
+  0xFFFF,
+  // UINT8                                   PciBusNumber;
+  0x00,
+  // UINT8                                   PciDeviceNumber;
+  0x00,
+  // UINT8                                   PciFunctionNumber;
+  0x00,
+  // UINT32                                  PciFlags;
+  0x00000000,
+  // UINT8                                   PciSegment;
+  0x00,
+  // UINT32                                  Reserved3;
+  EFI_ACPI_RESERVED_DWORD
+};
+
+//
+// Reference the table being generated to prevent the optimizer from removing the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Spcr;
+
diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
similarity index 87%
rename from Platform/RaspberryPi/AcpiTables/Spcr.aslc
rename to Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
index 07df3a718d..5a540adf08 100644
--- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
+++ b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
@@ -1,97 +1,91 @@
-/** @file
-* SPCR Table
-*
-* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
-* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
-*
-* SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#include <IndustryStandard/Acpi.h>
-#include <IndustryStandard/Bcm2836.h>
-#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
-#include <Library/AcpiLib.h>
-#include <Library/PcdLib.h>
-
-#include "AcpiTables.h"
-
-#define RPI_UART_FLOW_CONTROL_NONE           0
-
-// Prefer PL011 serial output on the Raspberry Pi 4
-#if (RPI_MODEL == 4)
-#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
-#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
-#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
-#else
-#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
-#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
-#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
-#endif
-STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
-  ACPI_HEADER (
-    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
-    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
-    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
-  ),
-  // UINT8                                   InterfaceType;
-  RPI_UART_INTERFACE_TYPE,
-  // UINT8                                   Reserved1[3];
-  {
-    EFI_ACPI_RESERVED_BYTE,
-    EFI_ACPI_RESERVED_BYTE,
-    EFI_ACPI_RESERVED_BYTE
-  },
-  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
-  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
-  // UINT8                                   InterruptType;
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
-  // UINT8                                   Irq;
-  0,                                         // Not used on ARM
-  // UINT32                                  GlobalSystemInterrupt;
-  RPI_UART_INTERRUPT,
-  // UINT8                                   BaudRate;
-#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
-#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
-#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
-#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
-#else
-#error Unsupported SPCR Baud Rate
-#endif
-  // UINT8                                   Parity;
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
-  // UINT8                                   StopBits;
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
-  // UINT8                                   FlowControl;
-  RPI_UART_FLOW_CONTROL_NONE,
-  // UINT8                                   TerminalType;
-  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
-  // UINT8                                   Reserved2;
-  EFI_ACPI_RESERVED_BYTE,
-  // UINT16                                  PciDeviceId;
-  0xFFFF,
-  // UINT16                                  PciVendorId;
-  0xFFFF,
-  // UINT8                                   PciBusNumber;
-  0x00,
-  // UINT8                                   PciDeviceNumber;
-  0x00,
-  // UINT8                                   PciFunctionNumber;
-  0x00,
-  // UINT32                                  PciFlags;
-  0x00000000,
-  // UINT8                                   PciSegment;
-  0x00,
-  // UINT32                                  Reserved3;
-  EFI_ACPI_RESERVED_DWORD
-};
-
-//
-// Reference the table being generated to prevent the optimizer from removing the
-// data structure from the executable
-//
-VOID* CONST ReferenceAcpiTable = &Spcr;
+/** @file
+* SPCR Table
+*
+* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
+* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#define RPI_UART_FLOW_CONTROL_NONE           0
+
+#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
+#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
+#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
+
+STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
+  ACPI_HEADER (
+    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
+  ),
+  // UINT8                                   InterfaceType;
+  RPI_UART_INTERFACE_TYPE,
+  // UINT8                                   Reserved1[3];
+  {
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE
+  },
+  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
+  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
+  // UINT8                                   InterruptType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
+  // UINT8                                   Irq;
+  0,                                         // Not used on ARM
+  // UINT32                                  GlobalSystemInterrupt;
+  RPI_UART_INTERRUPT,
+  // UINT8                                   BaudRate;
+#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
+#else
+#error Unsupported SPCR Baud Rate
+#endif
+  // UINT8                                   Parity;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
+  // UINT8                                   StopBits;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
+  // UINT8                                   FlowControl;
+  RPI_UART_FLOW_CONTROL_NONE,
+  // UINT8                                   TerminalType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
+  // UINT8                                   Reserved2;
+  EFI_ACPI_RESERVED_BYTE,
+  // UINT16                                  PciDeviceId;
+  0xFFFF,
+  // UINT16                                  PciVendorId;
+  0xFFFF,
+  // UINT8                                   PciBusNumber;
+  0x00,
+  // UINT8                                   PciDeviceNumber;
+  0x00,
+  // UINT8                                   PciFunctionNumber;
+  0x00,
+  // UINT32                                  PciFlags;
+  0x00000000,
+  // UINT8                                   PciSegment;
+  0x00,
+  // UINT32                                  Reserved3;
+  EFI_ACPI_RESERVED_DWORD
+};
+
+//
+// Reference the table being generated to prevent the optimizer from removing the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Spcr;
diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index 81ae6711af..8ce297078d 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -2,6 +2,7 @@
  *
  *  [DSDT] Serial devices (UART).
  *
+ *  Copyright (c) 2021, ARM Limited. All rights reserved.
  *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
  *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) Microsoft Corporation. All rights reserved.
@@ -101,7 +102,10 @@ Device(BTH0)
   {
     Name (RBUF, ResourceTemplate ()
     {
-      // BT UART: URT0 (PL011) or URTM (miniUART)
+      //
+      // BT UART: ResourceSource will be dynamically updated to
+      // either URT0 (PL011) or URTM (miniUART) during boot
+      //
       UARTSerialBus(
         115200,        // InitialBaudRate: in BPS
         ,              // BitsPerByte: default to 8 bits
@@ -126,11 +130,7 @@ Device(BTH0)
                        //   no flow control.
         16,            // ReceiveBufferSize
         16,            // TransmitBufferSize
-#if (RPI_MODEL == 4)
-        "\\_SB.GDV0.URTM",  // ResourceSource:
-#else
         "\\_SB.GDV0.URT0",  // ResourceSource:
-#endif
                        //   UART bus controller name
         ,              // ResourceSourceIndex: assumed to be 0
         ,              // ResourceUsage: assumed to be
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 22f86d4d44..68ba14c846 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -1,6 +1,6 @@
 /** @file
  *
- *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
+ *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
  *  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -12,6 +12,9 @@
 #include <IndustryStandard/Bcm2836.h>
 #include <IndustryStandard/Bcm2836Gpio.h>
 #include <IndustryStandard/RpiMbox.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
+
 #include <Library/AcpiLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
@@ -40,6 +43,8 @@ STATIC UINT32 mModelFamily = 0;
 STATIC UINT32 mModelInstalledMB = 0;

 STATIC EFI_MAC_ADDRESS  mMacAddress;
+BOOLEAN                 mUsePl011Uart;
+BOOLEAN                 mUseMiniUart;

 /*
  * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
@@ -699,6 +704,71 @@ UpdateSdtNameOps (
   }
 }

+//
+// BTH0._HID.BCM2EA6
+//
+#define BTH0_HID_PATTERN_LEN 17
+
+//
+// \_SB.GDV0.URT
+//
+#define RESOURCE_SOURCE_PATTERN_LEN 13
+
+//
+// Scan the given namespace table (DSDT/SSDT) for AML NameOps
+// listed in the NameOpReplace structure. If one is found then
+// update the value in the table from the specified Pcd
+//
+// This allows us to have conditionals in AML controlled
+// by options in the BDS or detected during firmware bootstrap.
+// We could extend this concept for strings/etc but due to len
+// variations its probably easier to encode the strings
+// in the ASL and pick the correct one based off a variable.
+//
+STATIC
+VOID
+UpdateDevBth0 (
+  EFI_ACPI_DESCRIPTION_HEADER  *AcpiTable
+  )
+{
+  UINTN   Index;
+  CHAR8   Bth0HidPattern[BTH0_HID_PATTERN_LEN] = {0x42, 0x54, 0x48, 0x30, 0x08, 0x5F, 0X48, 0x49, 0x44, 0x0D, 0x42, 0x43, 0x4D, 0x32, 0x45, 0x41, 0x36};
+  CHAR8   ResourceSourcePattern[BTH0_HID_PATTERN_LEN] = {0x5C, 0x5F, 0x53, 0x42, 0x2E, 0x47, 0X44, 0x56, 0x30, 0x2E, 0x55, 0x52, 0x54};
+  BOOLEAN FoundBth0HidPattern;
+  UINT8   *SdtPtr;
+  UINT32  SdtSize;
+
+  FoundBth0HidPattern = FALSE;
+  SdtSize = AcpiTable->Length;
+
+  if (SdtSize > 0) {
+    SdtPtr = (UINT8 *)AcpiTable;
+    for (Index = 0; Index < (SdtSize - BTH0_HID_PATTERN_LEN); Index++) {
+      if (!FoundBth0HidPattern) {
+        if (CompareMem (SdtPtr + Index, Bth0HidPattern, BTH0_HID_PATTERN_LEN) == 0) {
+          FoundBth0HidPattern = TRUE;
+        }
+      } else {
+        if (CompareMem (SdtPtr + Index, ResourceSourcePattern, RESOURCE_SOURCE_PATTERN_LEN) == 0) {
+          if (mUsePl011Uart) {
+            //
+            // Since PL011 has been set as Primary UART, set the last char in
+            // ResourceSource string to 'M' (0x4D) so that Mini UART can be used as Secondary UART for BlueTooth.
+            //
+            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x4D;
+          } else if (mUseMiniUart) {
+            //
+            // Since Mini UART has been set as Primary UART, set the last char in
+            // ResourceSource string to '0' (0x30) so that PL011 can be used as Secondary UART for BlueTooth.
+            //
+            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x30;
+          }
+          break;
+        }
+      }
+    }
+  }
+}

 STATIC
 BOOLEAN
@@ -770,8 +840,14 @@ HandleDynamicNamespace (
 {
   UINTN Tables;

+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *SpcrTable;
+  DBG2_TABLE                                     *Dbg2Table;
+
   switch (AcpiHeader->Signature) {
   case SIGNATURE_32 ('D', 'S', 'D', 'T'):
+    UpdateDevBth0 (AcpiHeader);
+    return TRUE;
+
   case SIGNATURE_32 ('S', 'S', 'D', 'T'):
     for (Tables = 0; SdtTables[Tables].OemTableId; Tables++) {
       if (AcpiHeader->OemTableId == SdtTables[Tables].OemTableId) {
@@ -779,14 +855,37 @@ HandleDynamicNamespace (
       }
     }
     DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
-
     return FALSE;
+
   case SIGNATURE_32 ('I', 'O', 'R', 'T'):
     // only enable the IORT on machines with >3G and no limit
     // to avoid problems with rhel/centos and other older OSs
     if (PcdGet32 (PcdRamLimitTo3GB) || !PcdGet32 (PcdRamMoreThan3GB)) {
       return FALSE;
     }
+    return TRUE;
+
+  case SIGNATURE_32 ('S', 'P', 'C', 'R'):
+    SpcrTable = (EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *)AcpiHeader;
+    if (mUsePl011Uart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART)) {
+      return TRUE;
+    } else if (mUseMiniUart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART)) {
+      return TRUE;
+    } else {
+      return FALSE;
+    }
+    return TRUE;
+
+  case SIGNATURE_32 ('D', 'B', 'G', '2'):
+    Dbg2Table = (DBG2_TABLE *)AcpiHeader;
+    if (mUsePl011Uart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)) {
+      return TRUE;
+    } else if (mUseMiniUart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART)) {
+      return TRUE;
+    } else {
+      return FALSE;
+    }
+    return TRUE;
   }

   return TRUE;
@@ -803,6 +902,9 @@ ConfigInitialize (
   EFI_STATUS                      Status;
   EFI_EVENT                       EndOfDxeEvent;

+  mUsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
+  mUseMiniUart  = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00012000);
+
   Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
                   NULL, (VOID**)&mFwProtocol);
   ASSERT_EFI_ERROR (Status);
@@ -859,3 +961,4 @@ ConfigInitialize (

   return EFI_SUCCESS;
 }
+
diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
new file mode 100644
index 0000000000..8f7452f97a
--- /dev/null
+++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
@@ -0,0 +1,34 @@
+/** @file
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+#ifndef __RPI_DEBUG_PORT_2_H__
+#define __RPI_DEBUG_PORT_2_H__
+
+#include <IndustryStandard/DebugPort2Table.h>
+
+#define RPI_DBG2_NUM_DEBUG_PORTS                        1
+#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
+#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
+
+#pragma pack(1)
+
+
+typedef struct {
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
+  UINT32                                                AddressSize;
+  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
+} DBG2_DEBUG_DEVICE_INFORMATION;
+
+typedef struct {
+  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
+  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
+} DBG2_TABLE;
+
+#pragma pack()
+#endif  //__RPI_DEBUG_PORT_2_H__
+
--
2.31.0.windows.1


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

* [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-03-24  9:45 [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Sunny Wang
@ 2021-03-24  9:45 ` Sunny Wang
  2021-03-24 19:50   ` Jeremy Linton
  2021-03-24 12:44 ` [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Pete Batard
  2021-03-24 19:40 ` Jeremy Linton
  2 siblings, 1 reply; 6+ messages in thread
From: Sunny Wang @ 2021-03-24  9:45 UTC (permalink / raw)
  To: devel
  Cc: Sunny Wang, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
	Pete Batard, Ard Biesheuvel, Sunny Wang

Merge changes in edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows OS.

Testing Done:
  - Successfully booted Windows 10 (20279.1) on SD (made by WOR) with
    the RPi-Windows-Drivers release ver 0.5 downloaded from
    https://github.com/worproject/RPi-Windows-Drivers/releases
    and checked that both Bluetooth and serial port (Mini UART) can
    work fine.

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
 Platform/RaspberryPi/AcpiTables/Uart.asl | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index 8ce297078d..e3165911a6 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -142,7 +148,7 @@ Device(BTH0)
       //
       // RPIQ connection for BT_ON/OFF
       //
-      GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
+      //GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
     })
     Return (RBUF)
   }
-- 
2.31.0.windows.1


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

* Re: [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI
  2021-03-24  9:45 [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Sunny Wang
  2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
@ 2021-03-24 12:44 ` Pete Batard
  2021-03-24 19:40 ` Jeremy Linton
  2 siblings, 0 replies; 6+ messages in thread
From: Pete Batard @ 2021-03-24 12:44 UTC (permalink / raw)
  To: Sunny Wang, devel
  Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton, Ard Biesheuvel

Hi Sunny,

I understand that you are going to re-send these patches due to e-mail 
mangling, but let me add a few of notes that might help you as you are 
doing that.

First of all, your patch series was missing a cover letter (there should 
have been a [PATCH v2 0/2]), that doesn't get applied, but that 
describes the series or the changes applied between 2 revisions.

This isn't a big deal for a series that has only 2 patches, but it seems 
to indicate that you missed some steps from Laszlo's helpful guide on 
how to configure your environment to send patches to EDK2, which you can 
find at:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers

Especially, you should have run the command:

git config format.coverletter true

Now, with regards to patch mangling, as you may be able to see, what I 
got is not as bad as what you seem to have been getting. In fact, 
barring a few issues (which I am pointing out below) your patch is 
almost usable, even with the double line feeds I am seeing from my 
e-mail client (because that's actually how I currently receive about 
half the patches that are posted on this list anyway, so much so that I 
have had to craft a quick script to remove them).

There are however some issues, that actually prevent your patch from 
applying properly, and, while I'm at it, I also added a few comments 
about some extra lines you added, that you should be able to fix for 
your next submission:

On 2021.03.24 09:45, Sunny Wang wrote:
> Changes:
> 
>    1. Add code to ConfigDxe driver and AcpiTables module to dynamically
> 
>       build either Mini UART or PL011 UART info in ACPI. This fixes the
> 
>       issue discussed in https://github.com/pftf/RPi4/issues/118.
> 
>    2. Cleanup by moving duplicate Debug Port 2 table related defines and
> 
>       structures to a newly created header file (RpiDebugPort2Table.h).
> 
> 
> 
> Testing Done:
> 
>    - Booted to UEFI shell and use acpiview command to check the result of
> 
>      the different UART settings in config.txt (enabling either Mini UART
> 
>      or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
> 
>      changed as expected.
> 
> 
> 
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> 
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> 
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> 
> Cc: Pete Batard <pete@akeo.ie>
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> 
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> 
> ---
> 
>   .../RaspberryPi/AcpiTables/AcpiTables.inf     |   9 +-
> 
>   .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 ++++++++
> 
>   .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 ++++++++---------
> 
>   .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +++++++++
> 
>   .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +++++++++---------
> 
>   Platform/RaspberryPi/AcpiTables/Uart.asl      |  10 +-
> 
>   .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +++++++++-
> 
>   .../IndustryStandard/RpiDebugPort2Table.h     |  34 ++++
> 
>   8 files changed, 497 insertions(+), 212 deletions(-)
> 
>   create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
>   rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
> 
>   create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
>   rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
> 
>   create mode 100644 Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> 
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> index d3363a76a1..fc8e927138 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> @@ -2,7 +2,7 @@
> 
>   #
> 
>   #  ACPI table data and ASL sources required to boot the platform.
> 
>   #
> 
> -#  Copyright (c) 2019, ARM Limited. All rights reserved.
> 
> +#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
> 
>   #  Copyright (c) 2017, Andrey Warkentin <andrey.warkentin@gmail.com>
> 
>   #  Copyright (c) Microsoft Corporation. All rights reserved.
> 
>   #
> 
> @@ -28,12 +28,14 @@
> 
>     Emmc.asl
> 
>     Madt.aslc
> 
>     Fadt.aslc
> 
> -  Dbg2.aslc
> 
> +  Dbg2MiniUart.aslc
> 
> +  Dbg2Pl011.aslc
> 
>     Gtdt.aslc
> 
>     Iort.aslc
> 
>     Dsdt.asl
> 
>     Csrt.aslc
> 
> -  Spcr.aslc
> 
> +  SpcrMiniUart.aslc
> 
> +  SpcrPl011.aslc
> 
>     Pptt.aslc
> 
>     SsdtThermal.asl
> 
> 

For the patch to be applicable, there should have been an additional 
space on the line above.
In other words, is should be ">  " and not just "> ".

> 
> @@ -71,3 +73,4 @@
> 
> 

Same here, and for about 4-5 more instances further down, that I'm not 
going to document.

If using 'git format-patch' and 'git send-email' as documented in 
Laszlo's guide, I would expect these to go away.

> 
>   [BuildOptions]
> 
>     GCC:*_*_*_ASL_FLAGS       = -vw3133 -vw3150
> 
> +

This doesn't have to do with patch mangling, but you are adding an 
unwarranted extra line here. Can you please remove it from your next 
submission?

> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
> new file mode 100644
> 
> index 0000000000..c83b978731
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
> @@ -0,0 +1,82 @@
> 
> +/** @file
> 
> + *
> 
> + *  Debug Port Table (DBG2)
> 
> + *
> 
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> 
> + *
> 
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> + *
> 
> + **/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> 
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> 
> +#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> 
> +//
> 
> +// RPI_UART_STR should match the value used Uart.asl
> 
> +//
> 
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> 
> +
> 
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> +    {                                                                                                                 \
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> +      0,                                                              /* UINT16    OemDataLength */                   \
> 
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> +    },                                                                                                                \
> 
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> +  }
> 
> +
> 
> +
> 
> +STATIC DBG2_TABLE Dbg2 = {
> 
> +  {
> 
> +    ACPI_HEADER (
> 
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> +      DBG2_TABLE,
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> +    ),
> 
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> +  },
> 
> +  {
> 
> +    /*
> 
> +     * Kernel Debug Port
> 
> +     */
> 
> +    DBG2_DEBUG_PORT_DDI (
> 
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> +      RPI_UART_INTERFACE_TYPE,
> 
> +      RPI_UART_BASE_ADDRESS,
> 
> +      RPI_UART_LENGTH,
> 
> +      RPI_UART_STR
> 
> +    ),
> 
> +  }
> 
> +};
> 
> +
> 
> +#pragma pack()
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing
> 
> +// the data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +

Extra line added. Please remove it.

> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> similarity index 72%
> 
> rename from Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> 
> rename to Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> index e3f2adae7e..dccfa24601 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> @@ -1,105 +1,82 @@
> 
> -/** @file
> 
> - *
> 
> - *  Debug Port Table (DBG2)
> 
> - *
> 
> - *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> - *  Copyright (c) 2012-2020, ARM Limited. All rights reserved.
> 
> - *
> 
> - *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> - *
> 
> - **/
> 
> -
> 
> -#include <IndustryStandard/Acpi.h>
> 
> -#include <IndustryStandard/Bcm2836.h>
> 
> -#include <IndustryStandard/DebugPort2Table.h>
> 
> -#include <Library/AcpiLib.h>
> 
> -#include <Library/PcdLib.h>
> 
> -
> 
> -#include "AcpiTables.h"
> 
> -
> 
> -#pragma pack(1)
> 
> -
> 
> -#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> 
> -#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> 
> -#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> 
> -
> 
> -#if (RPI_MODEL == 4)
> 
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> 
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> 
> -#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> 
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> 
> -#else
> 
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> 
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> 
> -#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> 
> -//
> 
> -// RPI_UART_STR should match the value used Uart.asl
> 
> -//
> 
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> 
> -#endif
> 
> -
> 
> -typedef struct {
> 
> -  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> 
> -  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> 
> -  UINT32                                                AddressSize;
> 
> -  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> 
> -} DBG2_DEBUG_DEVICE_INFORMATION;
> 
> -
> 
> -typedef struct {
> 
> -  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> 
> -  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> 
> -} DBG2_TABLE;
> 
> -
> 
> -
> 
> -#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> -    {                                                                                                                 \
> 
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> -      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> -      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> -      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> -      0,                                                              /* UINT16    OemDataLength */                   \
> 
> -      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> -      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> -      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> -      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> -    },                                                                                                                \
> 
> -    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> -    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> -    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> -  }
> 
> -
> 
> -
> 
> -STATIC DBG2_TABLE Dbg2 = {
> 
> -  {
> 
> -    ACPI_HEADER (
> 
> -      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> -      DBG2_TABLE,
> 
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> -    ),
> 
> -    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> -    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> -  },
> 
> -  {
> 
> -    /*
> 
> -     * Kernel Debug Port
> 
> -     */
> 
> -    DBG2_DEBUG_PORT_DDI (
> 
> -      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> -      RPI_UART_INTERFACE_TYPE,
> 
> -      RPI_UART_BASE_ADDRESS,
> 
> -      RPI_UART_LENGTH,
> 
> -      RPI_UART_STR
> 
> -    ),
> 
> -  }
> 
> -};
> 
> -
> 
> -#pragma pack()
> 
> -
> 
> -//
> 
> -// Reference the table being generated to prevent the optimizer from removing
> 
> -// the data structure from the executable
> 
> -//
> 
> -VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +/** @file
> 
> + *
> 
> + *  Debug Port Table (DBG2)
> 
> + *
> 
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> 
> + *
> 
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> + *
> 
> + **/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> 
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> 
> +#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> 
> +//
> 
> +// RPI_UART_STR should match the value used Uart.asl
> 
> +//
> 
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> 
> +
> 
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> +    {                                                                                                                 \
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> +      0,                                                              /* UINT16    OemDataLength */                   \
> 
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> +    },                                                                                                                \
> 
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> +  }
> 
> +
> 
> +
> 
> +STATIC DBG2_TABLE Dbg2 = {
> 
> +  {
> 
> +    ACPI_HEADER (
> 
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> +      DBG2_TABLE,
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> +    ),
> 
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> +  },
> 
> +  {
> 
> +    /*
> 
> +     * Kernel Debug Port
> 
> +     */
> 
> +    DBG2_DEBUG_PORT_DDI (
> 
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> +      RPI_UART_INTERFACE_TYPE,
> 
> +      RPI_UART_BASE_ADDRESS,
> 
> +      RPI_UART_LENGTH,
> 
> +      RPI_UART_STR
> 
> +    ),
> 
> +  }
> 
> +};
> 
> +
> 
> +#pragma pack()
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing
> 
> +// the data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +

Same issue with extra line added.

> diff --git a/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
> new file mode 100644
> 
> index 0000000000..aaf5c5317e
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
> @@ -0,0 +1,92 @@
> 
> +/** @file
> 
> +* SPCR Table
> 
> +*
> 
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> 
> +*
> 
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +*
> 
> +**/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> 
> +#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> 
> +#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> 
> +
> 
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> +  ACPI_HEADER (
> 
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> +  ),
> 
> +  // UINT8                                   InterfaceType;
> 
> +  RPI_UART_INTERFACE_TYPE,
> 
> +  // UINT8                                   Reserved1[3];
> 
> +  {
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE
> 
> +  },
> 
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> +  // UINT8                                   InterruptType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> +  // UINT8                                   Irq;
> 
> +  0,                                         // Not used on ARM
> 
> +  // UINT32                                  GlobalSystemInterrupt;
> 
> +  RPI_UART_INTERRUPT,
> 
> +  // UINT8                                   BaudRate;
> 
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> +#else
> 
> +#error Unsupported SPCR Baud Rate
> 
> +#endif
> 
> +  // UINT8                                   Parity;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> +  // UINT8                                   StopBits;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> +  // UINT8                                   FlowControl;
> 
> +  RPI_UART_FLOW_CONTROL_NONE,
> 
> +  // UINT8                                   TerminalType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> +  // UINT8                                   Reserved2;
> 
> +  EFI_ACPI_RESERVED_BYTE,
> 
> +  // UINT16                                  PciDeviceId;
> 
> +  0xFFFF,
> 
> +  // UINT16                                  PciVendorId;
> 
> +  0xFFFF,
> 
> +  // UINT8                                   PciBusNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciDeviceNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciFunctionNumber;
> 
> +  0x00,
> 
> +  // UINT32                                  PciFlags;
> 
> +  0x00000000,
> 
> +  // UINT8                                   PciSegment;
> 
> +  0x00,
> 
> +  // UINT32                                  Reserved3;
> 
> +  EFI_ACPI_RESERVED_DWORD
> 
> +};
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing the
> 
> +// data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> +

Same issue with unneeded extra line.

> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> similarity index 87%
> 
> rename from Platform/RaspberryPi/AcpiTables/Spcr.aslc
> 
> rename to Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> index 07df3a718d..5a540adf08 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
> 
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> @@ -1,97 +1,91 @@
> 
> -/** @file
> 
> -* SPCR Table
> 
> -*
> 
> -* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> -* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
> 
> -*
> 
> -* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -*
> 
> -**/
> 
> -
> 
> -#include <IndustryStandard/Acpi.h>
> 
> -#include <IndustryStandard/Bcm2836.h>
> 
> -#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> -#include <Library/AcpiLib.h>
> 
> -#include <Library/PcdLib.h>
> 
> -
> 
> -#include "AcpiTables.h"
> 
> -
> 
> -#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> -
> 
> -// Prefer PL011 serial output on the Raspberry Pi 4
> 
> -#if (RPI_MODEL == 4)
> 
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> 
> -#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> 
> -#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> 
> -#else
> 
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> 
> -#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> 
> -#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> 
> -#endif
> 
> -STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> -  ACPI_HEADER (
> 
> -    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> -  ),
> 
> -  // UINT8                                   InterfaceType;
> 
> -  RPI_UART_INTERFACE_TYPE,
> 
> -  // UINT8                                   Reserved1[3];
> 
> -  {
> 
> -    EFI_ACPI_RESERVED_BYTE,
> 
> -    EFI_ACPI_RESERVED_BYTE,
> 
> -    EFI_ACPI_RESERVED_BYTE
> 
> -  },
> 
> -  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> -  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> -  // UINT8                                   InterruptType;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> -  // UINT8                                   Irq;
> 
> -  0,                                         // Not used on ARM
> 
> -  // UINT32                                  GlobalSystemInterrupt;
> 
> -  RPI_UART_INTERRUPT,
> 
> -  // UINT8                                   BaudRate;
> 
> -#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> -#else
> 
> -#error Unsupported SPCR Baud Rate
> 
> -#endif
> 
> -  // UINT8                                   Parity;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> -  // UINT8                                   StopBits;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> -  // UINT8                                   FlowControl;
> 
> -  RPI_UART_FLOW_CONTROL_NONE,
> 
> -  // UINT8                                   TerminalType;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> -  // UINT8                                   Reserved2;
> 
> -  EFI_ACPI_RESERVED_BYTE,
> 
> -  // UINT16                                  PciDeviceId;
> 
> -  0xFFFF,
> 
> -  // UINT16                                  PciVendorId;
> 
> -  0xFFFF,
> 
> -  // UINT8                                   PciBusNumber;
> 
> -  0x00,
> 
> -  // UINT8                                   PciDeviceNumber;
> 
> -  0x00,
> 
> -  // UINT8                                   PciFunctionNumber;
> 
> -  0x00,
> 
> -  // UINT32                                  PciFlags;
> 
> -  0x00000000,
> 
> -  // UINT8                                   PciSegment;
> 
> -  0x00,
> 
> -  // UINT32                                  Reserved3;
> 
> -  EFI_ACPI_RESERVED_DWORD
> 
> -};
> 
> -
> 
> -//
> 
> -// Reference the table being generated to prevent the optimizer from removing the
> 
> -// data structure from the executable
> 
> -//
> 
> -VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> +/** @file
> 
> +* SPCR Table
> 
> +*
> 
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> 
> +*
> 
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +*
> 
> +**/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> 
> +#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> 
> +#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> 
> +
> 
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> +  ACPI_HEADER (
> 
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> +  ),
> 
> +  // UINT8                                   InterfaceType;
> 
> +  RPI_UART_INTERFACE_TYPE,
> 
> +  // UINT8                                   Reserved1[3];
> 
> +  {
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE
> 
> +  },
> 
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> +  // UINT8                                   InterruptType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> +  // UINT8                                   Irq;
> 
> +  0,                                         // Not used on ARM
> 
> +  // UINT32                                  GlobalSystemInterrupt;
> 
> +  RPI_UART_INTERRUPT,
> 
> +  // UINT8                                   BaudRate;
> 
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> +#else
> 
> +#error Unsupported SPCR Baud Rate
> 
> +#endif
> 
> +  // UINT8                                   Parity;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> +  // UINT8                                   StopBits;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> +  // UINT8                                   FlowControl;
> 
> +  RPI_UART_FLOW_CONTROL_NONE,
> 
> +  // UINT8                                   TerminalType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> +  // UINT8                                   Reserved2;
> 
> +  EFI_ACPI_RESERVED_BYTE,
> 
> +  // UINT16                                  PciDeviceId;
> 
> +  0xFFFF,
> 
> +  // UINT16                                  PciVendorId;
> 
> +  0xFFFF,
> 
> +  // UINT8                                   PciBusNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciDeviceNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciFunctionNumber;
> 
> +  0x00,
> 
> +  // UINT32                                  PciFlags;
> 
> +  0x00000000,
> 
> +  // UINT8                                   PciSegment;
> 
> +  0x00,
> 
> +  // UINT32                                  Reserved3;
> 
> +  EFI_ACPI_RESERVED_DWORD
> 
> +};
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing the
> 
> +// data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> index 81ae6711af..8ce297078d 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> @@ -2,6 +2,7 @@
> 
>    *
> 
>    *  [DSDT] Serial devices (UART).
> 
>    *
> 
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
> 
>    *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> 
>    *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
> 
>    *  Copyright (c) Microsoft Corporation. All rights reserved.
> 
> @@ -101,7 +102,10 @@ Device(BTH0)
> 
>     {
> 
>       Name (RBUF, ResourceTemplate ()
> 
>       {
> 
> -      // BT UART: URT0 (PL011) or URTM (miniUART)
> 
> +      //
> 
> +      // BT UART: ResourceSource will be dynamically updated to
> 
> +      // either URT0 (PL011) or URTM (miniUART) during boot
> 
> +      //
> 
>         UARTSerialBus(
> 
>           115200,        // InitialBaudRate: in BPS
> 
>           ,              // BitsPerByte: default to 8 bits
> 
> @@ -126,11 +130,7 @@ Device(BTH0)
> 
>                          //   no flow control.
> 
>           16,            // ReceiveBufferSize
> 
>           16,            // TransmitBufferSize
> 
> -#if (RPI_MODEL == 4)
> 
> -        "\\_SB.GDV0.URTM",  // ResourceSource:
> 
> -#else
> 
>           "\\_SB.GDV0.URT0",  // ResourceSource:
> 
> -#endif
> 
>                          //   UART bus controller name
> 
>           ,              // ResourceSourceIndex: assumed to be 0
> 
>           ,              // ResourceUsage: assumed to be
> 
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> index 22f86d4d44..68ba14c846 100644
> 
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> @@ -1,6 +1,6 @@
> 
>   /** @file
> 
>    *
> 
> - *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> 
> + *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> 
>    *  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> 
>    *
> 
>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -12,6 +12,9 @@
> 
>   #include <IndustryStandard/Bcm2836.h>
> 
>   #include <IndustryStandard/Bcm2836Gpio.h>
> 
>   #include <IndustryStandard/RpiMbox.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +
> 
>   #include <Library/AcpiLib.h>
> 
>   #include <Library/DebugLib.h>
> 
>   #include <Library/DevicePathLib.h>
> 
> @@ -40,6 +43,8 @@ STATIC UINT32 mModelFamily = 0;
> 
>   STATIC UINT32 mModelInstalledMB = 0;
> 
> 
> 
>   STATIC EFI_MAC_ADDRESS  mMacAddress;
> 
> +BOOLEAN                 mUsePl011Uart;
> 
> +BOOLEAN                 mUseMiniUart;
> 
> 
> 
>   /*
> 
>    * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
> 
> @@ -699,6 +704,71 @@ UpdateSdtNameOps (
> 
>     }
> 
>   }
> 
> 
> 
> +//
> 
> +// BTH0._HID.BCM2EA6
> 
> +//
> 
> +#define BTH0_HID_PATTERN_LEN 17
> 
> +
> 
> +//
> 
> +// \_SB.GDV0.URT
> 
> +//
> 
> +#define RESOURCE_SOURCE_PATTERN_LEN 13
> 
> +
> 
> +//
> 
> +// Scan the given namespace table (DSDT/SSDT) for AML NameOps
> 
> +// listed in the NameOpReplace structure. If one is found then
> 
> +// update the value in the table from the specified Pcd
> 
> +//
> 
> +// This allows us to have conditionals in AML controlled
> 
> +// by options in the BDS or detected during firmware bootstrap.
> 
> +// We could extend this concept for strings/etc but due to len
> 
> +// variations its probably easier to encode the strings
> 
> +// in the ASL and pick the correct one based off a variable.
> 
> +//
> 
> +STATIC
> 
> +VOID
> 
> +UpdateDevBth0 (
> 
> +  EFI_ACPI_DESCRIPTION_HEADER  *AcpiTable
> 
> +  )
> 
> +{
> 
> +  UINTN   Index;
> 
> +  CHAR8   Bth0HidPattern[BTH0_HID_PATTERN_LEN] = {0x42, 0x54, 0x48, 0x30, 0x08, 0x5F, 0X48, 0x49, 0x44, 0x0D, 0x42, 0x43, 0x4D, 0x32, 0x45, 0x41, 0x36};
> 
> +  CHAR8   ResourceSourcePattern[BTH0_HID_PATTERN_LEN] = {0x5C, 0x5F, 0x53, 0x42, 0x2E, 0x47, 0X44, 0x56, 0x30, 0x2E, 0x55, 0x52, 0x54};
> 
> +  BOOLEAN FoundBth0HidPattern;
> 
> +  UINT8   *SdtPtr;
> 
> +  UINT32  SdtSize;
> 
> +
> 
> +  FoundBth0HidPattern = FALSE;
> 
> +  SdtSize = AcpiTable->Length;
> 
> +
> 
> +  if (SdtSize > 0) {
> 
> +    SdtPtr = (UINT8 *)AcpiTable;
> 
> +    for (Index = 0; Index < (SdtSize - BTH0_HID_PATTERN_LEN); Index++) {
> 
> +      if (!FoundBth0HidPattern) {
> 
> +        if (CompareMem (SdtPtr + Index, Bth0HidPattern, BTH0_HID_PATTERN_LEN) == 0) {
> 
> +          FoundBth0HidPattern = TRUE;
> 
> +        }
> 
> +      } else {
> 
> +        if (CompareMem (SdtPtr + Index, ResourceSourcePattern, RESOURCE_SOURCE_PATTERN_LEN) == 0) {
> 
> +          if (mUsePl011Uart) {
> 
> +            //
> 
> +            // Since PL011 has been set as Primary UART, set the last char in
> 
> +            // ResourceSource string to 'M' (0x4D) so that Mini UART can be used as Secondary UART for BlueTooth.
> 
> +            //
> 
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x4D;
> 
> +          } else if (mUseMiniUart) {
> 
> +            //
> 
> +            // Since Mini UART has been set as Primary UART, set the last char in
> 
> +            // ResourceSource string to '0' (0x30) so that PL011 can be used as Secondary UART for BlueTooth.
> 
> +            //
> 
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x30;
> 
> +          }
> 
> +          break;
> 
> +        }
> 
> +      }
> 
> +    }
> 
> +  }
> 
> +}
> 
> 
> 
>   STATIC
> 
>   BOOLEAN
> 
> @@ -770,8 +840,14 @@ HandleDynamicNamespace (
> 
>   {
> 
>     UINTN Tables;
> 
> 
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *SpcrTable;
> 
> +  DBG2_TABLE                                     *Dbg2Table;
> 
> +
> 
>     switch (AcpiHeader->Signature) {
> 
>     case SIGNATURE_32 ('D', 'S', 'D', 'T'):
> 
> +    UpdateDevBth0 (AcpiHeader);
> 
> +    return TRUE;
> 
> +
> 
>     case SIGNATURE_32 ('S', 'S', 'D', 'T'):
> 
>       for (Tables = 0; SdtTables[Tables].OemTableId; Tables++) {
> 
>         if (AcpiHeader->OemTableId == SdtTables[Tables].OemTableId) {
> 
> @@ -779,14 +855,37 @@ HandleDynamicNamespace (
> 
>         }
> 
>       }
> 
>       DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
> 
> -
> 
>       return FALSE;
> 
> +
> 
>     case SIGNATURE_32 ('I', 'O', 'R', 'T'):
> 
>       // only enable the IORT on machines with >3G and no limit
> 
>       // to avoid problems with rhel/centos and other older OSs
> 
>       if (PcdGet32 (PcdRamLimitTo3GB) || !PcdGet32 (PcdRamMoreThan3GB)) {
> 
>         return FALSE;
> 
>       }
> 
> +    return TRUE;
> 
> +
> 
> +  case SIGNATURE_32 ('S', 'P', 'C', 'R'):
> 
> +    SpcrTable = (EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *)AcpiHeader;
> 
> +    if (mUsePl011Uart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART)) {
> 
> +      return TRUE;
> 
> +    } else if (mUseMiniUart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART)) {
> 
> +      return TRUE;
> 
> +    } else {
> 
> +      return FALSE;
> 
> +    }
> 
> +    return TRUE;
> 
> +
> 
> +  case SIGNATURE_32 ('D', 'B', 'G', '2'):
> 
> +    Dbg2Table = (DBG2_TABLE *)AcpiHeader;
> 
> +    if (mUsePl011Uart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)) {
> 
> +      return TRUE;
> 
> +    } else if (mUseMiniUart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART)) {
> 
> +      return TRUE;
> 
> +    } else {
> 
> +      return FALSE;
> 
> +    }
> 
> +    return TRUE;
> 
>     }
> 
> 
> 
>     return TRUE;
> 
> @@ -803,6 +902,9 @@ ConfigInitialize (
> 
>     EFI_STATUS                      Status;
> 
>     EFI_EVENT                       EndOfDxeEvent;
> 
> 
> 
> +  mUsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
> 
> +  mUseMiniUart  = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00012000);
> 
> +
> 
>     Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
> 
>                     NULL, (VOID**)&mFwProtocol);
> 
>     ASSERT_EFI_ERROR (Status);
> 
> @@ -859,3 +961,4 @@ ConfigInitialize (
> 
> 
> 
>     return EFI_SUCCESS;
> 
>   }
> 
> +
> 
> diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> new file mode 100644
> 
> index 0000000000..8f7452f97a
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> @@ -0,0 +1,34 @@
> 
> +/** @file
> 
> +
> 
> +  Copyright (c) 2021, ARM Limited. All rights reserved.
> 
> +
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> + **/
> 
> +#ifndef __RPI_DEBUG_PORT_2_H__
> 
> +#define __RPI_DEBUG_PORT_2_H__
> 
> +
> 
> +#include <IndustryStandard/DebugPort2Table.h>
> 
> +
> 
> +#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> 
> +#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> 
> +#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +
> 
> +typedef struct {
> 
> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> 
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> 
> +  UINT32                                                AddressSize;
> 
> +  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> 
> +} DBG2_DEBUG_DEVICE_INFORMATION;
> 
> +
> 
> +typedef struct {
> 
> +  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> 
> +  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> 
> +} DBG2_TABLE;
> 
> +
> 
> +#pragma pack()
> 
> +#endif  //__RPI_DEBUG_PORT_2_H__
> 
> +

And one last unneeded extra line that you can remove.

Hope this can help,

Regards,

/Pete

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

* Re: [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI
  2021-03-24  9:45 [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Sunny Wang
  2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
  2021-03-24 12:44 ` [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Pete Batard
@ 2021-03-24 19:40 ` Jeremy Linton
  2 siblings, 0 replies; 6+ messages in thread
From: Jeremy Linton @ 2021-03-24 19:40 UTC (permalink / raw)
  To: Sunny Wang, devel
  Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Pete Batard, Ard Biesheuvel

Hi,

Thanks for working on this!

Comments inline:


On 3/24/21 4:45 AM, Sunny Wang wrote:
> Changes:
>    1. Add code to ConfigDxe driver and AcpiTables module to dynamically
>       build either Mini UART or PL011 UART info in ACPI. This fixes the
>       issue discussed in https://github.com/pftf/RPi4/issues/118.
>    2. Cleanup by moving duplicate Debug Port 2 table related defines and
>       structures to a newly created header file (RpiDebugPort2Table.h).
> 
> Testing Done:
>    - Booted to UEFI shell and use acpiview command to check the result of
>      the different UART settings in config.txt (enabling either Mini UART
>      or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
>      changed as expected.
> 
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
>   .../RaspberryPi/AcpiTables/AcpiTables.inf     |   9 +-
>   .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 ++++++++
>   .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 ++++++++---------
>   .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +++++++++
>   .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +++++++++---------
>   Platform/RaspberryPi/AcpiTables/Uart.asl      |  10 +-
>   .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +++++++++-
>   .../IndustryStandard/RpiDebugPort2Table.h     |  34 ++++
>   8 files changed, 497 insertions(+), 212 deletions(-)
>   create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
>   rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
>   create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
>   rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
>   create mode 100644 Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> index d3363a76a1..fc8e927138 100644
> --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> @@ -2,7 +2,7 @@
>   #
>   #  ACPI table data and ASL sources required to boot the platform.
>   #
> -#  Copyright (c) 2019, ARM Limited. All rights reserved.
> +#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
>   #  Copyright (c) 2017, Andrey Warkentin <andrey.warkentin@gmail.com>
>   #  Copyright (c) Microsoft Corporation. All rights reserved.
>   #
> @@ -28,12 +28,14 @@
>     Emmc.asl
>     Madt.aslc
>     Fadt.aslc
> -  Dbg2.aslc
> +  Dbg2MiniUart.aslc
> +  Dbg2Pl011.aslc
>     Gtdt.aslc
>     Iort.aslc
>     Dsdt.asl
>     Csrt.aslc
> -  Spcr.aslc
> +  SpcrMiniUart.aslc
> +  SpcrPl011.aslc
>     Pptt.aslc
>     SsdtThermal.asl
> 
> @@ -71,3 +73,4 @@
> 
>   [BuildOptions]
>     GCC:*_*_*_ASL_FLAGS       = -vw3133 -vw3150
> +
> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> new file mode 100644
> index 0000000000..c83b978731
> --- /dev/null
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> @@ -0,0 +1,82 @@
> +/** @file
> + *
> + *  Debug Port Table (DBG2)
> + *
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> + *
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "AcpiTables.h"
> +
> +#pragma pack(1)
> +
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> +#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> +//
> +// RPI_UART_STR should match the value used Uart.asl
> +//
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> +
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> +    {                                                                                                                 \
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> +      0,                                                              /* UINT16    OemDataLength */                   \
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> +    },                                                                                                                \
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> +  }
> +
> +
> +STATIC DBG2_TABLE Dbg2 = {
> +  {
> +    ACPI_HEADER (
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> +      DBG2_TABLE,
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> +    ),
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> +  },
> +  {
> +    /*
> +     * Kernel Debug Port
> +     */
> +    DBG2_DEBUG_PORT_DDI (
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> +      RPI_UART_INTERFACE_TYPE,
> +      RPI_UART_BASE_ADDRESS,
> +      RPI_UART_LENGTH,
> +      RPI_UART_STR
> +    ),
> +  }
> +};
> +
> +#pragma pack()
> +
> +//
> +// Reference the table being generated to prevent the optimizer from removing
> +// the data structure from the executable
> +//
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> +
> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> similarity index 72%
> rename from Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> rename to Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> index e3f2adae7e..dccfa24601 100644
> --- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> @@ -1,105 +1,82 @@
> -/** @file
> - *
> - *  Debug Port Table (DBG2)
> - *
> - *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> - *  Copyright (c) 2012-2020, ARM Limited. All rights reserved.
> - *
> - *  SPDX-License-Identifier: BSD-2-Clause-Patent
> - *
> - **/
> -
> -#include <IndustryStandard/Acpi.h>
> -#include <IndustryStandard/Bcm2836.h>
> -#include <IndustryStandard/DebugPort2Table.h>
> -#include <Library/AcpiLib.h>
> -#include <Library/PcdLib.h>
> -
> -#include "AcpiTables.h"
> -
> -#pragma pack(1)
> -
> -#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> -#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> -#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> -
> -#if (RPI_MODEL == 4)
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> -#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> -#else
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> -#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> -//
> -// RPI_UART_STR should match the value used Uart.asl
> -//
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> -#endif
> -
> -typedef struct {
> -  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> -  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> -  UINT32                                                AddressSize;
> -  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> -} DBG2_DEBUG_DEVICE_INFORMATION;
> -
> -typedef struct {
> -  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> -  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> -} DBG2_TABLE;
> -
> -
> -#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> -    {                                                                                                                 \
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> -      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> -      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> -      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> -      0,                                                              /* UINT16    OemDataLength */                   \
> -      0,                                                              /* UINT16    OemDataOffset */                   \
> -      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> -      SubType,                                                        /* UINT16    Port Subtype */                    \
> -      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> -    },                                                                                                                \
> -    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> -    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> -    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> -  }
> -
> -
> -STATIC DBG2_TABLE Dbg2 = {
> -  {
> -    ACPI_HEADER (
> -      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> -      DBG2_TABLE,
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> -    ),
> -    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> -    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> -  },
> -  {
> -    /*
> -     * Kernel Debug Port
> -     */
> -    DBG2_DEBUG_PORT_DDI (
> -      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> -      RPI_UART_INTERFACE_TYPE,
> -      RPI_UART_BASE_ADDRESS,
> -      RPI_UART_LENGTH,
> -      RPI_UART_STR
> -    ),
> -  }
> -};
> -
> -#pragma pack()
> -
> -//
> -// Reference the table being generated to prevent the optimizer from removing
> -// the data structure from the executable
> -//
> -VOID* CONST ReferenceAcpiTable = &Dbg2;
> +/** @file
> + *
> + *  Debug Port Table (DBG2)
> + *
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> + *
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "AcpiTables.h"
> +
> +#pragma pack(1)
> +
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> +#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> +//
> +// RPI_UART_STR should match the value used Uart.asl
> +//
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> +
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> +    {                                                                                                                 \
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> +      0,                                                              /* UINT16    OemDataLength */                   \
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> +    },                                                                                                                \
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> +  }
> +
> +
> +STATIC DBG2_TABLE Dbg2 = {
> +  {
> +    ACPI_HEADER (
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> +      DBG2_TABLE,
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> +    ),
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> +  },
> +  {
> +    /*
> +     * Kernel Debug Port
> +     */
> +    DBG2_DEBUG_PORT_DDI (
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> +      RPI_UART_INTERFACE_TYPE,
> +      RPI_UART_BASE_ADDRESS,
> +      RPI_UART_LENGTH,
> +      RPI_UART_STR
> +    ),
> +  }
> +};
> +
> +#pragma pack()
> +
> +//
> +// Reference the table being generated to prevent the optimizer from removing
> +// the data structure from the executable
> +//
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> +
> diff --git a/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> new file mode 100644
> index 0000000000..aaf5c5317e
> --- /dev/null
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> @@ -0,0 +1,92 @@
> +/** @file
> +* SPCR Table
> +*
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "AcpiTables.h"
> +
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> +
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> +#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> +#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> +
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> +  ACPI_HEADER (
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> +  ),
> +  // UINT8                                   InterfaceType;
> +  RPI_UART_INTERFACE_TYPE,
> +  // UINT8                                   Reserved1[3];
> +  {
> +    EFI_ACPI_RESERVED_BYTE,
> +    EFI_ACPI_RESERVED_BYTE,
> +    EFI_ACPI_RESERVED_BYTE
> +  },
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> +  // UINT8                                   InterruptType;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> +  // UINT8                                   Irq;
> +  0,                                         // Not used on ARM
> +  // UINT32                                  GlobalSystemInterrupt;
> +  RPI_UART_INTERRUPT,
> +  // UINT8                                   BaudRate;
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> +#else
> +#error Unsupported SPCR Baud Rate
> +#endif
> +  // UINT8                                   Parity;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> +  // UINT8                                   StopBits;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> +  // UINT8                                   FlowControl;
> +  RPI_UART_FLOW_CONTROL_NONE,
> +  // UINT8                                   TerminalType;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> +  // UINT8                                   Reserved2;
> +  EFI_ACPI_RESERVED_BYTE,
> +  // UINT16                                  PciDeviceId;
> +  0xFFFF,
> +  // UINT16                                  PciVendorId;
> +  0xFFFF,
> +  // UINT8                                   PciBusNumber;
> +  0x00,
> +  // UINT8                                   PciDeviceNumber;
> +  0x00,
> +  // UINT8                                   PciFunctionNumber;
> +  0x00,
> +  // UINT32                                  PciFlags;
> +  0x00000000,
> +  // UINT8                                   PciSegment;
> +  0x00,
> +  // UINT32                                  Reserved3;
> +  EFI_ACPI_RESERVED_DWORD
> +};
> +
> +//
> +// Reference the table being generated to prevent the optimizer from removing the
> +// data structure from the executable
> +//
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> +
> diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> similarity index 87%
> rename from Platform/RaspberryPi/AcpiTables/Spcr.aslc
> rename to Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> index 07df3a718d..5a540adf08 100644
> --- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> @@ -1,97 +1,91 @@
> -/** @file
> -* SPCR Table
> -*
> -* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> -* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
> -*
> -* SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#include <IndustryStandard/Acpi.h>
> -#include <IndustryStandard/Bcm2836.h>
> -#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> -#include <Library/AcpiLib.h>
> -#include <Library/PcdLib.h>
> -
> -#include "AcpiTables.h"
> -
> -#define RPI_UART_FLOW_CONTROL_NONE           0
> -
> -// Prefer PL011 serial output on the Raspberry Pi 4
> -#if (RPI_MODEL == 4)
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> -#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> -#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> -#else
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> -#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> -#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> -#endif
> -STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> -  ACPI_HEADER (
> -    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> -  ),
> -  // UINT8                                   InterfaceType;
> -  RPI_UART_INTERFACE_TYPE,
> -  // UINT8                                   Reserved1[3];
> -  {
> -    EFI_ACPI_RESERVED_BYTE,
> -    EFI_ACPI_RESERVED_BYTE,
> -    EFI_ACPI_RESERVED_BYTE
> -  },
> -  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> -  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> -  // UINT8                                   InterruptType;
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> -  // UINT8                                   Irq;
> -  0,                                         // Not used on ARM
> -  // UINT32                                  GlobalSystemInterrupt;
> -  RPI_UART_INTERRUPT,
> -  // UINT8                                   BaudRate;
> -#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> -#else
> -#error Unsupported SPCR Baud Rate
> -#endif
> -  // UINT8                                   Parity;
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> -  // UINT8                                   StopBits;
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> -  // UINT8                                   FlowControl;
> -  RPI_UART_FLOW_CONTROL_NONE,
> -  // UINT8                                   TerminalType;
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> -  // UINT8                                   Reserved2;
> -  EFI_ACPI_RESERVED_BYTE,
> -  // UINT16                                  PciDeviceId;
> -  0xFFFF,
> -  // UINT16                                  PciVendorId;
> -  0xFFFF,
> -  // UINT8                                   PciBusNumber;
> -  0x00,
> -  // UINT8                                   PciDeviceNumber;
> -  0x00,
> -  // UINT8                                   PciFunctionNumber;
> -  0x00,
> -  // UINT32                                  PciFlags;
> -  0x00000000,
> -  // UINT8                                   PciSegment;
> -  0x00,
> -  // UINT32                                  Reserved3;
> -  EFI_ACPI_RESERVED_DWORD
> -};
> -
> -//
> -// Reference the table being generated to prevent the optimizer from removing the
> -// data structure from the executable
> -//
> -VOID* CONST ReferenceAcpiTable = &Spcr;
> +/** @file
> +* SPCR Table
> +*
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "AcpiTables.h"
> +
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> +
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> +#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> +#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> +
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> +  ACPI_HEADER (
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> +  ),
> +  // UINT8                                   InterfaceType;
> +  RPI_UART_INTERFACE_TYPE,
> +  // UINT8                                   Reserved1[3];
> +  {
> +    EFI_ACPI_RESERVED_BYTE,
> +    EFI_ACPI_RESERVED_BYTE,
> +    EFI_ACPI_RESERVED_BYTE
> +  },
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> +  // UINT8                                   InterruptType;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> +  // UINT8                                   Irq;
> +  0,                                         // Not used on ARM
> +  // UINT32                                  GlobalSystemInterrupt;
> +  RPI_UART_INTERRUPT,
> +  // UINT8                                   BaudRate;
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> +#else
> +#error Unsupported SPCR Baud Rate
> +#endif
> +  // UINT8                                   Parity;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> +  // UINT8                                   StopBits;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> +  // UINT8                                   FlowControl;
> +  RPI_UART_FLOW_CONTROL_NONE,
> +  // UINT8                                   TerminalType;
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> +  // UINT8                                   Reserved2;
> +  EFI_ACPI_RESERVED_BYTE,
> +  // UINT16                                  PciDeviceId;
> +  0xFFFF,
> +  // UINT16                                  PciVendorId;
> +  0xFFFF,
> +  // UINT8                                   PciBusNumber;
> +  0x00,
> +  // UINT8                                   PciDeviceNumber;
> +  0x00,
> +  // UINT8                                   PciFunctionNumber;
> +  0x00,
> +  // UINT32                                  PciFlags;
> +  0x00000000,
> +  // UINT8                                   PciSegment;
> +  0x00,
> +  // UINT32                                  Reserved3;
> +  EFI_ACPI_RESERVED_DWORD
> +};
> +
> +//
> +// Reference the table being generated to prevent the optimizer from removing the
> +// data structure from the executable
> +//
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
> index 81ae6711af..8ce297078d 100644
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> @@ -2,6 +2,7 @@
>    *
>    *  [DSDT] Serial devices (UART).
>    *
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
>    *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>    *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
>    *  Copyright (c) Microsoft Corporation. All rights reserved.
> @@ -101,7 +102,10 @@ Device(BTH0)
>     {
>       Name (RBUF, ResourceTemplate ()
>       {
> -      // BT UART: URT0 (PL011) or URTM (miniUART)
> +      //
> +      // BT UART: ResourceSource will be dynamically updated to
> +      // either URT0 (PL011) or URTM (miniUART) during boot
> +      //
>         UARTSerialBus(
>           115200,        // InitialBaudRate: in BPS
>           ,              // BitsPerByte: default to 8 bits
> @@ -126,11 +130,7 @@ Device(BTH0)
>                          //   no flow control.
>           16,            // ReceiveBufferSize
>           16,            // TransmitBufferSize
> -#if (RPI_MODEL == 4)
> -        "\\_SB.GDV0.URTM",  // ResourceSource:
> -#else
>           "\\_SB.GDV0.URT0",  // ResourceSource:
> -#endif
>                          //   UART bus controller name
>           ,              // ResourceSourceIndex: assumed to be 0
>           ,              // ResourceUsage: assumed to be
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 22f86d4d44..68ba14c846 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -1,6 +1,6 @@
>   /** @file
>    *
> - *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> + *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
>    *  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
>    *
>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -12,6 +12,9 @@
>   #include <IndustryStandard/Bcm2836.h>
>   #include <IndustryStandard/Bcm2836Gpio.h>
>   #include <IndustryStandard/RpiMbox.h>
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> +
>   #include <Library/AcpiLib.h>
>   #include <Library/DebugLib.h>
>   #include <Library/DevicePathLib.h>
> @@ -40,6 +43,8 @@ STATIC UINT32 mModelFamily = 0;
>   STATIC UINT32 mModelInstalledMB = 0;
> 
>   STATIC EFI_MAC_ADDRESS  mMacAddress;
> +BOOLEAN                 mUsePl011Uart;
> +BOOLEAN                 mUseMiniUart;

Turning this into a PCD will make more sense later..

> 
>   /*
>    * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
> @@ -699,6 +704,71 @@ UpdateSdtNameOps (
>     }
>   }
> 
> +//
> +// BTH0._HID.BCM2EA6
> +//
> +#define BTH0_HID_PATTERN_LEN 17
> +
> +//
> +// \_SB.GDV0.URT
> +//
> +#define RESOURCE_SOURCE_PATTERN_LEN 13
> +
> +//
> +// Scan the given namespace table (DSDT/SSDT) for AML NameOps
> +// listed in the NameOpReplace structure. If one is found then
> +// update the value in the table from the specified Pcd
> +//
> +// This allows us to have conditionals in AML controlled
> +// by options in the BDS or detected during firmware bootstrap.
> +// We could extend this concept for strings/etc but due to len
> +// variations its probably easier to encode the strings
> +// in the ASL and pick the correct one based off a variable.
> +//
> +STATIC
> +VOID
> +UpdateDevBth0 (
> +  EFI_ACPI_DESCRIPTION_HEADER  *AcpiTable
> +  )
> +{
> +  UINTN   Index;
> +  CHAR8   Bth0HidPattern[BTH0_HID_PATTERN_LEN] = {0x42, 0x54, 0x48, 0x30, 0x08, 0x5F, 0X48, 0x49, 0x44, 0x0D, 0x42, 0x43, 0x4D, 0x32, 0x45, 0x41, 0x36};
> +  CHAR8   ResourceSourcePattern[BTH0_HID_PATTERN_LEN] = {0x5C, 0x5F, 0x53, 0x42, 0x2E, 0x47, 0X44, 0x56, 0x30, 0x2E, 0x55, 0x52, 0x54};
> +  BOOLEAN FoundBth0HidPattern;
> +  UINT8   *SdtPtr;
> +  UINT32  SdtSize;
> +
> +  FoundBth0HidPattern = FALSE;
> +  SdtSize = AcpiTable->Length;
> +
> +  if (SdtSize > 0) {
> +    SdtPtr = (UINT8 *)AcpiTable;
> +    for (Index = 0; Index < (SdtSize - BTH0_HID_PATTERN_LEN); Index++) {
> +      if (!FoundBth0HidPattern) {
> +        if (CompareMem (SdtPtr + Index, Bth0HidPattern, BTH0_HID_PATTERN_LEN) == 0) {
> +          FoundBth0HidPattern = TRUE;
> +        }
> +      } else {
> +        if (CompareMem (SdtPtr + Index, ResourceSourcePattern, RESOURCE_SOURCE_PATTERN_LEN) == 0) {
> +          if (mUsePl011Uart) {
> +            //
> +            // Since PL011 has been set as Primary UART, set the last char in
> +            // ResourceSource string to 'M' (0x4D) so that Mini UART can be used as Secondary UART for BlueTooth.
> +            //
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x4D;
> +          } else if (mUseMiniUart) {
> +            //
> +            // Since Mini UART has been set as Primary UART, set the last char in
> +            // ResourceSource string to '0' (0x30) so that PL011 can be used as Secondary UART for BlueTooth.
> +            //
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x30;
> +          }
> +          break;
> +        }
> +      }
> +    }
> +  }
> +}

So, IMHO its better to extend the existing table driven nameop
replacement scheme than create a compeltely custom function here. Part
of that is because we will want to potentially apply some further user
configuration to the DSDT (say customizing the _PSV value), and some of
it is my own bias to keep this generic.

I think there are at least two ways to pull this off, the first is to
use a new NameOp integer tied to mUse???Uart to select the correct
string (similarly to how the emmc is selecting a differing Package()) in
AML. The other choice is to extend the existing UpdateSsdtNameOps to
suport strings. I personally would pick the first choice since it seems
easier.

> 
>   STATIC
>   BOOLEAN
> @@ -770,8 +840,14 @@ HandleDynamicNamespace (
>   {
>     UINTN Tables;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *SpcrTable;
> +  DBG2_TABLE                                     *Dbg2Table;
> +
>     switch (AcpiHeader->Signature) {
>     case SIGNATURE_32 ('D', 'S', 'D', 'T'):
> +    UpdateDevBth0 (AcpiHeader);
> +    return TRUE;
> +

See comment above about prefering a common DSDT/SSDT path.

>     case SIGNATURE_32 ('S', 'S', 'D', 'T'):
>       for (Tables = 0; SdtTables[Tables].OemTableId; Tables++) {
>         if (AcpiHeader->OemTableId == SdtTables[Tables].OemTableId) {
> @@ -779,14 +855,37 @@ HandleDynamicNamespace (
>         }
>       }
>       DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
> -
>       return FALSE;
> +
>     case SIGNATURE_32 ('I', 'O', 'R', 'T'):
>       // only enable the IORT on machines with >3G and no limit
>       // to avoid problems with rhel/centos and other older OSs
>       if (PcdGet32 (PcdRamLimitTo3GB) || !PcdGet32 (PcdRamMoreThan3GB)) {
>         return FALSE;
>       }
> +    return TRUE;
> +
> +  case SIGNATURE_32 ('S', 'P', 'C', 'R'):
> +    SpcrTable = (EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *)AcpiHeader;
> +    if (mUsePl011Uart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART)) {
> +      return TRUE;
> +    } else if (mUseMiniUart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART)) {
> +      return TRUE;
> +    } else {
> +      return FALSE;
> +    }
> +    return TRUE;

This else/return, return is odd since the second return should be 
redundant yet has a differing return.


> +
> +  case SIGNATURE_32 ('D', 'B', 'G', '2'):
> +    Dbg2Table = (DBG2_TABLE *)AcpiHeader;
> +    if (mUsePl011Uart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)) {
> +      return TRUE;
> +    } else if (mUseMiniUart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART)) {
> +      return TRUE;
> +    } else {
> +      return FALSE;
> +    }
> +    return TRUE;

Same here:

So that said, it might be worth considering if the DBG2/SPCR OemId's are 
diffrent that these table could be handled by the same logic handing 
enabling/disabling the SSDT's since each table is enabled by a single 
true/false value.

>     }
> 
>     return TRUE;
> @@ -803,6 +902,9 @@ ConfigInitialize (
>     EFI_STATUS                      Status;
>     EFI_EVENT                       EndOfDxeEvent;
> 
> +  mUsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
> +  mUseMiniUart  = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00012000);

Is there a case where we don't want at least one of these set?

Thanks again for working on this.



> +
>     Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
>                     NULL, (VOID**)&mFwProtocol);
>     ASSERT_EFI_ERROR (Status);
> @@ -859,3 +961,4 @@ ConfigInitialize (
> 
>     return EFI_SUCCESS;
>   }
> +
> diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> new file mode 100644
> index 0000000000..8f7452f97a
> --- /dev/null
> +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> @@ -0,0 +1,34 @@
> +/** @file
> +
> +  Copyright (c) 2021, ARM Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + **/
> +#ifndef __RPI_DEBUG_PORT_2_H__
> +#define __RPI_DEBUG_PORT_2_H__
> +
> +#include <IndustryStandard/DebugPort2Table.h>
> +
> +#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> +#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> +#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> +
> +#pragma pack(1)
> +
> +
> +typedef struct {
> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> +  UINT32                                                AddressSize;
> +  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> +} DBG2_DEBUG_DEVICE_INFORMATION;
> +
> +typedef struct {
> +  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> +  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> +} DBG2_TABLE;
> +
> +#pragma pack()
> +#endif  //__RPI_DEBUG_PORT_2_H__
> +
> --
> 2.31.0.windows.1
> 


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

* Re: [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
@ 2021-03-24 19:50   ` Jeremy Linton
  2021-03-26 14:33     ` [edk2-devel] " Mario Bălănică
  0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Linton @ 2021-03-24 19:50 UTC (permalink / raw)
  To: Sunny Wang, devel
  Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Pete Batard, Ard Biesheuvel

Hi,

On 3/24/21 4:45 AM, Sunny Wang wrote:
> Merge changes in edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
> https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
> Bluetooth and serial port (Mini UART) in Windows OS.
> 
> Testing Done:
>    - Successfully booted Windows 10 (20279.1) on SD (made by WOR) with
>      the RPi-Windows-Drivers release ver 0.5 downloaded from
>      https://github.com/worproject/RPi-Windows-Drivers/releases
>      and checked that both Bluetooth and serial port (Mini UART) can
>      work fine.
> 
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
>   Platform/RaspberryPi/AcpiTables/Uart.asl | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
> index 8ce297078d..e3165911a6 100644
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> @@ -30,6 +30,12 @@ Device (URT0)
>     {
>       MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>       Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 31 }
>     })
>     Method (_CRS, 0x0, Serialized)
>     {
> @@ -142,7 +148,7 @@ Device(BTH0)
>         //
>         // RPIQ connection for BT_ON/OFF
>         //
> -      GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
> +      //GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
>       })
>       Return (RBUF)
>     }
> 

Having come to this a bit late, its always made me a bit uncomfortable 
that some of these tables are doing blanket power/etc configuration in 
_CRS, is there are reason for continuing this rather than doing this via 
power states?



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

* Re: [edk2-devel] [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-03-24 19:50   ` Jeremy Linton
@ 2021-03-26 14:33     ` Mario Bălănică
  0 siblings, 0 replies; 6+ messages in thread
From: Mario Bălănică @ 2021-03-26 14:33 UTC (permalink / raw)
  To: Jeremy Linton, devel

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

> 
> - GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0,
> ResourceConsumer, , ) { 128 }
> + //GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0,
> ResourceConsumer, , ) { 128 }
> 

This line should be completely removed.
BT_ON is configured by the firmware, and the Windows driver doesn't touch it.

> 
> +
> + PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> + // fake the CTS signal as we don't support HW flow control yet
> + // BCM_ALT2 is set as output (low) by default
> + PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
> 

This always assumes that PL011 is used for Bluetooth. The pin muxing could be done in ConfigDxe depending on which UART is unused.
Also, pin 31 should be set as a LOW output pin rather than relying on the default configuration of BCM_ALT2.

[-- Attachment #2: Type: text/html, Size: 1543 bytes --]

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

end of thread, other threads:[~2021-03-26 14:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-24  9:45 [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Sunny Wang
2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
2021-03-24 19:50   ` Jeremy Linton
2021-03-26 14:33     ` [edk2-devel] " Mario Bălănică
2021-03-24 12:44 ` [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Pete Batard
2021-03-24 19:40 ` Jeremy Linton

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