From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web09.13107.1576688740072266126 for ; Wed, 18 Dec 2019 09:05:40 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Wz+gGpXP; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1576688739; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HrC4ILdgCMRzY/Mik7Q8ibk2eweebSR0y/OooIoEBMc=; b=Wz+gGpXPxT7ejfSNpJvoCFmrgOLNUWxBZ9ZeYEHONRqV3y5QACeAof794krmyUl94sfVYF 9AE/Mjc9qli1BlMGvAhLkCNKRAgSTUpuqFVM+KqUP02WtkU9ERh3nxaAIGmEy/k3riG4Fk 7sIRotS5P2k160Eb/ZWtAtanDxV1/eo= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-16-ebN6Pt_kMUCV_WD3eu0kpg-1; Wed, 18 Dec 2019 12:05:35 -0500 Received: by mail-wm1-f72.google.com with SMTP id m133so713578wmf.2 for ; Wed, 18 Dec 2019 09:05:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=GwSXEFYYRLbD38cvQkfXO1B0VrqTR0DqRJOTfaHBAwk=; b=JMXmsxYNZeX8KAvzjWOiv1KnjzOguia8OErOUMJqxQPRow6VGktxGeOZWwob1aygC5 Zc0B7brwQRwXib+tE8yWci1MIeV+UF0i/VbXmQ6XNuz3SRxEYiQKaOwPrc32rs0XY9mb Gy9LnPJTqipzC3wTla+U2L40059CWMB4xXR0S4xMmZSsG3GQCllxTYIcxCGANm4DiBqA i/9ibsVU6Nq0VLgBCngV7ywjOeb1/OPhTPxZAD77lc+2sRe6azuOpe0vbFsC8oMITFW9 qPxvs1O1iDndAe8mH7qgg7edqAkgGXXGhaE+O22LWTndl03JnmGxyIGJxNJKJcAjShAq Cirw== X-Gm-Message-State: APjAAAW9vPllapi/oYtsQQ2y8yk0jNkW9SF8Bq4FAO6zVin9F0960EMb hR+TBHrWnNDYgGkztSYEDbrrtOM0pYbjJBHiw6B6qit8pOfs1aIGrYvnK/JVngbjYNblt7V5UHU yTy0QladQUACASw== X-Received: by 2002:a7b:c934:: with SMTP id h20mr4363727wml.103.1576688734263; Wed, 18 Dec 2019 09:05:34 -0800 (PST) X-Google-Smtp-Source: APXvYqy6j04HheUrg5QetEhBrCDUhtV9DLZrbvi9xDI/IW9Q/IAUVv9N7u075XrKKiXdTHHS5YEbsw== X-Received: by 2002:a7b:c934:: with SMTP id h20mr4363689wml.103.1576688733940; Wed, 18 Dec 2019 09:05:33 -0800 (PST) Return-Path: Received: from [192.168.1.35] (34.red-83-42-66.dynamicip.rima-tde.net. [83.42.66.34]) by smtp.gmail.com with ESMTPSA id n14sm3005693wmi.26.2019.12.18.09.05.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 18 Dec 2019 09:05:33 -0800 (PST) Subject: Re: [edk2-devel] [edk2-platforms][PATCH 4/6] Platform/RPi4: Add switch to select between PL011 and miniUART To: Pete Batard , devel@edk2.groups.io Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org References: <20191218114156.9036-1-pete@akeo.ie> <20191218114156.9036-5-pete@akeo.ie> <682dfe70-95f4-642f-953f-db20a659c8aa@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: <42a8949b-d554-ea4d-ff31-26ec946741aa@redhat.com> Date: Wed, 18 Dec 2019 18:05:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: X-MC-Unique: ebN6Pt_kMUCV_WD3eu0kpg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable On 12/18/19 5:59 PM, Pete Batard wrote: > On 2019.12.18 16:05, Philippe Mathieu-Daud=C3=A9 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 >>> --- >> [...] >>> index 65f48ceae688..e5c10c923626 100644 >>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc >>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc >>> @@ -18,12 +18,19 @@ >>> =C2=A0 #define RPI_UART_FLOW_CONTROL_NONE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0 >>> +#ifdef PL011_ENABLE >>> +#define RPI_UART_INTERFACE_TYPE=20 >>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011= _UART=20 >>> >>> +#define RPI_UART_BASE_ADDRESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 FixedPcdGet64=20 >>> (PcdSerialRegisterBase) >> >> See comment below. >> >>> +#define RPI_UART_INTERRUPT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0x99 >>> +#else >>> +#define RPI_UART_INTERFACE_TYPE=20 >>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_U= ART=20 >>> >>> =C2=A0 // >>> =C2=A0 // When using the miniUART, PcdSerialRegisterBase points to the= =20 >>> 8250 base address, >>> =C2=A0 // which is offset by 0x40 from the actual Bcm2835 base address >>> =C2=A0 // >>> =C2=A0 #define RPI_UART_BASE_ADDRESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (FixedPcdGet64=20 >>> (PcdSerialRegisterBase) - 0x40) >>> =C2=A0 #define RPI_UART_INTERRUPT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0x= 7D >>> +#endif >>> =C2=A0 STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr =3D { >>> =C2=A0=C2=A0=C2=A0 ACPI_HEADER ( >>> @@ -32,7 +39,7 @@ STATIC=20 >>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr =3D { >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION= _TABLE_REVISION >>> =C2=A0=C2=A0=C2=A0 ), >>> =C2=A0=C2=A0=C2=A0 // UINT8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 InterfaceType; >>> -=20 >>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_U= ART,=20 >>> >>> +=C2=A0 RPI_UART_INTERFACE_TYPE, >>> =C2=A0=C2=A0=C2=A0 // UINT8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 Reserved1[3]; >>> =C2=A0=C2=A0=C2=A0 { >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EFI_ACPI_RESERVED_BYTE, >>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl=20 >>> 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) >>> =C2=A0=C2=A0=C2=A0 { >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (RBUF, ResourceTemplate () >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 { >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // BT UART: UART0 (PL011) >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // BT UART: URT0 (PL011) or URTM (miniU= ART) >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 UARTSerialBus( >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 115200,=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // InitialBaudRate: in BPS >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ,=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // BitsPerB= yte: default to 8 bits >>> @@ -133,7 +133,11 @@ Device(BTH0) >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= //=C2=A0=C2=A0 no flow control. >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 16,=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // ReceiveBufferSize >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 16,=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // TransmitBufferSiz= e >>> +#ifdef PL011_ENABLE >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "\\_SB.URTM",=C2=A0 // Reso= urceSource: >>> +#else >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "\\_SB.URT0",=C2= =A0 // ResourceSource: >>> +#endif >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= //=C2=A0=C2=A0 UART bus controller name >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ,=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // Resource= SourceIndex: assumed to be 0 >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ,=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // Resource= Usage: assumed to be >>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc=20 >>> 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] >>> =C2=A0=C2=A0=C2=A0 DEFINE SECURE_BOOT_ENABLE=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 =3D FALSE >>> =C2=A0=C2=A0=C2=A0 DEFINE INCLUDE_TFTP_COMMAND=C2=A0=C2=A0=C2=A0 =3D FA= LSE >>> =C2=A0=C2=A0=C2=A0 DEFINE DEBUG_PRINT_ERROR_LEVEL =3D 0x8000004F >>> +=C2=A0 DEFINE PL011_ENABLE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 =3D FALSE >>> #######################################################################= #########=20 >>> >>> =C2=A0 # >>> @@ -116,10 +117,16 @@ [LibraryClasses.common] >>> =C2=A0=C2=A0=C2=A0 ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf >>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/A= rmGenericTimerPhyCounterLib.inf=20 >>> >>> +!if $(PL011_ENABLE) =3D=3D TRUE >>> +=20 >>> PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClo= ckLib.inf=20 >>> >>> +=C2=A0 PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.i= nf >>> +=20 >>> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPort= Lib.inf=20 >>> >>> +!else >>> =C2=A0=C2=A0=C2=A0 PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib= .inf >>> =C2=A0=C2=A0=C2=A0 PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.in= f >>> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatfo= rmHookLibNull.inf=20 >>> >>> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPor= tLib16550.inf=20 >>> >>> +!endif >>> =C2=A0=C2=A0=C2=A0 # Cryptographic libraries >>> =C2=A0=C2=A0=C2=A0 IntrinsicLib|CryptoPkg/Library/IntrinsicLib/Intrinsi= cLib.inf >>> @@ -229,6 +236,12 @@ [BuildOptions] >>> =C2=A0=C2=A0=C2=A0 GCC:*_*_AARCH64_DLINK_FLAGS =3D -Wl,--fix-cortex-a53= -843419 >>> =C2=A0=C2=A0=C2=A0 GCC:RELEASE_*_*_CC_FLAGS=C2=A0=C2=A0=C2=A0 =3D -DMDE= PKG_NDEBUG -DNDEBUG >>> +!if $(PL011_ENABLE) =3D=3D TRUE >>> +=C2=A0 GCC:*_*_*_CC_FLAGS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 =3D -DPL011_ENABLE >>> +=C2=A0 GCC:*_*_*_ASLPP_FLAGS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D -= DPL011_ENABLE >>> +=C2=A0 GCC:*_*_*_ASLCC_FLAGS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D -= DPL011_ENABLE >>> +!endif >>> + >>> =C2=A0 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] >>> =C2=A0=C2=A0=C2=A0 GCC:*_*_AARCH64_DLINK_FLAGS =3D -z common-page-size= =3D0x10000 >>> @@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common] >>> =C2=A0=C2=A0=C2=A0 gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ff= ffff >>> =C2=A0=C2=A0=C2=A0 gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600= 000000 >>> +!if $(PL011_ENABLE) =3D=3D TRUE >>> +=C2=A0 ## PL011 UART >>> +=C2=A0 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000 >> >> Can we use relative to=20 >> gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress instead? >> >> And instead of RPI_UART_BASE_ADDRESS(): >> >> #define BCM283X_UART_OFFSET=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 0x00201000 >> #define BCM283X_UART_BASE_ADDRESS=C2=A0=C2=A0 (FixedPcdGet64=20 >> (PcdBcm283xRegistersAddress) \ >> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 + BCM= 283X_UART_OFFSET) >=20 > That's not a bad suggestion but if we do that then I'd rather introduce= =20 > 2 new BCM2836_MINI_UART_OFFSET and BCM2836_PL011_UART_OFFSET constants=20 > in Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h and then= =20 > set BCM283X_UART_BASE_ADDRESS to PcdBcm283xRegistersAddress + one or the= =20 > other. >=20 > The whole point was to avoid adding offsets, which may or may not remain= =20 > constant over the evolution of the Bcm283x family and derivatives, into= =20 > individual sources, because then we way have to hunt these constants in= =20 > places we may not even remember they exist. If the family evolves, we can handle the new case or copy/paste again. I insist with this modularity because then with little work we can=20 support the RPi1, using: gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x20000000 > On the other hand, I wouldn't mind avoiding that confusing 0x40 offset=20 > we need to substract when using the 8250 UART. So I think I'll go with=20 > the BCM283X_UART_BASE_ADDRESS + offset route, after adding an extra=20 > patch to the series that adds the UART offset constants to=20 > Silicon/.../Bcm2836.h. If you don't mind doing the work, I believe this is the best way. Thanks! Phil.