* [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