public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library
@ 2017-11-16 17:47 Ard Biesheuvel
  2017-11-16 17:47 ` [PATCH v2 1/2] ArmPlatformPkg: reorganize PL011 code Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-11-16 17:47 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm; +Cc: lersek, Ard Biesheuvel

Clean up the PL011 UART support library, by moving it into the appropriate
place for libraries, and splitting the header file into an implementation
and an interface part.

Ard Biesheuvel (2):
  ArmPlatformPkg: reorganize PL011 code
  ArmVirtPkg: switch to new PL011UartLib implementation

 ArmPlatformPkg/ArmPlatformPkg.dec                                     |   3 +
 ArmPlatformPkg/Include/Library/PL011UartLib.h                         | 187 ++++++++
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c        |   5 +-
 ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h                       | 120 +++++
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c                    | 474 ++++++++++++++++++++
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf                  |  43 ++
 ArmVirtPkg/ArmVirt.dsc.inc                                            |   2 +-
 ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c |   5 +-
 ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      |   5 +-
 9 files changed, 834 insertions(+), 10 deletions(-)
 create mode 100644 ArmPlatformPkg/Include/Library/PL011UartLib.h
 create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h
 create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
 create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf

-- 
2.11.0



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

* [PATCH v2 1/2] ArmPlatformPkg: reorganize PL011 code
  2017-11-16 17:47 [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Ard Biesheuvel
@ 2017-11-16 17:47 ` Ard Biesheuvel
  2017-11-16 17:47 ` [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation Ard Biesheuvel
  2017-11-16 18:41 ` [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Leif Lindholm
  2 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-11-16 17:47 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm; +Cc: lersek, Ard Biesheuvel

The PL011 code in ArmPlatformPkg is organized in a weird way: there is
a single PL011Uart.h header file under Include/Drivers containing both
register definitions and function entry points. The PL011Uart library
itself is in Drivers/ but it is actually a library.

So let's clean this up: add a new PL011UartLib library class and associated
header file containing only the library prototypes, and move the library
itself under Library/ using a new GUID, with the register definitions moved
into a local header file.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/ArmPlatformPkg.dec                              |   3 +
 ArmPlatformPkg/Include/Library/PL011UartLib.h                  | 187 ++++++++
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c |   5 +-
 ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h                | 120 +++++
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c             | 474 ++++++++++++++++++++
 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf           |  43 ++
 6 files changed, 829 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 2d82ead7612a..e282e76667b1 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -31,6 +31,9 @@ [Defines]
 [Includes.common]
   Include                        # Root include for the package
 
+[LibraryClasses]
+  PL011UartLib|Include/Library/PL011UartLib.h
+
 [Guids.common]
   gArmPlatformTokenSpaceGuid   = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
   #
diff --git a/ArmPlatformPkg/Include/Library/PL011UartLib.h b/ArmPlatformPkg/Include/Library/PL011UartLib.h
new file mode 100644
index 000000000000..9e923a2218d1
--- /dev/null
+++ b/ArmPlatformPkg/Include/Library/PL011UartLib.h
@@ -0,0 +1,187 @@
+/** @file
+*
+*  Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD License
+*  which accompanies this distribution.  The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#ifndef __PL011_UART_LIB_H__
+#define __PL011_UART_LIB_H__
+
+#include <Protocol/SerialIo.h>
+
+/**
+
+  Initialise the serial port to the specified settings.
+  All unspecified settings will be set to the default values.
+
+  @param[in]  UartBase            The base address of the serial device.
+  @param[in]  UartClkInHz         The clock in Hz for the serial device.
+                                  Ignored if the PCD PL011UartInteger is not 0
+  @param[in out] BaudRate         The baud rate of the serial device. If the
+                                  baud rate is not supported, the speed will be
+                                  reduced to the nearest supported one and the
+                                  variable's value will be updated accordingly.
+  @param[in out] ReceiveFifoDepth The number of characters the device will
+                                  buffer on input.  Value of 0 will use the
+                                  device's default FIFO depth.
+  @param[in out]  Parity          If applicable, this is the EFI_PARITY_TYPE
+                                  that is computed or checked as each character
+                                  is transmitted or received. If the device
+                                  does not support parity, the value is the
+                                  default parity value.
+  @param[in out]  DataBits        The number of data bits in each character.
+  @param[in out]  StopBits        If applicable, the EFI_STOP_BITS_TYPE number
+                                  of stop bits per character.
+                                  If the device does not support stop bits, the
+                                  value is the default stop bit value.
+
+  @retval RETURN_SUCCESS            All attributes were set correctly on the
+                                    serial device.
+  @retval RETURN_INVALID_PARAMETER  One or more of the attributes has an
+                                    unsupported value.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartInitializePort (
+  IN     UINTN               UartBase,
+  IN     UINT32              UartClkInHz,
+  IN OUT UINT64              *BaudRate,
+  IN OUT UINT32              *ReceiveFifoDepth,
+  IN OUT EFI_PARITY_TYPE     *Parity,
+  IN OUT UINT8               *DataBits,
+  IN OUT EFI_STOP_BITS_TYPE  *StopBits
+  );
+
+/**
+
+  Assert or deassert the control signals on a serial port.
+  The following control signals are set according their bit settings :
+  . Request to Send
+  . Data Terminal Ready
+
+  @param[in]  UartBase  UART registers base address
+  @param[in]  Control   The following bits are taken into account :
+                        . EFI_SERIAL_REQUEST_TO_SEND : assert/deassert the
+                          "Request To Send" control signal if this bit is
+                          equal to one/zero.
+                        . EFI_SERIAL_DATA_TERMINAL_READY : assert/deassert
+                          the "Data Terminal Ready" control signal if this
+                          bit is equal to one/zero.
+                        . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : enable/disable
+                          the hardware loopback if this bit is equal to
+                          one/zero.
+                        . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : not supported.
+                        . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : enable/
+                          disable the hardware flow control based on CTS (Clear
+                          To Send) and RTS (Ready To Send) control signals.
+
+  @retval  RETURN_SUCCESS      The new control bits were set on the device.
+  @retval  RETURN_UNSUPPORTED  The device does not support this operation.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartSetControl (
+  IN UINTN   UartBase,
+  IN UINT32  Control
+  );
+
+/**
+
+  Retrieve the status of the control bits on a serial device.
+
+  @param[in]   UartBase  UART registers base address
+  @param[out]  Control   Status of the control bits on a serial device :
+
+                         . EFI_SERIAL_DATA_CLEAR_TO_SEND,
+                           EFI_SERIAL_DATA_SET_READY,
+                           EFI_SERIAL_RING_INDICATE,
+                           EFI_SERIAL_CARRIER_DETECT,
+                           EFI_SERIAL_REQUEST_TO_SEND,
+                           EFI_SERIAL_DATA_TERMINAL_READY
+                           are all related to the DTE (Data Terminal Equipment)
+                           and DCE (Data Communication Equipment) modes of
+                           operation of the serial device.
+                         . EFI_SERIAL_INPUT_BUFFER_EMPTY : equal to one if the
+                           receive buffer is empty, 0 otherwise.
+                         . EFI_SERIAL_OUTPUT_BUFFER_EMPTY : equal to one if the
+                           transmit buffer is empty, 0 otherwise.
+                         . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : equal to one if
+                           the hardware loopback is enabled (the ouput feeds the
+                           receive buffer), 0 otherwise.
+                         . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : equal to one if
+                           a loopback is accomplished by software, 0 otherwise.
+                         . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : equal to
+                           one if the hardware flow control based on CTS (Clear
+                           To Send) and RTS (Ready To Send) control signals is
+                           enabled, 0 otherwise.
+
+  @retval RETURN_SUCCESS  The control bits were read from the serial device.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartGetControl (
+  IN UINTN     UartBase,
+  OUT UINT32  *Control
+  );
+
+/**
+  Write data to serial device.
+
+  @param  Buffer           Point of data buffer which need to be written.
+  @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
+
+  @retval 0                Write data failed.
+  @retval !0               Actual number of bytes written to serial device.
+
+**/
+UINTN
+EFIAPI
+PL011UartWrite (
+  IN  UINTN       UartBase,
+  IN  UINT8       *Buffer,
+  IN  UINTN       NumberOfBytes
+  );
+
+/**
+  Read data from serial device and save the data in buffer.
+
+  @param  Buffer           Point of data buffer which need to be written.
+  @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
+
+  @retval 0                Read data failed.
+  @retval !0               Actual number of bytes read from serial device.
+
+**/
+UINTN
+EFIAPI
+PL011UartRead (
+  IN  UINTN       UartBase,
+  OUT UINT8       *Buffer,
+  IN  UINTN       NumberOfBytes
+  );
+
+/**
+  Check to see if any data is available to be read from the debug device.
+
+  @retval TRUE       At least one byte of data is available to be read
+  @retval FALSE      No data is available to be read
+
+**/
+BOOLEAN
+EFIAPI
+PL011UartPoll (
+  IN  UINTN       UartBase
+  );
+
+#endif
diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
index 571d0618c379..9c54e7880e4c 100644
--- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
+++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
@@ -15,14 +15,13 @@
 
 **/
 
-#include <Base.h>
+#include <Uefi.h>
 
 #include <Library/IoLib.h>
 #include <Library/PcdLib.h>
+#include <Library/PL011UartLib.h>
 #include <Library/SerialPortLib.h>
 
-#include <Drivers/PL011Uart.h>
-
 /** Initialise the serial device hardware with default settings.
 
   @retval RETURN_SUCCESS            The serial device was initialised.
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h b/ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h
new file mode 100644
index 000000000000..0ea3855cf3eb
--- /dev/null
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h
@@ -0,0 +1,120 @@
+/** @file
+*
+*  Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD License
+*  which accompanies this distribution.  The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#ifndef __PL011_UART_H__
+#define __PL011_UART_H__
+
+#define PL011_VARIANT_ZTE 1
+
+// PL011 Registers
+#if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE
+#define UARTDR                    0x004
+#define UARTRSR                   0x010
+#define UARTECR                   0x010
+#define UARTFR                    0x014
+#define UARTIBRD                  0x024
+#define UARTFBRD                  0x028
+#define UARTLCR_H                 0x030
+#define UARTCR                    0x034
+#define UARTIFLS                  0x038
+#define UARTIMSC                  0x040
+#define UARTRIS                   0x044
+#define UARTMIS                   0x048
+#define UARTICR                   0x04c
+#define UARTDMACR                 0x050
+#else
+#define UARTDR                    0x000
+#define UARTRSR                   0x004
+#define UARTECR                   0x004
+#define UARTFR                    0x018
+#define UARTILPR                  0x020
+#define UARTIBRD                  0x024
+#define UARTFBRD                  0x028
+#define UARTLCR_H                 0x02C
+#define UARTCR                    0x030
+#define UARTIFLS                  0x034
+#define UARTIMSC                  0x038
+#define UARTRIS                   0x03C
+#define UARTMIS                   0x040
+#define UARTICR                   0x044
+#define UARTDMACR                 0x048
+#endif
+
+#define UARTPID0                  0xFE0
+#define UARTPID1                  0xFE4
+#define UARTPID2                  0xFE8
+#define UARTPID3                  0xFEC
+
+// Data status bits
+#define UART_DATA_ERROR_MASK      0x0F00
+
+// Status reg bits
+#define UART_STATUS_ERROR_MASK    0x0F
+
+// Flag reg bits
+#if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE
+#define PL011_UARTFR_RI           (1 << 0)  // Ring indicator
+#define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty
+#define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full
+#define PL011_UARTFR_TXFF         (1 << 5)  // Transmit FIFO full
+#define PL011_UARTFR_RXFE         (1 << 4)  // Receive  FIFO empty
+#define PL011_UARTFR_BUSY         (1 << 8)  // UART busy
+#define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect
+#define PL011_UARTFR_DSR          (1 << 3)  // Data set ready
+#define PL011_UARTFR_CTS          (1 << 1)  // Clear to send
+#else
+#define PL011_UARTFR_RI           (1 << 8)  // Ring indicator
+#define PL011_UARTFR_TXFE         (1 << 7)  // Transmit FIFO empty
+#define PL011_UARTFR_RXFF         (1 << 6)  // Receive  FIFO full
+#define PL011_UARTFR_TXFF         (1 << 5)  // Transmit FIFO full
+#define PL011_UARTFR_RXFE         (1 << 4)  // Receive  FIFO empty
+#define PL011_UARTFR_BUSY         (1 << 3)  // UART busy
+#define PL011_UARTFR_DCD          (1 << 2)  // Data carrier detect
+#define PL011_UARTFR_DSR          (1 << 1)  // Data set ready
+#define PL011_UARTFR_CTS          (1 << 0)  // Clear to send
+#endif
+
+// Flag reg bits - alternative names
+#define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE
+#define UART_RX_FULL_FLAG_MASK    PL011_UARTFR_RXFF
+#define UART_TX_FULL_FLAG_MASK    PL011_UARTFR_TXFF
+#define UART_RX_EMPTY_FLAG_MASK   PL011_UARTFR_RXFE
+#define UART_BUSY_FLAG_MASK       PL011_UARTFR_BUSY
+
+// Control reg bits
+#define PL011_UARTCR_CTSEN        (1 << 15) // CTS hardware flow control enable
+#define PL011_UARTCR_RTSEN        (1 << 14) // RTS hardware flow control enable
+#define PL011_UARTCR_RTS          (1 << 11) // Request to send
+#define PL011_UARTCR_DTR          (1 << 10) // Data transmit ready.
+#define PL011_UARTCR_RXE          (1 << 9)  // Receive enable
+#define PL011_UARTCR_TXE          (1 << 8)  // Transmit enable
+#define PL011_UARTCR_LBE          (1 << 7)  // Loopback enable
+#define PL011_UARTCR_UARTEN       (1 << 0)  // UART Enable
+
+// Line Control Register Bits
+#define PL011_UARTLCR_H_SPS       (1 << 7)  // Stick parity select
+#define PL011_UARTLCR_H_WLEN_8    (3 << 5)
+#define PL011_UARTLCR_H_WLEN_7    (2 << 5)
+#define PL011_UARTLCR_H_WLEN_6    (1 << 5)
+#define PL011_UARTLCR_H_WLEN_5    (0 << 5)
+#define PL011_UARTLCR_H_FEN       (1 << 4)  // FIFOs Enable
+#define PL011_UARTLCR_H_STP2      (1 << 3)  // Two stop bits select
+#define PL011_UARTLCR_H_EPS       (1 << 2)  // Even parity select
+#define PL011_UARTLCR_H_PEN       (1 << 1)  // Parity Enable
+#define PL011_UARTLCR_H_BRK       (1 << 0)  // Send break
+
+#define PL011_UARTPID2_VER(X)     (((X) >> 4) & 0xF)
+#define PL011_VER_R1P4            0x2
+
+#endif
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
new file mode 100644
index 000000000000..61a34fda0b83
--- /dev/null
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -0,0 +1,474 @@
+/** @file
+  Serial I/O Port library functions with no library constructor/destructor
+
+  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+  Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Uefi.h>
+
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+
+#include <Protocol/SerialIo.h>
+
+#include "PL011Uart.h"
+
+#define FRACTION_PART_SIZE_IN_BITS  6
+#define FRACTION_PART_MASK          ((1 << FRACTION_PART_SIZE_IN_BITS) - 1)
+
+//
+// EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE is the only
+// control bit that is not supported.
+//
+STATIC CONST UINT32 mInvalidControlBits = EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE;
+
+/**
+
+  Initialise the serial port to the specified settings.
+  The serial port is re-configured only if the specified settings
+  are different from the current settings.
+  All unspecified settings will be set to the default values.
+
+  @param  UartBase                The base address of the serial device.
+  @param  UartClkInHz             The clock in Hz for the serial device.
+                                  Ignored if the PCD PL011UartInteger is not 0
+  @param  BaudRate                The baud rate of the serial device. If the
+                                  baud rate is not supported, the speed will be
+                                  reduced to the nearest supported one and the
+                                  variable's value will be updated accordingly.
+  @param  ReceiveFifoDepth        The number of characters the device will
+                                  buffer on input.  Value of 0 will use the
+                                  device's default FIFO depth.
+  @param  Parity                  If applicable, this is the EFI_PARITY_TYPE
+                                  that is computed or checked as each character
+                                  is transmitted or received. If the device
+                                  does not support parity, the value is the
+                                  default parity value.
+  @param  DataBits                The number of data bits in each character.
+  @param  StopBits                If applicable, the EFI_STOP_BITS_TYPE number
+                                  of stop bits per character.
+                                  If the device does not support stop bits, the
+                                  value is the default stop bit value.
+
+  @retval RETURN_SUCCESS            All attributes were set correctly on the
+                                    serial device.
+  @retval RETURN_INVALID_PARAMETER  One or more of the attributes has an
+                                    unsupported value.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartInitializePort (
+  IN     UINTN               UartBase,
+  IN     UINT32              UartClkInHz,
+  IN OUT UINT64              *BaudRate,
+  IN OUT UINT32              *ReceiveFifoDepth,
+  IN OUT EFI_PARITY_TYPE     *Parity,
+  IN OUT UINT8               *DataBits,
+  IN OUT EFI_STOP_BITS_TYPE  *StopBits
+  )
+{
+  UINT32      LineControl;
+  UINT32      Divisor;
+  UINT32      Integer;
+  UINT32      Fractional;
+  UINT32      HardwareFifoDepth;
+
+  HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \
+                       > PL011_VER_R1P4) \
+                      ? 32 : 16 ;
+  // The PL011 supports a buffer of 1, 16 or 32 chars. Therefore we can accept
+  // 1 char buffer as the minimum FIFO size. Because everything can be rounded
+  // down, there is no maximum FIFO size.
+  if ((*ReceiveFifoDepth == 0) || (*ReceiveFifoDepth >= HardwareFifoDepth)) {
+    // Enable FIFO
+    LineControl = PL011_UARTLCR_H_FEN;
+    *ReceiveFifoDepth = HardwareFifoDepth;
+  } else {
+    // Disable FIFO
+    LineControl = 0;
+    // Nothing else to do. 1 byte FIFO is default.
+    *ReceiveFifoDepth = 1;
+  }
+
+  //
+  // Parity
+  //
+  switch (*Parity) {
+  case DefaultParity:
+    *Parity = NoParity;
+  case NoParity:
+    // Nothing to do. Parity is disabled by default.
+    break;
+  case EvenParity:
+    LineControl |= (PL011_UARTLCR_H_PEN | PL011_UARTLCR_H_EPS);
+    break;
+  case OddParity:
+    LineControl |= PL011_UARTLCR_H_PEN;
+    break;
+  case MarkParity:
+    LineControl |= (  PL011_UARTLCR_H_PEN \
+                    | PL011_UARTLCR_H_SPS \
+                    | PL011_UARTLCR_H_EPS);
+    break;
+  case SpaceParity:
+    LineControl |= (PL011_UARTLCR_H_PEN | PL011_UARTLCR_H_SPS);
+    break;
+  default:
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Data Bits
+  //
+  switch (*DataBits) {
+  case 0:
+    *DataBits = 8;
+  case 8:
+    LineControl |= PL011_UARTLCR_H_WLEN_8;
+    break;
+  case 7:
+    LineControl |= PL011_UARTLCR_H_WLEN_7;
+    break;
+  case 6:
+    LineControl |= PL011_UARTLCR_H_WLEN_6;
+    break;
+  case 5:
+    LineControl |= PL011_UARTLCR_H_WLEN_5;
+    break;
+  default:
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Stop Bits
+  //
+  switch (*StopBits) {
+  case DefaultStopBits:
+    *StopBits = OneStopBit;
+  case OneStopBit:
+    // Nothing to do. One stop bit is enabled by default.
+    break;
+  case TwoStopBits:
+    LineControl |= PL011_UARTLCR_H_STP2;
+    break;
+  case OneFiveStopBits:
+    // Only 1 or 2 stop bits are supported
+  default:
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  // Don't send the LineControl value to the PL011 yet,
+  // wait until after the Baud Rate setting.
+  // This ensures we do not mess up the UART settings halfway through
+  // in the rare case when there is an error with the Baud Rate.
+
+  //
+  // Baud Rate
+  //
+
+  // If PL011 Integer value has been defined then always ignore the BAUD rate
+  if (FixedPcdGet32 (PL011UartInteger) != 0) {
+    Integer = FixedPcdGet32 (PL011UartInteger);
+    Fractional = FixedPcdGet32 (PL011UartFractional);
+  } else {
+    // If BAUD rate is zero then replace it with the system default value
+    if (*BaudRate == 0) {
+      *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
+      if (*BaudRate == 0) {
+        return RETURN_INVALID_PARAMETER;
+      }
+    }
+    if (0 == UartClkInHz) {
+      return RETURN_INVALID_PARAMETER;
+    }
+
+    Divisor = (UartClkInHz * 4) / *BaudRate;
+    Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
+    Fractional = Divisor & FRACTION_PART_MASK;
+  }
+
+  //
+  // If PL011 is already initialized, check the current settings
+  // and re-initialize only if the settings are different.
+  //
+  if (((MmioRead32 (UartBase + UARTCR) & PL011_UARTCR_UARTEN) != 0) &&
+       (MmioRead32 (UartBase + UARTLCR_H) == LineControl) &&
+       (MmioRead32 (UartBase + UARTIBRD) == Integer) &&
+       (MmioRead32 (UartBase + UARTFBRD) == Fractional)) {
+    // Nothing to do - already initialized with correct attributes
+    return RETURN_SUCCESS;
+  }
+
+  // Wait for the end of transmission
+  while ((MmioRead32 (UartBase + UARTFR) & PL011_UARTFR_TXFE) == 0);
+
+  // Disable UART: "The UARTLCR_H, UARTIBRD, and UARTFBRD registers must not be changed
+  // when the UART is enabled"
+  MmioWrite32 (UartBase + UARTCR, 0);
+
+  // Set Baud Rate Registers
+  MmioWrite32 (UartBase + UARTIBRD, Integer);
+  MmioWrite32 (UartBase + UARTFBRD, Fractional);
+
+  // No parity, 1 stop, no fifo, 8 data bits
+  MmioWrite32 (UartBase + UARTLCR_H, LineControl);
+
+  // Clear any pending errors
+  MmioWrite32 (UartBase + UARTECR, 0);
+
+  // Enable Tx, Rx, and UART overall
+  MmioWrite32 (UartBase + UARTCR,
+               PL011_UARTCR_RXE | PL011_UARTCR_TXE | PL011_UARTCR_UARTEN);
+
+  return RETURN_SUCCESS;
+}
+
+/**
+
+  Assert or deassert the control signals on a serial port.
+  The following control signals are set according their bit settings :
+  . Request to Send
+  . Data Terminal Ready
+
+  @param[in]  UartBase  UART registers base address
+  @param[in]  Control   The following bits are taken into account :
+                        . EFI_SERIAL_REQUEST_TO_SEND : assert/deassert the
+                          "Request To Send" control signal if this bit is
+                          equal to one/zero.
+                        . EFI_SERIAL_DATA_TERMINAL_READY : assert/deassert
+                          the "Data Terminal Ready" control signal if this
+                          bit is equal to one/zero.
+                        . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : enable/disable
+                          the hardware loopback if this bit is equal to
+                          one/zero.
+                        . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : not supported.
+                        . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : enable/
+                          disable the hardware flow control based on CTS (Clear
+                          To Send) and RTS (Ready To Send) control signals.
+
+  @retval  RETURN_SUCCESS      The new control bits were set on the device.
+  @retval  RETURN_UNSUPPORTED  The device does not support this operation.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartSetControl (
+    IN UINTN   UartBase,
+    IN UINT32  Control
+  )
+{
+  UINT32  Bits;
+
+  if (Control & (mInvalidControlBits)) {
+    return RETURN_UNSUPPORTED;
+  }
+
+  Bits = MmioRead32 (UartBase + UARTCR);
+
+  if (Control & EFI_SERIAL_REQUEST_TO_SEND) {
+    Bits |= PL011_UARTCR_RTS;
+  } else {
+    Bits &= ~PL011_UARTCR_RTS;
+  }
+
+  if (Control & EFI_SERIAL_DATA_TERMINAL_READY) {
+    Bits |= PL011_UARTCR_DTR;
+  } else {
+    Bits &= ~PL011_UARTCR_DTR;
+  }
+
+  if (Control & EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE) {
+    Bits |= PL011_UARTCR_LBE;
+  } else {
+    Bits &= ~PL011_UARTCR_LBE;
+  }
+
+  if (Control & EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE) {
+    Bits |= (PL011_UARTCR_CTSEN | PL011_UARTCR_RTSEN);
+  } else {
+    Bits &= ~(PL011_UARTCR_CTSEN | PL011_UARTCR_RTSEN);
+  }
+
+  MmioWrite32 (UartBase + UARTCR, Bits);
+
+  return RETURN_SUCCESS;
+}
+
+/**
+
+  Retrieve the status of the control bits on a serial device.
+
+  @param[in]   UartBase  UART registers base address
+  @param[out]  Control   Status of the control bits on a serial device :
+
+                         . EFI_SERIAL_DATA_CLEAR_TO_SEND,
+                           EFI_SERIAL_DATA_SET_READY,
+                           EFI_SERIAL_RING_INDICATE,
+                           EFI_SERIAL_CARRIER_DETECT,
+                           EFI_SERIAL_REQUEST_TO_SEND,
+                           EFI_SERIAL_DATA_TERMINAL_READY
+                           are all related to the DTE (Data Terminal Equipment)
+                           and DCE (Data Communication Equipment) modes of
+                           operation of the serial device.
+                         . EFI_SERIAL_INPUT_BUFFER_EMPTY : equal to one if the
+                           receive buffer is empty, 0 otherwise.
+                         . EFI_SERIAL_OUTPUT_BUFFER_EMPTY : equal to one if the
+                           transmit buffer is empty, 0 otherwise.
+                         . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : equal to one if
+                           the hardware loopback is enabled (the ouput feeds the
+                           receive buffer), 0 otherwise.
+                         . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : equal to one if
+                           a loopback is accomplished by software, 0 otherwise.
+                         . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : equal to
+                           one if the hardware flow control based on CTS (Clear
+                           To Send) and RTS (Ready To Send) control signals is
+                           enabled, 0 otherwise.
+
+  @retval RETURN_SUCCESS  The control bits were read from the serial device.
+
+**/
+RETURN_STATUS
+EFIAPI
+PL011UartGetControl (
+    IN UINTN     UartBase,
+    OUT UINT32  *Control
+  )
+{
+  UINT32      FlagRegister;
+  UINT32      ControlRegister;
+
+
+  FlagRegister = MmioRead32 (UartBase + UARTFR);
+  ControlRegister = MmioRead32 (UartBase + UARTCR);
+
+  *Control = 0;
+
+  if ((FlagRegister & PL011_UARTFR_CTS) == PL011_UARTFR_CTS) {
+    *Control |= EFI_SERIAL_CLEAR_TO_SEND;
+  }
+
+  if ((FlagRegister & PL011_UARTFR_DSR) == PL011_UARTFR_DSR) {
+    *Control |= EFI_SERIAL_DATA_SET_READY;
+  }
+
+  if ((FlagRegister & PL011_UARTFR_RI) == PL011_UARTFR_RI) {
+    *Control |= EFI_SERIAL_RING_INDICATE;
+  }
+
+  if ((FlagRegister & PL011_UARTFR_DCD) == PL011_UARTFR_DCD) {
+    *Control |= EFI_SERIAL_CARRIER_DETECT;
+  }
+
+  if ((ControlRegister & PL011_UARTCR_RTS) == PL011_UARTCR_RTS) {
+    *Control |= EFI_SERIAL_REQUEST_TO_SEND;
+  }
+
+  if ((ControlRegister & PL011_UARTCR_DTR) == PL011_UARTCR_DTR) {
+    *Control |= EFI_SERIAL_DATA_TERMINAL_READY;
+  }
+
+  if ((FlagRegister & PL011_UARTFR_RXFE) == PL011_UARTFR_RXFE) {
+    *Control |= EFI_SERIAL_INPUT_BUFFER_EMPTY;
+  }
+
+  if ((FlagRegister & PL011_UARTFR_TXFE) == PL011_UARTFR_TXFE) {
+    *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY;
+  }
+
+  if ((ControlRegister & (PL011_UARTCR_CTSEN | PL011_UARTCR_RTSEN))
+       == (PL011_UARTCR_CTSEN | PL011_UARTCR_RTSEN)) {
+    *Control |= EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE;
+  }
+
+  if ((ControlRegister & PL011_UARTCR_LBE) == PL011_UARTCR_LBE) {
+    *Control |= EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE;
+  }
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Write data to serial device.
+
+  @param  Buffer           Point of data buffer which need to be written.
+  @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
+
+  @retval 0                Write data failed.
+  @retval !0               Actual number of bytes written to serial device.
+
+**/
+UINTN
+EFIAPI
+PL011UartWrite (
+  IN  UINTN    UartBase,
+  IN UINT8     *Buffer,
+  IN UINTN     NumberOfBytes
+  )
+{
+  UINT8* CONST Final = &Buffer[NumberOfBytes];
+
+  while (Buffer < Final) {
+    // Wait until UART able to accept another char
+    while ((MmioRead32 (UartBase + UARTFR) & UART_TX_FULL_FLAG_MASK));
+
+    MmioWrite8 (UartBase + UARTDR, *Buffer++);
+  }
+
+  return NumberOfBytes;
+}
+
+/**
+  Read data from serial device and save the data in buffer.
+
+  @param  Buffer           Point of data buffer which need to be written.
+  @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
+
+  @retval 0                Read data failed.
+  @retval !0               Actual number of bytes read from serial device.
+
+**/
+UINTN
+EFIAPI
+PL011UartRead (
+  IN  UINTN     UartBase,
+  OUT UINT8     *Buffer,
+  IN  UINTN     NumberOfBytes
+  )
+{
+  UINTN   Count;
+
+  for (Count = 0; Count < NumberOfBytes; Count++, Buffer++) {
+    while ((MmioRead32 (UartBase + UARTFR) & UART_RX_EMPTY_FLAG_MASK) != 0);
+    *Buffer = MmioRead8 (UartBase + UARTDR);
+  }
+
+  return NumberOfBytes;
+}
+
+/**
+  Check to see if any data is available to be read from the debug device.
+
+  @retval TRUE       At least one byte of data is available to be read
+  @retval FALSE      No data is available to be read
+
+**/
+BOOLEAN
+EFIAPI
+PL011UartPoll (
+  IN  UINTN     UartBase
+  )
+{
+  return ((MmioRead32 (UartBase + UARTFR) & UART_RX_EMPTY_FLAG_MASK) == 0);
+}
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
new file mode 100644
index 000000000000..4fc974494a33
--- /dev/null
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
@@ -0,0 +1,43 @@
+#/** @file
+#
+#  Component description file for PL011Uart module
+#
+#  Copyright (c) 2011-2016, ARM Ltd. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = PL011UartLib
+  FILE_GUID                      = 6a2c5714-8910-44f0-861f-804abc18ce39
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = PL011UartLib
+
+[Sources.common]
+  PL011Uart.h
+  PL011UartLib.c
+
+[LibraryClasses]
+  DebugLib
+  IoLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate
+
+  gArmPlatformTokenSpaceGuid.PL011UartInteger
+  gArmPlatformTokenSpaceGuid.PL011UartFractional
+  gArmPlatformTokenSpaceGuid.PL011UartRegOffsetVariant
-- 
2.11.0



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

* [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation
  2017-11-16 17:47 [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Ard Biesheuvel
  2017-11-16 17:47 ` [PATCH v2 1/2] ArmPlatformPkg: reorganize PL011 code Ard Biesheuvel
@ 2017-11-16 17:47 ` Ard Biesheuvel
  2017-11-16 22:17   ` Laszlo Ersek
  2017-11-16 18:41 ` [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Leif Lindholm
  2 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2017-11-16 17:47 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm; +Cc: lersek, Ard Biesheuvel

Switch to the new, cleaned up PL011UartLib implementation so we will
be able to remove the old one.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc                                            | 2 +-
 ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c | 5 ++---
 ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      | 5 ++---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index c92a69281ae4..50eb8675d1c0 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -106,7 +106,7 @@ [LibraryClasses.common]
   RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
   # ARM PL011 UART Driver
-  PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
+  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf
 
   #
diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
index e28750f3b4c4..d9fd0ef98359 100644
--- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
+++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
@@ -16,14 +16,13 @@
 
 **/
 
-#include <Base.h>
+#include <Uefi.h>
 
 #include <Library/PcdLib.h>
+#include <Library/PL011UartLib.h>
 #include <Library/SerialPortLib.h>
 #include <libfdt.h>
 
-#include <Drivers/PL011Uart.h>
-
 RETURN_STATUS
 EFIAPI
 SerialPortInitialize (
diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
index 05d3547fda91..c161dd6349d3 100644
--- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
+++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
@@ -17,9 +17,10 @@
 
 **/
 
-#include <Base.h>
+#include <Uefi.h>
 
 #include <Library/PcdLib.h>
+#include <Library/PL011UartLib.h>
 #include <Library/SerialPortLib.h>
 #include <Pi/PiBootMode.h>
 #include <Uefi/UefiBaseType.h>
@@ -28,8 +29,6 @@
 #include <Library/HobLib.h>
 #include <Guid/EarlyPL011BaseAddress.h>
 
-#include <Drivers/PL011Uart.h>
-
 STATIC UINTN mSerialBaseAddress;
 
 RETURN_STATUS
-- 
2.11.0



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

* Re: [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library
  2017-11-16 17:47 [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Ard Biesheuvel
  2017-11-16 17:47 ` [PATCH v2 1/2] ArmPlatformPkg: reorganize PL011 code Ard Biesheuvel
  2017-11-16 17:47 ` [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation Ard Biesheuvel
@ 2017-11-16 18:41 ` Leif Lindholm
  2017-11-17 10:06   ` Ard Biesheuvel
  2 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2017-11-16 18:41 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, lersek

On Thu, Nov 16, 2017 at 05:47:06PM +0000, Ard Biesheuvel wrote:
> Clean up the PL011 UART support library, by moving it into the appropriate
> place for libraries, and splitting the header file into an implementation
> and an interface part.

For the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Ard Biesheuvel (2):
>   ArmPlatformPkg: reorganize PL011 code
>   ArmVirtPkg: switch to new PL011UartLib implementation
> 
>  ArmPlatformPkg/ArmPlatformPkg.dec                                     |   3 +
>  ArmPlatformPkg/Include/Library/PL011UartLib.h                         | 187 ++++++++
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c        |   5 +-
>  ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h                       | 120 +++++
>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c                    | 474 ++++++++++++++++++++
>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf                  |  43 ++
>  ArmVirtPkg/ArmVirt.dsc.inc                                            |   2 +-
>  ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c |   5 +-
>  ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      |   5 +-
>  9 files changed, 834 insertions(+), 10 deletions(-)
>  create mode 100644 ArmPlatformPkg/Include/Library/PL011UartLib.h
>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h
>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> 
> -- 
> 2.11.0
> 


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

* Re: [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation
  2017-11-16 17:47 ` [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation Ard Biesheuvel
@ 2017-11-16 22:17   ` Laszlo Ersek
  2017-11-17 10:01     ` Ard Biesheuvel
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2017-11-16 22:17 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, leif.lindholm

On 11/16/17 18:47, Ard Biesheuvel wrote:
> Switch to the new, cleaned up PL011UartLib implementation so we will
> be able to remove the old one.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc                                            | 2 +-
>  ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c | 5 ++---
>  ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      | 5 ++---
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index c92a69281ae4..50eb8675d1c0 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -106,7 +106,7 @@ [LibraryClasses.common]
>    RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>    # ARM PL011 UART Driver
> -  PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
> +  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>    SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf
>  
>    #
> diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
> index e28750f3b4c4..d9fd0ef98359 100644
> --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
> +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
> @@ -16,14 +16,13 @@
>  
>  **/
>  
> -#include <Base.h>
> +#include <Uefi.h>
>  
>  #include <Library/PcdLib.h>
> +#include <Library/PL011UartLib.h>
>  #include <Library/SerialPortLib.h>
>  #include <libfdt.h>
>  
> -#include <Drivers/PL011Uart.h>
> -
>  RETURN_STATUS
>  EFIAPI
>  SerialPortInitialize (
> diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
> index 05d3547fda91..c161dd6349d3 100644
> --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
> +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
> @@ -17,9 +17,10 @@
>  
>  **/
>  
> -#include <Base.h>
> +#include <Uefi.h>
>  
>  #include <Library/PcdLib.h>
> +#include <Library/PL011UartLib.h>
>  #include <Library/SerialPortLib.h>
>  #include <Pi/PiBootMode.h>
>  #include <Uefi/UefiBaseType.h>
> @@ -28,8 +29,6 @@
>  #include <Library/HobLib.h>
>  #include <Guid/EarlyPL011BaseAddress.h>
>  
> -#include <Drivers/PL011Uart.h>
> -
>  STATIC UINTN mSerialBaseAddress;
>  
>  RETURN_STATUS
> 

Awesome idea!

One comment: can you please check whether, if you replace

#include <Uefi.h>

with

#include <Uefi/UefiBaseType.h>

then stuff will still build?

Both SerialPortLib instances are BASE, so we should not include <Uefi.h>
(in particular <Uefi/UefiSpec.h> included by it).

If it works, then:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

If it doesn't work, then I think we should figure out why not; it could
be a sign of some layering violation, which would be nice to document at
least in the commit message.

Thanks
Laszlo


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

* Re: [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation
  2017-11-16 22:17   ` Laszlo Ersek
@ 2017-11-17 10:01     ` Ard Biesheuvel
  0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-11-17 10:01 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Leif Lindholm

On 16 November 2017 at 22:17, Laszlo Ersek <lersek@redhat.com> wrote:
> On 11/16/17 18:47, Ard Biesheuvel wrote:
>> Switch to the new, cleaned up PL011UartLib implementation so we will
>> be able to remove the old one.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc                                            | 2 +-
>>  ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c | 5 ++---
>>  ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      | 5 ++---
>>  3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index c92a69281ae4..50eb8675d1c0 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -106,7 +106,7 @@ [LibraryClasses.common]
>>    RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
>>    TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>>    # ARM PL011 UART Driver
>> -  PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
>> +  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>    SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf
>>
>>    #
>> diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
>> index e28750f3b4c4..d9fd0ef98359 100644
>> --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
>> +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c
>> @@ -16,14 +16,13 @@
>>
>>  **/
>>
>> -#include <Base.h>
>> +#include <Uefi.h>
>>
>>  #include <Library/PcdLib.h>
>> +#include <Library/PL011UartLib.h>
>>  #include <Library/SerialPortLib.h>
>>  #include <libfdt.h>
>>
>> -#include <Drivers/PL011Uart.h>
>> -
>>  RETURN_STATUS
>>  EFIAPI
>>  SerialPortInitialize (
>> diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
>> index 05d3547fda91..c161dd6349d3 100644
>> --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
>> +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
>> @@ -17,9 +17,10 @@
>>
>>  **/
>>
>> -#include <Base.h>
>> +#include <Uefi.h>
>>
>>  #include <Library/PcdLib.h>
>> +#include <Library/PL011UartLib.h>
>>  #include <Library/SerialPortLib.h>
>>  #include <Pi/PiBootMode.h>
>>  #include <Uefi/UefiBaseType.h>
>> @@ -28,8 +29,6 @@
>>  #include <Library/HobLib.h>
>>  #include <Guid/EarlyPL011BaseAddress.h>
>>
>> -#include <Drivers/PL011Uart.h>
>> -
>>  STATIC UINTN mSerialBaseAddress;
>>
>>  RETURN_STATUS
>>
>
> Awesome idea!
>
> One comment: can you please check whether, if you replace
>
> #include <Uefi.h>
>
> with
>
> #include <Uefi/UefiBaseType.h>
>
> then stuff will still build?
>
> Both SerialPortLib instances are BASE, so we should not include <Uefi.h>
> (in particular <Uefi/UefiSpec.h> included by it).
>
> If it works, then:
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> If it doesn't work, then I think we should figure out why not; it could
> be a sign of some layering violation, which would be nice to document at
> least in the commit message.
>

Yes, that works. I added #include <Uefi/UefiBaseType.h> to
Library/PL011UartLib.h, which includes Protocol/SerialIo.h and
therefore needs the UEFI types. With that, I can drop the change from
the SerialPortLib implementations, in both patches.


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

* Re: [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library
  2017-11-16 18:41 ` [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Leif Lindholm
@ 2017-11-17 10:06   ` Ard Biesheuvel
  0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-11-17 10:06 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Laszlo Ersek

On 16 November 2017 at 18:41, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Nov 16, 2017 at 05:47:06PM +0000, Ard Biesheuvel wrote:
>> Clean up the PL011 UART support library, by moving it into the appropriate
>> place for libraries, and splitting the header file into an implementation
>> and an interface part.
>
> For the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Pushed as

12156134fe82 ArmPlatformPkg: reorganize PL011 code
1195b8578764 ArmVirtPkg: switch to new PL011UartLib implementation

(with Laszlo's feedback incorporated)


>> Ard Biesheuvel (2):
>>   ArmPlatformPkg: reorganize PL011 code
>>   ArmVirtPkg: switch to new PL011UartLib implementation
>>
>>  ArmPlatformPkg/ArmPlatformPkg.dec                                     |   3 +
>>  ArmPlatformPkg/Include/Library/PL011UartLib.h                         | 187 ++++++++
>>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c        |   5 +-
>>  ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h                       | 120 +++++
>>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c                    | 474 ++++++++++++++++++++
>>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf                  |  43 ++
>>  ArmVirtPkg/ArmVirt.dsc.inc                                            |   2 +-
>>  ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c |   5 +-
>>  ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c      |   5 +-
>>  9 files changed, 834 insertions(+), 10 deletions(-)
>>  create mode 100644 ArmPlatformPkg/Include/Library/PL011UartLib.h
>>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011Uart.h
>>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
>>  create mode 100644 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>
>> --
>> 2.11.0
>>


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

end of thread, other threads:[~2017-11-17 10:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-16 17:47 [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Ard Biesheuvel
2017-11-16 17:47 ` [PATCH v2 1/2] ArmPlatformPkg: reorganize PL011 code Ard Biesheuvel
2017-11-16 17:47 ` [PATCH v2 2/2] ArmVirtPkg: switch to new PL011UartLib implementation Ard Biesheuvel
2017-11-16 22:17   ` Laszlo Ersek
2017-11-17 10:01     ` Ard Biesheuvel
2017-11-16 18:41 ` [PATCH v2 0/2] ArmplatformPkg: clean up PL011 support library Leif Lindholm
2017-11-17 10:06   ` Ard Biesheuvel

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