Re: [edk2-platforms][PATCH 4/6] Platform/RPi4: Add switch to select between PL011 and miniUART

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 12/18/19 12:41 PM, Pete Batard wrote:
> The PL011 can be a better choice for the serial console on the RPi4,
> given that its baud clock is not derived from the CPU clock (which
> may change under our feet unless we keep it fixed at a low rate), and
> given the fact that the SBSA/SBBR specs that describe ARM specific
> architectural requirements for ACPI only permit PL011 based UARTs to
> begin with.
> 
> Therefore we add a new PL011_ENABLE build switch to tell the firmware
> to use PL011 for all console serial I/O, including for TF-A, SPCR and
> DBG2, as well as toggle the BlueTooth module to use the mini UART.
> 
> For the time being, the option is disabled by default because it
> requires an overlay to be enabled in config.txt that pinmuxes the
> PL011 TX/RX lines to the UART pins on the connector block.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
[...]
> index 65f48ceae688..e5c10c923626 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
> @@ -18,12 +18,19 @@
>   
>   #define RPI_UART_FLOW_CONTROL_NONE           0
>   
> +#ifdef PL011_ENABLE
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> +#define RPI_UART_BASE_ADDRESS                FixedPcdGet64 (PcdSerialRegisterBase)

See comment below.

> +#define RPI_UART_INTERRUPT                   0x99
> +#else
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
>   //
>   // When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
>   // which is offset by 0x40 from the actual Bcm2835 base address
>   //
>   #define RPI_UART_BASE_ADDRESS                (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
>   #define RPI_UART_INTERRUPT                   0x7D
> +#endif
>   
>   STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>     ACPI_HEADER (
> @@ -32,7 +39,7 @@ STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>       EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
>     ),
>     // UINT8                                   InterfaceType;
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
> +  RPI_UART_INTERFACE_TYPE,
>     // UINT8                                   Reserved1[3];
>     {
>       EFI_ACPI_RESERVED_BYTE,
> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> index 15149892f3b0..5b59f2dd3e16 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> @@ -108,7 +108,7 @@ Device(BTH0)
>     {
>       Name (RBUF, ResourceTemplate ()
>       {
> -      // BT UART: UART0 (PL011)
> +      // BT UART: URT0 (PL011) or URTM (miniUART)
>         UARTSerialBus(
>           115200,        // InitialBaudRate: in BPS
>           ,              // BitsPerByte: default to 8 bits
> @@ -133,7 +133,11 @@ Device(BTH0)
>                          //   no flow control.
>           16,            // ReceiveBufferSize
>           16,            // TransmitBufferSize
> +#ifdef PL011_ENABLE
> +        "\\_SB.URTM",  // ResourceSource:
> +#else
>           "\\_SB.URT0",  // ResourceSource:
> +#endif
>                          //   UART bus controller name
>           ,              // ResourceSourceIndex: assumed to be 0
>           ,              // ResourceUsage: assumed to be
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 1624ebda27d7..ccf5bd5b9ef3 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -38,6 +38,7 @@ [Defines]
>     DEFINE SECURE_BOOT_ENABLE      = FALSE
>     DEFINE INCLUDE_TFTP_COMMAND    = FALSE
>     DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
> +  DEFINE PL011_ENABLE            = FALSE
>   
>   ################################################################################
>   #
> @@ -116,10 +117,16 @@ [LibraryClasses.common]
>     ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
>     ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>   
> +!if $(PL011_ENABLE) == TRUE
> +  PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
> +  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> +  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +!else
>     PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
>     PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>     PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
>     SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +!endif
>   
>     # Cryptographic libraries
>     IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> @@ -229,6 +236,12 @@ [BuildOptions]
>     GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
>     GCC:RELEASE_*_*_CC_FLAGS    = -DMDEPKG_NDEBUG -DNDEBUG
>   
> +!if $(PL011_ENABLE) == TRUE
> +  GCC:*_*_*_CC_FLAGS          = -DPL011_ENABLE
> +  GCC:*_*_*_ASLPP_FLAGS       = -DPL011_ENABLE
> +  GCC:*_*_*_ASLCC_FLAGS       = -DPL011_ENABLE
> +!endif
> +
>   [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>     GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>   
> @@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common]
>     gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
>     gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
>   
> +!if $(PL011_ENABLE) == TRUE
> +  ## PL011 UART
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000

Can we use relative to gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress 
instead?

And instead of RPI_UART_BASE_ADDRESS():

#define BCM283X_UART_OFFSET          0x00201000
#define BCM283X_UART_BASE_ADDRESS   (FixedPcdGet64 
(PcdBcm283xRegistersAddress) \

                                     + BCM283X_UART_OFFSET)


> +  gArmPlatformTokenSpaceGuid.PL011UartInteger|0
> +  gArmPlatformTokenSpaceGuid.PL011UartFractional|0
> +  gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
> +!else
>     ## NS16550 compatible UART
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe215040
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> @@ -398,7 +418,10 @@ [PcdsFixedAtBuild.common]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
> +!endif
> +
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>   
[...]