[edk2-platforms][PATCH 0/4] Platform/RPi: Automate runtime UART selection

[edk2-platforms][PATCH 0/4] Platform/RPi: Automate runtime UART selection

[Pete Batard]

The Raspberry Pi platform contains two UARTs, one PL011-based and the
other (called miniUART) 16650-compatible, that are pinmuxed to the GPIO
serial port according to whether a Device Tree overlay is present in
config.txt or not. In most cases, it takes only the user commenting
or uncommenting an overlay line in their config to switch between PL011
and miniUART.

As such, the use of a build time option to select the UART should be
avoided when it is effectively possible to detect which of the UART
is in use at runtime, through an MMIO call.

This series of patches achieves just this by:

* Adding the relevant clock manager constant to Silicon (which we'll
  need to retrieve the VPU divisor, needed to set the 16550 baudrate).

* Adding a new DualSerialPortLibrary that handles runtime detection
  and switching between PL011 and 16650.

* Enabling the use of DualSerialPortLibrar for both the RPi3 and RPi4.

Important notes:

* This patch does not cover ACPI serial bindings, as this requires
  switching to using DynamicTablesPkg / ConfigurationManagerDxe to
  generate the ACPI tables at runtime. As such, each of the RPi
  platforms is currently hardcoded to use one of the UARTs for ACPI:
  miniUART for RPi3 and PL011 for RPi4. Of course, there is no issue
  for Device Tree usage, since the relevant UART overlay will have
  been applied then. We don't see the current ACPI UART hardcoding
  as a major issue, as this doesn't change anything for RPi3 and we
  expect RPi4 users to prefer PL011 over miniUART anyway, but we
  will look into using DynamicTablesPkg once we see clearer in terms
  of ACPI for RPi4.

* There is work underway to produce a PL011 vs miniUART aware TF-A,
  which we hope will be completed by the next TF-A release. Once
  that release has occurred, we will update the TF-A blobs in 
  non-osi, since the ones we have right now are hardcoded to output
  through one UART only.

* One the subject of TF-A usage, there appears to be an issue when
  using a 16650 (miniUART) based TF-A in a PL011 configuration as
  the system freezes then. This issue does not occur when using a
  PL011 based TF-A in a 16650/miniUART configuration (which is one
  of the the reason why we picked the PL011 TF-A binary over the
  16650 one for RPi4). What this means is that, unless you replace
  the current RPi3 TF-A blobs from non-osi with a version that
  outputs to PL011, and attempt to use a Raspberry Pi 3 in PL011
  mode, then a boot freezout will happen before the UEFI firmware
  gets a chance to apply UART runtime selection. This is an issue
  that will of course resolve itself once we replace the current
  TF-A  blobs with the upcoming runtime selection version. However,
  I can also produce a patch that replaces the current 16650-based
  RPI3 TF-A in non-osi with PL011-based ones, if we think it's
  needed before we get the upcoming runtime selection TF-A binaries.

* It appears that we are issuing serial write calls before the 
  UARTs are initialized, which is a problem if 16650 is being used
  instead of PL011 (produces a freezout similar to what occurs when
  using 16650 TF-A in a PL011 enabled conf). As such, we do perform
  miniUART vs PL011 detection in both initialize and write.

* The 16650 code applies the recent bugfix from Ashish Singhal in
  https://edk2.groups.io/g/devel/message/53487.

Regards,

/Pete

Pete Batard (4):
  Silicon/Broadcom/Bcm283x: Add clock manager constants
  Platform/RPi: Add serial lib for runtime PL011 vs miniUART detection
  Platform/RPi3: Enable the use of DualSerialPortLib
  Platform/RPi4: Enable the use of DualSerialPortLib

 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 836 ++++++++++++++++++++
 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf |  55 ++
 Platform/RaspberryPi/RPi3/RPi3.dsc                                   |  15 +-
 Platform/RaspberryPi/RPi3/Readme.md                                  |   7 +
 Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf                  |   7 +
 Platform/RaspberryPi/RPi4/RPi4.dsc                                   |  26 +-
 Platform/RaspberryPi/RPi4/RPi4.fdf                                   |   4 -
 Platform/RaspberryPi/RPi4/Readme.md                                  |  21 +-
 Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h          |  22 +
 9 files changed, 944 insertions(+), 49 deletions(-)
 create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
 create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

-- 
2.21.0.windows.1

[edk2-platforms][PATCH 1/4] Silicon/Broadcom/Bcm283x: Add clock manager constants

[Pete Batard]

We need these to be able to read the VPU divisor to set the
16650 serial baudrate on the Raspberry Pi.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h | 22 ++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
index dd9a698f7218..7ba4877337a8 100644
--- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
+++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
@@ -31,6 +31,28 @@
 #define BCM2836_WDOG_RSTC_WRCFG_MASK                        0x00000030
 #define BCM2836_WDOG_RSTC_WRCFG_FULL_RESET                  0x00000020
 
+/* clock manager constants */
+#define BCM2836_CM_OFFSET                                   0x00101000
+#define BCM2836_CM_BASE                                     (BCM2836_SOC_REGISTERS + BCM2836_CM_OFFSET)
+#define BCM2836_CM_GEN_CLOCK_CONTROL                        0x0000
+#define BCM2836_CM_GEN_CLOCK_DIVISOR                        0x0004
+#define BCM2836_CM_VPU_CLOCK_CONTROL                        0x0008
+#define BCM2836_CM_VPU_CLOCK_DIVISOR                        0x000c
+#define BCM2836_CM_SYSTEM_CLOCK_CONTROL                     0x0010
+#define BCM2836_CM_SYSTEM_CLOCK_DIVISOR                     0x0014
+#define BCM2836_CM_H264_CLOCK_CONTROL                       0x0028
+#define BCM2836_CM_H264_CLOCK_DIVISOR                       0x002c
+#define BCM2836_CM_PWM_CLOCK_CONTROL                        0x00a0
+#define BCM2836_CM_PWM_CLOCK_DIVISOR                        0x00a4
+#define BCM2836_CM_UART_CLOCK_CONTROL                       0x00f0
+#define BCM2836_CM_UART_CLOCK_DIVISOR                       0x00f4
+#define BCM2836_CM_SDC_CLOCK_CONTROL                        0x01a8
+#define BCM2836_CM_SDC_CLOCK_DIVISOR                        0x01ac
+#define BCM2836_CM_ARM_CLOCK_CONTROL                        0x01b0
+#define BCM2836_CM_ARM_CLOCK_DIVISOR                        0x01b4
+#define BCM2836_CM_EMMC_CLOCK_CONTROL                       0x01c0
+#define BCM2836_CM_EMMC_CLOCK_DIVISOR                       0x01c4
+
 /* mailbox interface constants */
 #define BCM2836_MBOX_OFFSET                                 0x0000b880
 #define BCM2836_MBOX_BASE_ADDRESS                           (BCM2836_SOC_REGISTERS + BCM2836_MBOX_OFFSET)
-- 
2.21.0.windows.1

[edk2-platforms][PATCH 2/4] Platform/RPi: Add serial lib for runtime PL011 vs miniUART detection

[Pete Batard]

The Raspberry Pi platform contains two UARTs, one PL011-based and the
other (called miniUART) 16650-compatible, that are pinmuxed to the GPIO
serial port according to whether a Device Tree overlay is present in
config.txt or not. In most cases, it takes only the user commenting or
uncommenting an overlay line in config.txt to switch between PL011 and
miniUART.

As such, the use of a build time option to select the UART should be
avoided when it is effectively possible to detect which of the UART is
in use at runtime, through a simple MMIO call.

This patch does just this by adding a new DualSerialPortLib that
directs the SerialPortLib implementation to use either 16650 or PL011
according to the GPIO pinmuxing.

It should be noted that this code mostly a merge of
* MdeModulePkg/Library/BaseSerialPortLib16550
* ArmPlatformPkg/Library/PL011SerialPortLib
with non-relevant elements stripped from BaseSerialPortLib16550 (such
as PCI support) and a new call added to retreive the 16650 baudrate
divisor, since it is dependent on the platform's VPU's clock divisor.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 836 ++++++++++++++++++++
 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf |  55 ++
 2 files changed, 891 insertions(+)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
new file mode 100644
index 000000000000..05e12f383785
--- /dev/null
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -0,0 +1,836 @@
+/** @file
+  16550 and PL011 Serial Port library functions for Raspberry Pi
+
+  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+  Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR>
+  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.<BR>
+  Copyright (c) 2012 - 2016, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/Bcm2836Gpio.h>
+#include <Library/BaseLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PL011UartClockLib.h>
+#include <Library/PL011UartLib.h>
+#include <Library/SerialPortLib.h>
+
+BOOLEAN UsePl011Uart          = FALSE;
+BOOLEAN UsePl011UartSet       = FALSE;
+
+#define PL011_UART_REGISTER_BASE      BCM2836_PL011_UART_BASE_ADDRESS
+#define MINI_UART_REGISTER_BASE       (BCM2836_MINI_UART_BASE_ADDRESS + 0x40)
+
+//
+// 16550 UART register offsets and bitfields
+//
+#define R_UART_RXBUF          0   // LCR_DLAB = 0
+#define R_UART_TXBUF          0   // LCR_DLAB = 0
+#define R_UART_BAUD_LOW       0   // LCR_DLAB = 1
+#define R_UART_BAUD_HIGH      1   // LCR_DLAB = 1
+#define R_UART_IER            1   // LCR_DLAB = 0
+#define R_UART_FCR            2
+#define   B_UART_FCR_FIFOE    BIT0
+#define   B_UART_FCR_FIFO64   BIT5
+#define R_UART_LCR            3
+#define   B_UART_LCR_DLAB     BIT7
+#define R_UART_MCR            4
+#define   B_UART_MCR_DTRC     BIT0
+#define   B_UART_MCR_RTS      BIT1
+#define R_UART_LSR            5
+#define   B_UART_LSR_RXRDY    BIT0
+#define   B_UART_LSR_TXRDY    BIT5
+#define   B_UART_LSR_TEMT     BIT6
+#define R_UART_MSR            6
+#define   B_UART_MSR_CTS      BIT4
+#define   B_UART_MSR_DSR      BIT5
+#define   B_UART_MSR_RI       BIT6
+#define   B_UART_MSR_DCD      BIT7
+
+/**
+  Read an 8-bit 16550 register.
+
+  @param  Base    The base address register of UART device.
+  @param  Offset  The offset of the 16550 register to read.
+
+  @return The value read from the 16550 register.
+
+**/
+UINT8
+SerialPortReadRegister (
+  UINTN  Base,
+  UINTN  Offset
+  )
+{
+  return MmioRead8 (Base + Offset * PcdGet32 (PcdSerialRegisterStride));
+}
+
+/**
+  Write an 8-bit 16550 register.
+
+  @param  Base    The base address register of UART device.
+  @param  Offset  The offset of the 16550 register to write.
+  @param  Value   The value to write to the 16550 register specified by Offset.
+
+  @return The value written to the 16550 register.
+
+**/
+UINT8
+SerialPortWriteRegister (
+  UINTN  Base,
+  UINTN  Offset,
+  UINT8  Value
+  )
+{
+  return MmioWrite8 (Base + Offset * PcdGet32 (PcdSerialRegisterStride), Value);
+}
+
+/**
+  Return whether the hardware flow control signal allows writing.
+
+  @param  SerialRegisterBase The base address register of UART device.
+
+  @retval TRUE  The serial port is writable.
+  @retval FALSE The serial port is not writable.
+**/
+BOOLEAN
+SerialPortWritable (
+  UINTN  SerialRegisterBase
+  )
+{
+  if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+    if (PcdGetBool (PcdSerialDetectCable)) {
+      //
+      // Wait for both DSR and CTS to be set
+      //   DSR is set if a cable is connected.
+      //   CTS is set if it is ok to transmit data
+      //
+      //   DSR  CTS  Description                               Action
+      //   ===  ===  ========================================  ========
+      //    0    0   No cable connected.                       Wait
+      //    0    1   No cable connected.                       Wait
+      //    1    0   Cable connected, but not clear to send.   Wait
+      //    1    1   Cable connected, and clear to send.       Transmit
+      //
+      return (BOOLEAN) ((SerialPortReadRegister (SerialRegisterBase, R_UART_MSR) & (B_UART_MSR_DSR | B_UART_MSR_CTS)) == (B_UART_MSR_DSR | B_UART_MSR_CTS));
+    } else {
+      //
+      // Wait for both DSR and CTS to be set OR for DSR to be clear.
+      //   DSR is set if a cable is connected.
+      //   CTS is set if it is ok to transmit data
+      //
+      //   DSR  CTS  Description                               Action
+      //   ===  ===  ========================================  ========
+      //    0    0   No cable connected.                       Transmit
+      //    0    1   No cable connected.                       Transmit
+      //    1    0   Cable connected, but not clear to send.   Wait
+      //    1    1   Cable connected, and clar to send.        Transmit
+      //
+      return (BOOLEAN) ((SerialPortReadRegister (SerialRegisterBase, R_UART_MSR) & (B_UART_MSR_DSR | B_UART_MSR_CTS)) != (B_UART_MSR_DSR));
+    }
+  }
+
+  return TRUE;
+}
+
+/**
+  Return the baud generator divisor to use for 16650 setup.
+
+  @param  SerialBaudRate The desired baud rate.
+
+  @return The baud generator divisor.
+**/
+UINT32
+SerialPortGetDivisor (
+  UINT32  SerialBaudRate
+)
+{
+  UINT64              BaseClockRate;
+  UINT32              Divisor;
+
+  //
+  // On the Raspberry Pi, the clock to use for the 16650-compatible UART
+  // is the base clock divided by the 12.12 fixed point VPU clock divisor.
+  //
+  BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate) * 4;
+  Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;
+  if (Divisor != 0)
+    BaseClockRate = (BaseClockRate << 12) / Divisor;
+
+  //
+  // Now calculate divisor for baud generator
+  //    Ref_Clk_Rate / Baud_Rate / 16
+  //
+  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16);
+  if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= SerialBaudRate * 8) {
+    Divisor++;
+  }
+  return Divisor;
+}
+
+/**
+  Initialize the serial device hardware.
+
+  If no initialization is required, then return RETURN_SUCCESS.
+  If the serial device was successfully initialized, then return RETURN_SUCCESS.
+  If the serial device could not be initialized, then return RETURN_DEVICE_ERROR.
+
+  @retval RETURN_SUCCESS        The serial device was initialized.
+  @retval RETURN_DEVICE_ERROR   The serial device could not be initialized.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortInitialize (
+  VOID
+  )
+{
+  UINTN               SerialRegisterBase;
+  UINT32              Divisor;
+  UINT32              CurrentDivisor;
+  BOOLEAN             Initialized;
+  UINT64              BaudRate;
+  UINT32              ReceiveFifoDepth;
+  EFI_PARITY_TYPE     Parity;
+  UINT8               DataBits;
+  EFI_STOP_BITS_TYPE  StopBits;
+
+  //
+  // First thing we need to do is determine which of PL011 or miniUART is selected
+  //
+  if (!UsePl011UartSet) {
+    UsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
+    UsePl011UartSet = TRUE;
+  }
+
+  if (UsePl011Uart) {
+    BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+    ReceiveFifoDepth = 0;         // Use default FIFO depth
+    Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
+    DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
+    StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
+
+    return PL011UartInitializePort (
+             PL011_UART_REGISTER_BASE,
+             PL011UartClockGetFreq(),
+             &BaudRate,
+             &ReceiveFifoDepth,
+             &Parity,
+             &DataBits,
+             &StopBits
+             );
+  } else {
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+    Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
+
+    //
+    // See if the serial port is already initialized
+    //
+    Initialized = TRUE;
+    if ((SerialPortReadRegister (SerialRegisterBase, R_UART_LCR) & 0x3F) != (PcdGet8 (PcdSerialLineControl) & 0x3F)) {
+      Initialized = FALSE;
+    }
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, (UINT8)(SerialPortReadRegister (SerialRegisterBase, R_UART_LCR) | B_UART_LCR_DLAB));
+    CurrentDivisor =  SerialPortReadRegister (SerialRegisterBase, R_UART_BAUD_HIGH) << 8;
+    CurrentDivisor |= (UINT32) SerialPortReadRegister (SerialRegisterBase, R_UART_BAUD_LOW);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, (UINT8)(SerialPortReadRegister (SerialRegisterBase, R_UART_LCR) & ~B_UART_LCR_DLAB));
+    if (CurrentDivisor != Divisor) {
+      Initialized = FALSE;
+    }
+    if (Initialized) {
+      return RETURN_SUCCESS;
+    }
+
+    //
+    // Wait for the serial port to be ready.
+    // Verify that both the transmit FIFO and the shift register are empty.
+    //
+    while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+
+    //
+    // Configure baud rate
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, B_UART_LCR_DLAB);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_BAUD_HIGH, (UINT8) (Divisor >> 8));
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_BAUD_LOW, (UINT8) (Divisor & 0xff));
+
+    //
+    // Clear DLAB and configure Data Bits, Parity, and Stop Bits.
+    // Strip reserved bits from PcdSerialLineControl
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, (UINT8)(PcdGet8 (PcdSerialLineControl) & 0x3F));
+
+    //
+    // Enable and reset FIFOs
+    // Strip reserved bits from PcdSerialFifoControl
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_FCR, 0x00);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_FCR, (UINT8)(PcdGet8 (PcdSerialFifoControl) & (B_UART_FCR_FIFOE | B_UART_FCR_FIFO64)));
+
+    //
+    // Set FIFO Polled Mode by clearing IER after setting FCR
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_IER, 0x00);
+
+    //
+    // Put Modem Control Register(MCR) into its reset state of 0x00.
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
+
+    return RETURN_SUCCESS;
+  }
+}
+
+/**
+  Write data from buffer to serial device.
+
+  Writes NumberOfBytes data bytes from Buffer to the serial device.
+  The number of bytes actually written to the serial device is returned.
+  If the return value is less than NumberOfBytes, then the write operation failed.
+
+  If Buffer is NULL, then ASSERT().
+
+  If NumberOfBytes is zero, then return 0.
+
+  @param  Buffer           Pointer to the data buffer to be written.
+  @param  NumberOfBytes    Number of bytes to written to the serial device.
+
+  @retval 0                NumberOfBytes is 0.
+  @retval >0               The number of bytes written to the serial device.
+                           If this value is less than NumberOfBytes, then the write operation failed.
+
+**/
+UINTN
+EFIAPI
+SerialPortWrite (
+  IN UINT8     *Buffer,
+  IN UINTN     NumberOfBytes
+  )
+{
+  UINTN  SerialRegisterBase;
+  UINTN  Result;
+  UINTN  Index;
+  UINTN  FifoSize;
+
+  //
+  // Serial writes may happen *before* the UART has been initialized
+  // and if we use the wrong UART then, all kind of bad things happen.
+  // To alleviate this, we add UART detection in SerialPortWrite and
+  // guard the UART detection with a second boolean.
+  //
+  if (!UsePl011UartSet) {
+    UsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
+    UsePl011UartSet = TRUE;
+  }
+
+  if (UsePl011Uart) {
+    return PL011UartWrite (PL011_UART_REGISTER_BASE, Buffer, NumberOfBytes);
+  } else {
+    if (Buffer == NULL) {
+      return 0;
+    }
+
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    if (NumberOfBytes == 0) {
+      //
+      // Flush the hardware
+      //
+
+      //
+      // Wait for both the transmit FIFO and shift register empty.
+      //
+      while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+
+      //
+      // Wait for the hardware flow control signal
+      //
+      while (!SerialPortWritable (SerialRegisterBase));
+      return 0;
+    }
+
+    //
+    // Compute the maximum size of the Tx FIFO
+    //
+    FifoSize = 1;
+    if ((PcdGet8 (PcdSerialFifoControl) & B_UART_FCR_FIFOE) != 0) {
+      if ((PcdGet8 (PcdSerialFifoControl) & B_UART_FCR_FIFO64) == 0) {
+        FifoSize = 16;
+      } else {
+        FifoSize = PcdGet32 (PcdSerialExtendedTxFifoSize);
+      }
+    }
+
+    Result = NumberOfBytes;
+    while (NumberOfBytes != 0) {
+      //
+      // Wait for the serial port to be ready, to make sure both the transmit FIFO
+      // and shift register empty.
+      //
+      while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+
+      //
+      // Fill then entire Tx FIFO
+      //
+      for (Index = 0; Index < FifoSize && NumberOfBytes != 0; Index++, NumberOfBytes--, Buffer++) {
+        //
+        // Wait for the hardware flow control signal
+        //
+        while (!SerialPortWritable (SerialRegisterBase));
+
+        //
+        // Write byte to the transmit buffer.
+        //
+        SerialPortWriteRegister (SerialRegisterBase, R_UART_TXBUF, *Buffer);
+      }
+    }
+    return Result;
+  }
+}
+
+/**
+  Reads data from a serial device into a buffer.
+
+  @param  Buffer           Pointer to the data buffer to store the data read from the serial device.
+  @param  NumberOfBytes    Number of bytes to read from the serial device.
+
+  @retval 0                NumberOfBytes is 0.
+  @retval >0               The number of bytes read from the serial device.
+                           If this value is less than NumberOfBytes, then the read operation failed.
+
+**/
+UINTN
+EFIAPI
+SerialPortRead (
+  OUT UINT8     *Buffer,
+  IN  UINTN     NumberOfBytes
+  )
+{
+  UINTN  SerialRegisterBase;
+  UINTN  Result;
+  UINT8  Mcr;
+
+  if (UsePl011Uart) {
+    return PL011UartRead (PL011_UART_REGISTER_BASE, Buffer, NumberOfBytes);
+  } else {
+    if (NULL == Buffer) {
+      return 0;
+    }
+
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    Mcr = (UINT8)(SerialPortReadRegister (SerialRegisterBase, R_UART_MCR) & ~B_UART_MCR_RTS);
+
+    for (Result = 0; NumberOfBytes-- != 0; Result++, Buffer++) {
+      //
+      // Wait for the serial port to have some data.
+      //
+      while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & B_UART_LSR_RXRDY) == 0) {
+        if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+          //
+          // Set RTS to let the peer send some data
+          //
+          SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, (UINT8)(Mcr | B_UART_MCR_RTS));
+        }
+      }
+      if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+        //
+        // Clear RTS to prevent peer from sending data
+        //
+        SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, Mcr);
+      }
+
+      //
+      // Read byte from the receive buffer.
+      //
+      *Buffer = SerialPortReadRegister (SerialRegisterBase, R_UART_RXBUF);
+    }
+
+    return Result;
+  }
+}
+
+/**
+  Polls a serial device to see if there is any data waiting to be read.
+
+  Polls aserial device to see if there is any data waiting to be read.
+  If there is data waiting to be read from the serial device, then TRUE is returned.
+  If there is no data waiting to be read from the serial device, then FALSE is returned.
+
+  @retval TRUE             Data is waiting to be read from the serial device.
+  @retval FALSE            There is no data waiting to be read from the serial device.
+
+**/
+BOOLEAN
+EFIAPI
+SerialPortPoll (
+  VOID
+  )
+{
+  UINTN  SerialRegisterBase;
+
+  if (UsePl011Uart) {
+    return PL011UartPoll (PL011_UART_REGISTER_BASE);
+  } else {
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    //
+    // Read the serial port status
+    //
+    if ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & B_UART_LSR_RXRDY) != 0) {
+      if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+        //
+        // Clear RTS to prevent peer from sending data
+        //
+        SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, (UINT8)(SerialPortReadRegister (SerialRegisterBase, R_UART_MCR) & ~B_UART_MCR_RTS));
+      }
+      return TRUE;
+    }
+
+    if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+      //
+      // Set RTS to let the peer send some data
+      //
+      SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, (UINT8)(SerialPortReadRegister (SerialRegisterBase, R_UART_MCR) | B_UART_MCR_RTS));
+    }
+
+    return FALSE;
+  }
+}
+
+/**
+  Sets the control bits on a serial device.
+
+  @param Control                Sets the bits of Control that are settable.
+
+  @retval RETURN_SUCCESS        The new control bits were set on the serial device.
+  @retval RETURN_UNSUPPORTED    The serial device does not support this operation.
+  @retval RETURN_DEVICE_ERROR   The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortSetControl (
+  IN UINT32 Control
+  )
+{
+  UINTN SerialRegisterBase;
+  UINT8 Mcr;
+
+  if (UsePl011Uart) {
+    return PL011UartSetControl (PL011_UART_REGISTER_BASE, Control);
+  } else {
+    //
+    // First determine the parameter is invalid.
+    //
+    if ((Control & (~(EFI_SERIAL_REQUEST_TO_SEND | EFI_SERIAL_DATA_TERMINAL_READY |
+                      EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE))) != 0) {
+      return RETURN_UNSUPPORTED;
+    }
+
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    //
+    // Read the Modem Control Register.
+    //
+    Mcr = SerialPortReadRegister (SerialRegisterBase, R_UART_MCR);
+    Mcr &= (~(B_UART_MCR_DTRC | B_UART_MCR_RTS));
+
+    if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == EFI_SERIAL_DATA_TERMINAL_READY) {
+      Mcr |= B_UART_MCR_DTRC;
+    }
+
+    if ((Control & EFI_SERIAL_REQUEST_TO_SEND) == EFI_SERIAL_REQUEST_TO_SEND) {
+      Mcr |= B_UART_MCR_RTS;
+    }
+
+    //
+    // Write the Modem Control Register.
+    //
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, Mcr);
+
+    return RETURN_SUCCESS;
+  }
+}
+
+/**
+  Retrieve the status of the control bits on a serial device.
+
+  @param Control                A pointer to return the current control signals from the serial device.
+
+  @retval RETURN_SUCCESS        The control bits were read from the serial device.
+  @retval RETURN_UNSUPPORTED    The serial device does not support this operation.
+  @retval RETURN_DEVICE_ERROR   The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortGetControl (
+  OUT UINT32 *Control
+  )
+{
+  UINTN SerialRegisterBase;
+  UINT8 Msr;
+  UINT8 Mcr;
+  UINT8 Lsr;
+
+  if (UsePl011Uart) {
+    return PL011UartGetControl (PL011_UART_REGISTER_BASE, Control);
+  } else {
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    *Control = 0;
+
+    //
+    // Read the Modem Status Register.
+    //
+    Msr = SerialPortReadRegister (SerialRegisterBase, R_UART_MSR);
+
+    if ((Msr & B_UART_MSR_CTS) == B_UART_MSR_CTS) {
+      *Control |= EFI_SERIAL_CLEAR_TO_SEND;
+    }
+
+    if ((Msr & B_UART_MSR_DSR) == B_UART_MSR_DSR) {
+      *Control |= EFI_SERIAL_DATA_SET_READY;
+    }
+
+    if ((Msr & B_UART_MSR_RI) == B_UART_MSR_RI) {
+      *Control |= EFI_SERIAL_RING_INDICATE;
+    }
+
+    if ((Msr & B_UART_MSR_DCD) == B_UART_MSR_DCD) {
+      *Control |= EFI_SERIAL_CARRIER_DETECT;
+    }
+
+    //
+    // Read the Modem Control Register.
+    //
+    Mcr = SerialPortReadRegister (SerialRegisterBase, R_UART_MCR);
+
+    if ((Mcr & B_UART_MCR_DTRC) == B_UART_MCR_DTRC) {
+      *Control |= EFI_SERIAL_DATA_TERMINAL_READY;
+    }
+
+    if ((Mcr & B_UART_MCR_RTS) == B_UART_MCR_RTS) {
+      *Control |= EFI_SERIAL_REQUEST_TO_SEND;
+    }
+
+    if (PcdGetBool (PcdSerialUseHardwareFlowControl)) {
+      *Control |= EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE;
+    }
+
+    //
+    // Read the Line Status Register.
+    //
+    Lsr = SerialPortReadRegister (SerialRegisterBase, R_UART_LSR);
+
+    if ((Lsr & (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) == (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) {
+      *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY;
+    }
+
+    if ((Lsr & B_UART_LSR_RXRDY) == 0) {
+      *Control |= EFI_SERIAL_INPUT_BUFFER_EMPTY;
+    }
+
+    return RETURN_SUCCESS;
+  }
+}
+
+/**
+  Sets the baud rate, receive FIFO depth, transmit/receice time out, parity,
+  data bits, and stop bits on a serial device.
+
+  @param BaudRate           The requested baud rate. A BaudRate value of 0 will use the
+                            device's default interface speed.
+                            On output, the value actually set.
+  @param ReveiveFifoDepth   The requested depth of the FIFO on the receive side of the
+                            serial interface. A ReceiveFifoDepth value of 0 will use
+                            the device's default FIFO depth.
+                            On output, the value actually set.
+  @param Timeout            The requested time out for a single character in microseconds.
+                            This timeout applies to both the transmit and receive side of the
+                            interface. A Timeout value of 0 will use the device's default time
+                            out value.
+                            On output, the value actually set.
+  @param Parity             The type of parity to use on this serial device. A Parity value of
+                            DefaultParity will use the device's default parity value.
+                            On output, the value actually set.
+  @param DataBits           The number of data bits to use on the serial device. A DataBits
+                            vaule of 0 will use the device's default data bit setting.
+                            On output, the value actually set.
+  @param StopBits           The number of stop bits to use on this serial device. A StopBits
+                            value of DefaultStopBits will use the device's default number of
+                            stop bits.
+                            On output, the value actually set.
+
+  @retval RETURN_SUCCESS            The new attributes were set on the serial device.
+  @retval RETURN_UNSUPPORTED        The serial device does not support this operation.
+  @retval RETURN_INVALID_PARAMETER  One or more of the attributes has an unsupported value.
+  @retval RETURN_DEVICE_ERROR       The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortSetAttributes (
+  IN OUT UINT64             *BaudRate,
+  IN OUT UINT32             *ReceiveFifoDepth,
+  IN OUT UINT32             *Timeout,
+  IN OUT EFI_PARITY_TYPE    *Parity,
+  IN OUT UINT8              *DataBits,
+  IN OUT EFI_STOP_BITS_TYPE *StopBits
+  )
+{
+  UINTN     SerialRegisterBase;
+  UINT32    SerialBaudRate;
+  UINTN     Divisor;
+  UINT8     Lcr;
+  UINT8     LcrData;
+  UINT8     LcrParity;
+  UINT8     LcrStop;
+
+  if (UsePl011Uart) {
+    return PL011UartInitializePort (
+             PL011_UART_REGISTER_BASE,
+             PL011UartClockGetFreq(),
+             BaudRate,
+             ReceiveFifoDepth,
+             Parity,
+             DataBits,
+             StopBits
+             );
+  } else {
+    SerialRegisterBase = MINI_UART_REGISTER_BASE;
+
+    //
+    // Check for default settings and fill in actual values.
+    //
+    if (*BaudRate == 0) {
+      *BaudRate = PcdGet32 (PcdSerialBaudRate);
+    }
+    SerialBaudRate = (UINT32) *BaudRate;
+
+    if (*DataBits == 0) {
+      LcrData = (UINT8) (PcdGet8 (PcdSerialLineControl) & 0x3);
+      *DataBits = LcrData + 5;
+    } else {
+      if ((*DataBits < 5) || (*DataBits > 8)) {
+        return RETURN_INVALID_PARAMETER;
+      }
+      //
+      // Map 5..8 to 0..3
+      //
+      LcrData = (UINT8) (*DataBits - (UINT8) 5);
+    }
+
+    if (*Parity == DefaultParity) {
+      LcrParity = (UINT8) ((PcdGet8 (PcdSerialLineControl) >> 3) & 0x7);
+      switch (LcrParity) {
+        case 0:
+          *Parity = NoParity;
+          break;
+
+        case 3:
+          *Parity = EvenParity;
+          break;
+
+        case 1:
+          *Parity = OddParity;
+          break;
+
+        case 7:
+          *Parity = SpaceParity;
+          break;
+
+        case 5:
+          *Parity = MarkParity;
+          break;
+
+        default:
+          break;
+      }
+    } else {
+      switch (*Parity) {
+        case NoParity:
+          LcrParity = 0;
+          break;
+
+        case EvenParity:
+          LcrParity = 3;
+          break;
+
+        case OddParity:
+          LcrParity = 1;
+          break;
+
+        case SpaceParity:
+          LcrParity = 7;
+          break;
+
+        case MarkParity:
+          LcrParity = 5;
+          break;
+
+        default:
+          return RETURN_INVALID_PARAMETER;
+      }
+    }
+
+    if (*StopBits == DefaultStopBits) {
+      LcrStop = (UINT8) ((PcdGet8 (PcdSerialLineControl) >> 2) & 0x1);
+      switch (LcrStop) {
+        case 0:
+          *StopBits = OneStopBit;
+          break;
+
+        case 1:
+          if (*DataBits == 5) {
+            *StopBits = OneFiveStopBits;
+          } else {
+            *StopBits = TwoStopBits;
+          }
+          break;
+
+        default:
+          break;
+      }
+    } else {
+      switch (*StopBits) {
+        case OneStopBit:
+          LcrStop = 0;
+          break;
+
+        case OneFiveStopBits:
+        case TwoStopBits:
+          LcrStop = 1;
+          break;
+
+        default:
+          return RETURN_INVALID_PARAMETER;
+      }
+    }
+
+    //
+    // Configure baud rate
+    //
+    Divisor = SerialPortGetDivisor (SerialBaudRate);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, B_UART_LCR_DLAB);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_BAUD_HIGH, (UINT8) (Divisor >> 8));
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_BAUD_LOW, (UINT8) (Divisor & 0xff));
+
+    //
+    // Clear DLAB and configure Data Bits, Parity, and Stop Bits.
+    // Strip reserved bits from line control value
+    //
+    Lcr = (UINT8) ((LcrParity << 3) | (LcrStop << 2) | LcrData);
+    SerialPortWriteRegister (SerialRegisterBase, R_UART_LCR, (UINT8) (Lcr & 0x3F));
+
+    return RETURN_SUCCESS;
+  }
+}
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
new file mode 100644
index 000000000000..af1e6b026fe6
--- /dev/null
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
@@ -0,0 +1,55 @@
+## @file
+#
+#  SerialPortLib instance for both PL011 and 16550 UART.
+#
+#  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = DualSerialPortLib
+  FILE_GUID                      = FD3EB93E-B59E-42B7-ABA4-ADA4B988B095
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SerialPortLib
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  Silicon/Broadcom/Bcm283x/Bcm283x.dec
+
+[LibraryClasses]
+  IoLib
+  PcdLib
+  PL011UartClockLib
+  PL011UartLib
+
+[Sources]
+  DualSerialPortLib.c
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterAccessWidth     ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio                 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl  ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialDetectCable             ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate                ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl             ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl             ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate               ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo           ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride          ## CONSUMES
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PL011UartClkInHz
+  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
-- 
2.21.0.windows.1

[edk2-platforms][PATCH 3/4] Platform/RPi3: Enable the use of DualSerialPortLib

[Pete Batard]

Note that, for the time being, the TF-A binary only outputs to the
miniUART, but work is underway to add runtime UART detection to TF-A,
after which we will use such a binary.

Also note that this patch currently enforces the use of miniUART in
the ACPI tables, as we have to pick one until we can switch to using
DynamicTablesPkg/ConfigurationManagerDxe for ACPI generation.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/RPi3/RPi3.dsc  | 15 +++++++++------
 Platform/RaspberryPi/RPi3/Readme.md |  7 +++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 2368b04f40a4..40fb0879c2ed 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -114,10 +114,10 @@ [LibraryClasses.common]
   ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
 
-  PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
-  PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
-  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
-  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  # Dual serial port library
+  PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
+  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
+  SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
 
   # Cryptographic libraries
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
@@ -379,14 +379,17 @@ [PcdsFixedAtBuild.common]
   #
   gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x3f000000
 
-  ## NS16550 compatible UART
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x3f215040
+  # UARTs
+  gArmPlatformTokenSpaceGuid.PL011UartInteger|0
+  gArmPlatformTokenSpaceGuid.PL011UartFractional|0
+  gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
 
   ## Default Terminal Type
   ## 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
diff --git a/Platform/RaspberryPi/RPi3/Readme.md b/Platform/RaspberryPi/RPi3/Readme.md
index 797da1bab4a9..58c0124ef75e 100644
--- a/Platform/RaspberryPi/RPi3/Readme.md
+++ b/Platform/RaspberryPi/RPi3/Readme.md
@@ -52,6 +52,13 @@ Build instructions from the top level edk2-platforms Readme.md apply.
   armstub=RPI_EFI.fd
   disable_commandline_tags=1
   ```
+  Additionally, if you want to use PL011 instead of the miniUART, you can add the lines:
+  ```
+  device_tree_address=0x20000
+  device_tree_end=0x30000
+  device_tree=bcm2710-rpi-3-b[-plus].dtb
+  dtoverlay=miniuart-bt
+  ```
 5. Insert the uSD card and power up the Pi.
 
 Note that if you have a model 3+ or a model 3 where you enabled USB boot through OTP
-- 
2.21.0.windows.1

[edk2-platforms][PATCH 4/4] Platform/RPi4: Enable the use of DualSerialPortLib

[Pete Batard]

With DualSerialPortLib available, we can remove the PL011_ENABLE
option and use this library instead of being tied to selecting only
one of PL011SerialPortLib or BaseSerialPortLib16550.

Note that, for the time being, we choose to default to selecting the
PL011 based TF-A binary, since we have to pick one and we expect that
most usage of the firmware will be for a PL011 configuration (we of
course validated that the only drawback of using PL011 with a miniUART
configuration the loss of the 2 lines of serial debug output from TF-A
on startup and that there was no other issue besides that), but work
is underway to add runtime UART detection to TF-A, after which we will
revert to using a single TF-A binary that supports both UARTs.

Also note that this patch currently enforces the use of PL011 for the
ACPI tables, as we also have to pick one until we can switch to using
DynamicTablesPkg/ConfigurationManagerDxe for ACPI generation, which
we should do in a future update, and which will enable us to update
the ACPI tables at runtime according to the user-selected UART.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |  7 ++++++
 Platform/RaspberryPi/RPi4/RPi4.dsc                  | 26 +++-----------------
 Platform/RaspberryPi/RPi4/RPi4.fdf                  |  4 ---
 Platform/RaspberryPi/RPi4/Readme.md                 | 21 ++++------------
 4 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
index 5ce4c0b52b32..aa8f67dec95e 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
@@ -54,3 +54,10 @@ [FixedPcd]
   gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
   gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
+
+# The following is a stopgap solution to default to PL011
+# usage in ACPI (most common case), until we can switch
+# to using DynamicTablesPkg/ConfigurationManagerDxe.
+[BuildOptions]
+  GCC:*_*_*_ASLPP_FLAGS       = -DPL011_ENABLE
+  GCC:*_*_*_ASLCC_FLAGS       = -DPL011_ENABLE
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index bd3800c1d653..7c1937672597 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -38,7 +38,6 @@ [Defines]
   DEFINE SECURE_BOOT_ENABLE      = FALSE
   DEFINE INCLUDE_TFTP_COMMAND    = FALSE
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
-  DEFINE PL011_ENABLE            = FALSE
   DEFINE ACPI_BASIC_MODE_ENABLE  = FALSE
 
 ################################################################################
@@ -118,16 +117,10 @@ [LibraryClasses.common]
   ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
 
-!if $(PL011_ENABLE) == TRUE
+  # Dual serial port library
   PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
-  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
-!else
-  PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
-  PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
-  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
-  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
-!endif
+  SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
 
   # Cryptographic libraries
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
@@ -239,12 +232,6 @@ [BuildOptions]
   GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
   GCC:RELEASE_*_*_CC_FLAGS    = -DMDEPKG_NDEBUG -DNDEBUG
 
-!if $(PL011_ENABLE) == TRUE
-  GCC:*_*_*_CC_FLAGS          = -DPL011_ENABLE
-  GCC:*_*_*_ASLPP_FLAGS       = -DPL011_ENABLE
-  GCC:*_*_*_ASLCC_FLAGS       = -DPL011_ENABLE
-!endif
-
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
 
@@ -410,22 +397,15 @@ [PcdsFixedAtBuild.common]
   gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
   gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
 
-!if $(PL011_ENABLE) == TRUE
-  ## PL011 UART
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000
+  # UARTs
   gArmPlatformTokenSpaceGuid.PL011UartInteger|0
   gArmPlatformTokenSpaceGuid.PL011UartFractional|0
   gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
-!else
-  ## NS16550 compatible UART
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe215040
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
-!endif
-
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
 
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index db393d47bcea..52ae1e5b65cb 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -51,11 +51,7 @@ [FD.RPI_EFI]
 # ATF primary boot image
 #
 0x00000000|0x00020000
-!if $(PL011_ENABLE) == TRUE
 FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
-!else
-FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_miniuart.bin
-!endif
 
 #
 # DTB.
diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md
index 74afc0f89451..758d0124a6cf 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -20,8 +20,8 @@ following __major__ limitations:
   missing/incomplete ACPI tables as well as other factors. For Linux, using
   the `ACPI_BASIC_MODE_ENABLE` build option may help.
 - Serial I/O from the OS may not work due to CPU throttling affecting the
-  miniUART baudrate. This can be worked around by using the `PL011_ENABLE`
-  compilation option.
+  miniUART baudrate. This can be worked around by using the PL011 UART
+  through the device tree overlays.
 
 # Building
 
@@ -32,11 +32,6 @@ The following additional build options are also available:
   ACPI (by disabling the Device Tree driver altogether). This may be required
   to boot Operating Systems such as Linux on account of the current PCIe/xHCI
   limitations.
-- `-D PL011_ENABLE=1`: Selects PL011 for the serial console instead of the
-  miniUART (default). This doesn't change the GPIO pinout for the UART but
-  can be useful if you find that the miniUART baud rate changes when the
-  OS throttles the CPU. Note that this requires one of `disable-bt.dtbo` or
-  `miniuart-bt.dtbo` overlays to have been applied (see below).
 
 # Booting the firmware
 
@@ -48,27 +43,21 @@ The following additional build options are also available:
   - `start4.elf`
   - `overlays/miniuart-bt.dbto` or `overlays/disable-bt.dtbo` (Optional)
 4. Create a `config.txt` with the following content:
-  - For a firmware **without** the `PL011_ENABLE` build option:
     ```
     arm_64bit=1
     enable_uart=1
-    core_freq=250
     enable_gic=1
     armstub=RPI_EFI.fd
     disable_commandline_tags=1
     ```
-  - For a firmware **with** the `PL011_ENABLE` build option:
+    Additionally, if you want to use PL011 instead of the miniUART, you can add the lines:
     ```
-    arm_64bit=1
-    enable_gic=1
-    armstub=RPI_EFI.fd
-    disable_commandline_tags=1
     device_tree_address=0x20000
     device_tree_end=0x30000
     device_tree=bcm2711-rpi-4-b.dtb
     dtoverlay=miniuart-bt
     ```
-    The above also requires `miniuart-bt.dbto` to have been copied into an `overlays/`
+    Note that doing so requires `miniuart-bt.dbto` to have been copied into an `overlays/`
     directory on the uSD card. Alternatively, you may use `disable-bt` instead of
     `miniuart-bt` if you don't require BlueTooth.
 5. Insert the uSD card and power up the Pi.
@@ -80,7 +69,7 @@ The following additional build options are also available:
 The TF-A binaries were compiled from a TF-A source over which 2 serial-output related
 patches were applied, the first one to fix the miniUART baud rate not being properly
 set to 115200 bauds with recent versions of `start4.elf` and the second one to allow
-swicthing between miniUART and PL011 at build time.
+the use of the PL011 UART.
 
 No other alterations to the official source have been applied.
 
-- 
2.21.0.windows.1

Re: [edk2-platforms][PATCH 0/4] Platform/RPi: Automate runtime UART selection

[Ard Biesheuvel]

Hey Pete,

On Tue, 28 Jan 2020 at 18:20, Pete Batard <pete@akeo.ie> wrote:
>
> The Raspberry Pi platform contains two UARTs, one PL011-based and the
> other (called miniUART) 16650-compatible, that are pinmuxed to the GPIO
> serial port according to whether a Device Tree overlay is present in
> config.txt or not. In most cases, it takes only the user commenting
> or uncommenting an overlay line in their config to switch between PL011
> and miniUART.
>
> As such, the use of a build time option to select the UART should be
> avoided when it is effectively possible to detect which of the UART
> is in use at runtime, through an MMIO call.
>
> This series of patches achieves just this by:
>
> * Adding the relevant clock manager constant to Silicon (which we'll
>   need to retrieve the VPU divisor, needed to set the 16550 baudrate).
>
> * Adding a new DualSerialPortLibrary that handles runtime detection
>   and switching between PL011 and 16650.
>
> * Enabling the use of DualSerialPortLibrar for both the RPi3 and RPi4.
>
> Important notes:
>
> * This patch does not cover ACPI serial bindings, as this requires
>   switching to using DynamicTablesPkg / ConfigurationManagerDxe to
>   generate the ACPI tables at runtime. As such, each of the RPi
>   platforms is currently hardcoded to use one of the UARTs for ACPI:
>   miniUART for RPi3 and PL011 for RPi4. Of course, there is no issue
>   for Device Tree usage, since the relevant UART overlay will have
>   been applied then. We don't see the current ACPI UART hardcoding
>   as a major issue, as this doesn't change anything for RPi3 and we
>   expect RPi4 users to prefer PL011 over miniUART anyway, but we
>   will look into using DynamicTablesPkg once we see clearer in terms
>   of ACPI for RPi4.
>

I'd prefer avoiding DynamicTablesPkg if we can, especially for simple
cases like this one, where we can simply carry two SSDTs in the
firmware, and only install the one that matches the configuration we
detect at runtime.

However, it highly depends on whether OS support for the miniUART
emerges in ACPI mode, or the point is moot afaict.

> * There is work underway to produce a PL011 vs miniUART aware TF-A,
>   which we hope will be completed by the next TF-A release. Once
>   that release has occurred, we will update the TF-A blobs in
>   non-osi, since the ones we have right now are hardcoded to output
>   through one UART only.
>
> * One the subject of TF-A usage, there appears to be an issue when
>   using a 16650 (miniUART) based TF-A in a PL011 configuration as
>   the system freezes then. This issue does not occur when using a
>   PL011 based TF-A in a 16650/miniUART configuration (which is one
>   of the the reason why we picked the PL011 TF-A binary over the
>   16650 one for RPi4). What this means is that, unless you replace
>   the current RPi3 TF-A blobs from non-osi with a version that
>   outputs to PL011, and attempt to use a Raspberry Pi 3 in PL011
>   mode, then a boot freezout will happen before the UEFI firmware
>   gets a chance to apply UART runtime selection. This is an issue
>   that will of course resolve itself once we replace the current
>   TF-A  blobs with the upcoming runtime selection version. However,
>   I can also produce a patch that replaces the current 16650-based
>   RPI3 TF-A in non-osi with PL011-based ones, if we think it's
>   needed before we get the upcoming runtime selection TF-A binaries.
>
> * It appears that we are issuing serial write calls before the
>   UARTs are initialized, which is a problem if 16650 is being used
>   instead of PL011 (produces a freezout similar to what occurs when
>   using 16650 TF-A in a PL011 enabled conf). As such, we do perform
>   miniUART vs PL011 detection in both initialize and write.
>
> * The 16650 code applies the recent bugfix from Ashish Singhal in
>   https://edk2.groups.io/g/devel/message/53487.
>
> Regards,
>
> /Pete
>
> Pete Batard (4):
>   Silicon/Broadcom/Bcm283x: Add clock manager constants
>   Platform/RPi: Add serial lib for runtime PL011 vs miniUART detection
>   Platform/RPi3: Enable the use of DualSerialPortLib
>   Platform/RPi4: Enable the use of DualSerialPortLib
>
>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 836 ++++++++++++++++++++
>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf |  55 ++
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                   |  15 +-
>  Platform/RaspberryPi/RPi3/Readme.md                                  |   7 +
>  Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf                  |   7 +
>  Platform/RaspberryPi/RPi4/RPi4.dsc                                   |  26 +-
>  Platform/RaspberryPi/RPi4/RPi4.fdf                                   |   4 -
>  Platform/RaspberryPi/RPi4/Readme.md                                  |  21 +-
>  Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h          |  22 +
>  9 files changed, 944 insertions(+), 49 deletions(-)
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
>
> --
> 2.21.0.windows.1
>

Re: [edk2-platforms][PATCH 0/4] Platform/RPi: Automate runtime UART selection

[Ard Biesheuvel]

On Tue, 28 Jan 2020 at 18:20, Pete Batard <pete@akeo.ie> wrote:
>
> The Raspberry Pi platform contains two UARTs, one PL011-based and the
> other (called miniUART) 16650-compatible, that are pinmuxed to the GPIO
> serial port according to whether a Device Tree overlay is present in
> config.txt or not. In most cases, it takes only the user commenting
> or uncommenting an overlay line in their config to switch between PL011
> and miniUART.
>
> As such, the use of a build time option to select the UART should be
> avoided when it is effectively possible to detect which of the UART
> is in use at runtime, through an MMIO call.
>
> This series of patches achieves just this by:
>
> * Adding the relevant clock manager constant to Silicon (which we'll
>   need to retrieve the VPU divisor, needed to set the 16550 baudrate).
>
> * Adding a new DualSerialPortLibrary that handles runtime detection
>   and switching between PL011 and 16650.
>
> * Enabling the use of DualSerialPortLibrar for both the RPi3 and RPi4.
>
> Important notes:
>
> * This patch does not cover ACPI serial bindings, as this requires
>   switching to using DynamicTablesPkg / ConfigurationManagerDxe to
>   generate the ACPI tables at runtime. As such, each of the RPi
>   platforms is currently hardcoded to use one of the UARTs for ACPI:
>   miniUART for RPi3 and PL011 for RPi4. Of course, there is no issue
>   for Device Tree usage, since the relevant UART overlay will have
>   been applied then. We don't see the current ACPI UART hardcoding
>   as a major issue, as this doesn't change anything for RPi3 and we
>   expect RPi4 users to prefer PL011 over miniUART anyway, but we
>   will look into using DynamicTablesPkg once we see clearer in terms
>   of ACPI for RPi4.
>
> * There is work underway to produce a PL011 vs miniUART aware TF-A,
>   which we hope will be completed by the next TF-A release. Once
>   that release has occurred, we will update the TF-A blobs in
>   non-osi, since the ones we have right now are hardcoded to output
>   through one UART only.
>
> * One the subject of TF-A usage, there appears to be an issue when
>   using a 16650 (miniUART) based TF-A in a PL011 configuration as
>   the system freezes then. This issue does not occur when using a
>   PL011 based TF-A in a 16650/miniUART configuration (which is one
>   of the the reason why we picked the PL011 TF-A binary over the
>   16650 one for RPi4). What this means is that, unless you replace
>   the current RPi3 TF-A blobs from non-osi with a version that
>   outputs to PL011, and attempt to use a Raspberry Pi 3 in PL011
>   mode, then a boot freezout will happen before the UEFI firmware
>   gets a chance to apply UART runtime selection. This is an issue
>   that will of course resolve itself once we replace the current
>   TF-A  blobs with the upcoming runtime selection version. However,
>   I can also produce a patch that replaces the current 16650-based
>   RPI3 TF-A in non-osi with PL011-based ones, if we think it's
>   needed before we get the upcoming runtime selection TF-A binaries.
>
> * It appears that we are issuing serial write calls before the
>   UARTs are initialized, which is a problem if 16650 is being used
>   instead of PL011 (produces a freezout similar to what occurs when
>   using 16650 TF-A in a PL011 enabled conf). As such, we do perform
>   miniUART vs PL011 detection in both initialize and write.
>
> * The 16650 code applies the recent bugfix from Ashish Singhal in
>   https://edk2.groups.io/g/devel/message/53487.
>
> Regards,
>
> /Pete
>
> Pete Batard (4):
>   Silicon/Broadcom/Bcm283x: Add clock manager constants
>   Platform/RPi: Add serial lib for runtime PL011 vs miniUART detection
>   Platform/RPi3: Enable the use of DualSerialPortLib
>   Platform/RPi4: Enable the use of DualSerialPortLib
>

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

Pushed as 9369b01e86ad..41c1d9ba3304

Thanks!

>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 836 ++++++++++++++++++++
>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf |  55 ++
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                   |  15 +-
>  Platform/RaspberryPi/RPi3/Readme.md                                  |   7 +
>  Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf                  |   7 +
>  Platform/RaspberryPi/RPi4/RPi4.dsc                                   |  26 +-
>  Platform/RaspberryPi/RPi4/RPi4.fdf                                   |   4 -
>  Platform/RaspberryPi/RPi4/Readme.md                                  |  21 +-
>  Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h          |  22 +
>  9 files changed, 944 insertions(+), 49 deletions(-)
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
>
> --
> 2.21.0.windows.1
>