[edk2-devel] [edk2-platforms][PATCH 4/6] Platform/RPi4: Add switch to select between PL011 and miniUART
Pete Batard
pete at akeo.ie
Wed Dec 18 16:59:41 UTC 2019
On 2019.12.18 16:05, Philippe Mathieu-Daudé wrote:
> 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 at 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)
That's not a bad suggestion but if we do that then I'd rather introduce
2 new BCM2836_MINI_UART_OFFSET and BCM2836_PL011_UART_OFFSET constants
in Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h and then
set BCM283X_UART_BASE_ADDRESS to PcdBcm283xRegistersAddress + one or the
other.
The whole point was to avoid adding offsets, which may or may not remain
constant over the evolution of the Bcm283x family and derivatives, into
individual sources, because then we way have to hunt these constants in
places we may not even remember they exist.
On the other hand, I wouldn't mind avoiding that confusing 0x40 offset
we need to substract when using the 8250 UART. So I think I'll go with
the BCM283X_UART_BASE_ADDRESS + offset route, after adding an extra
patch to the series that adds the UART offset constants to
Silicon/.../Bcm2836.h.
Regards,
/Pete
>
>
>> + 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
> [...]
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52366): https://edk2.groups.io/g/devel/message/52366
Mute This Topic: https://groups.io/mt/68791814/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list