Re: [edk2-platforms][PATCH 0/4] Platform/RPi: Automate runtime UART selection

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

Hey Pete,

On Tue, 28 Jan 2020 at 18:20, Pete Batard <pete@akeo.ie> wrote:
>
> The Raspberry Pi platform contains two UARTs, one PL011-based and the
> other (called miniUART) 16650-compatible, that are pinmuxed to the GPIO
> serial port according to whether a Device Tree overlay is present in
> config.txt or not. In most cases, it takes only the user commenting
> or uncommenting an overlay line in their config to switch between PL011
> and miniUART.
>
> As such, the use of a build time option to select the UART should be
> avoided when it is effectively possible to detect which of the UART
> is in use at runtime, through an MMIO call.
>
> This series of patches achieves just this by:
>
> * Adding the relevant clock manager constant to Silicon (which we'll
>   need to retrieve the VPU divisor, needed to set the 16550 baudrate).
>
> * Adding a new DualSerialPortLibrary that handles runtime detection
>   and switching between PL011 and 16650.
>
> * Enabling the use of DualSerialPortLibrar for both the RPi3 and RPi4.
>
> Important notes:
>
> * This patch does not cover ACPI serial bindings, as this requires
>   switching to using DynamicTablesPkg / ConfigurationManagerDxe to
>   generate the ACPI tables at runtime. As such, each of the RPi
>   platforms is currently hardcoded to use one of the UARTs for ACPI:
>   miniUART for RPi3 and PL011 for RPi4. Of course, there is no issue
>   for Device Tree usage, since the relevant UART overlay will have
>   been applied then. We don't see the current ACPI UART hardcoding
>   as a major issue, as this doesn't change anything for RPi3 and we
>   expect RPi4 users to prefer PL011 over miniUART anyway, but we
>   will look into using DynamicTablesPkg once we see clearer in terms
>   of ACPI for RPi4.
>

I'd prefer avoiding DynamicTablesPkg if we can, especially for simple
cases like this one, where we can simply carry two SSDTs in the
firmware, and only install the one that matches the configuration we
detect at runtime.

However, it highly depends on whether OS support for the miniUART
emerges in ACPI mode, or the point is moot afaict.

> * There is work underway to produce a PL011 vs miniUART aware TF-A,
>   which we hope will be completed by the next TF-A release. Once
>   that release has occurred, we will update the TF-A blobs in
>   non-osi, since the ones we have right now are hardcoded to output
>   through one UART only.
>
> * One the subject of TF-A usage, there appears to be an issue when
>   using a 16650 (miniUART) based TF-A in a PL011 configuration as
>   the system freezes then. This issue does not occur when using a
>   PL011 based TF-A in a 16650/miniUART configuration (which is one
>   of the the reason why we picked the PL011 TF-A binary over the
>   16650 one for RPi4). What this means is that, unless you replace
>   the current RPi3 TF-A blobs from non-osi with a version that
>   outputs to PL011, and attempt to use a Raspberry Pi 3 in PL011
>   mode, then a boot freezout will happen before the UEFI firmware
>   gets a chance to apply UART runtime selection. This is an issue
>   that will of course resolve itself once we replace the current
>   TF-A  blobs with the upcoming runtime selection version. However,
>   I can also produce a patch that replaces the current 16650-based
>   RPI3 TF-A in non-osi with PL011-based ones, if we think it's
>   needed before we get the upcoming runtime selection TF-A binaries.
>
> * It appears that we are issuing serial write calls before the
>   UARTs are initialized, which is a problem if 16650 is being used
>   instead of PL011 (produces a freezout similar to what occurs when
>   using 16650 TF-A in a PL011 enabled conf). As such, we do perform
>   miniUART vs PL011 detection in both initialize and write.
>
> * The 16650 code applies the recent bugfix from Ashish Singhal in
>   https://edk2.groups.io/g/devel/message/53487.
>
> Regards,
>
> /Pete
>
> Pete Batard (4):
>   Silicon/Broadcom/Bcm283x: Add clock manager constants
>   Platform/RPi: Add serial lib for runtime PL011 vs miniUART detection
>   Platform/RPi3: Enable the use of DualSerialPortLib
>   Platform/RPi4: Enable the use of DualSerialPortLib
>
>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 836 ++++++++++++++++++++
>  Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf |  55 ++
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                   |  15 +-
>  Platform/RaspberryPi/RPi3/Readme.md                                  |   7 +
>  Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf                  |   7 +
>  Platform/RaspberryPi/RPi4/RPi4.dsc                                   |  26 +-
>  Platform/RaspberryPi/RPi4/RPi4.fdf                                   |   4 -
>  Platform/RaspberryPi/RPi4/Readme.md                                  |  21 +-
>  Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h          |  22 +
>  9 files changed, 944 insertions(+), 49 deletions(-)
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
>  create mode 100644 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
>
> --
> 2.21.0.windows.1
>