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.

* 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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53507): https://edk2.groups.io/g/devel/message/53507
Mute This Topic: https://groups.io/mt/70222625/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.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
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53512): https://edk2.groups.io/g/devel/message/53512
Mute This Topic: https://groups.io/mt/70222625/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as 9369b01e86ad..41c1d9ba3304

Thanks!

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54437): https://edk2.groups.io/g/devel/message/54437
Mute This Topic: https://groups.io/mt/70222625/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-