Re: [edk2-devel] [PATCH v4 1/3] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

2021-06-12 Thread Ard Biesheuvel 
On Sat, 12 Jun 2021 at 15:40, Pete Batard  wrote:
>
> No more comments on this series for me.
>
> I have also tested this patch using Putty (serial) on Windows ARM64, to
> validate that COM1: was set to the serial output defined in config.txt,
> be it miniUART or PL011.
>
> The only thing I saw was that the baudrate for PL011 was double the one
> set in Putty, but this is an issue with the Windows drivers using
> hardcoded clocks
> (https://github.com/raspberrypi/windows-drivers/issues/33) and unrelated
> to these changes.
>
> With this:
>
> On 2021.06.07 08:53, Sunny Wang wrote:
> > Changes:
> >1. Add code to ConfigDxe driver and AcpiTables module to dynamically
> >   build either Mini UART or PL011 UART info in ACPI. This also fixes
> >   the issue discussed in https://github.com/pftf/RPi4/issues/118.
> >2. Cleanup by moving duplicate Debug Port 2 table related defines and
> >   structures to a newly created header file (RpiDebugPort2Table.h).
> >
> > Testing Done:
> >- Booted to UEFI shell and use acpiview command to check the result of
> >  the different UART settings in config.txt (enabling either Mini UART
> >  or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
> >  changed as expected.
> >
> > Cc: Samer El-Haj-Mahmoud 
> > Cc: Sami Mujawar 
> > Cc: Jeremy Linton 
> > Cc: Pete Batard 
> > Cc: Ard Biesheuvel 
> > Cc: Mario Bălănică 
> > Signed-off-by: Sunny Wang 
> > ---
> >   .../RaspberryPi/AcpiTables/AcpiTables.inf |   8 +-
> >   .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  81 +
> >   .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  |  30 +---
> >   .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  91 ++
> >   .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  |  10 +-
> >   Platform/RaspberryPi/AcpiTables/Uart.asl  | 155 +-
> >   .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  48 +-
> >   .../Drivers/ConfigDxe/ConfigDxe.inf   |   1 +
> >   .../IndustryStandard/RpiDebugPort2Table.h |  33 
> >   Platform/RaspberryPi/Include/UartSelection.h  |  20 +++
> >   Platform/RaspberryPi/RPi3/RPi3.dsc|   8 +
> >   Platform/RaspberryPi/RPi4/RPi4.dsc|   8 +
> >   Platform/RaspberryPi/RaspberryPi.dec  |   1 +
> >   13 files changed, 410 insertions(+), 84 deletions(-)
> >   create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> >   rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
> >   create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> >   rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
> >   create mode 100644 
> > Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> >   create mode 100644 Platform/RaspberryPi/Include/UartSelection.h
> >
> > diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
> > b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> > index d3363a76a1..1ddc9ca5fe 100644
> > --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> > +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> > @@ -2,7 +2,7 @@
> >   #
> >   #  ACPI table data and ASL sources required to boot the platform.
> >   #
> > -#  Copyright (c) 2019, ARM Limited. All rights reserved.
> > +#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
> >   #  Copyright (c) 2017, Andrey Warkentin 
> >   #  Copyright (c) Microsoft Corporation. All rights reserved.
> >   #
> > @@ -28,12 +28,14 @@
> > Emmc.asl
> > Madt.aslc
> > Fadt.aslc
> > -  Dbg2.aslc
> > +  Dbg2MiniUart.aslc
> > +  Dbg2Pl011.aslc
> > Gtdt.aslc
> > Iort.aslc
> > Dsdt.asl
> > Csrt.aslc
> > -  Spcr.aslc
> > +  SpcrMiniUart.aslc
> > +  SpcrPl011.aslc
> > Pptt.aslc
> > SsdtThermal.asl
> >
> > diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
> > b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> > new file mode 100644
> > index 00..be7d96c179
> > --- /dev/null
> > +++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> > @@ -0,0 +1,81 @@
> > +/** @file
> > + *
> > + *  Debug Port Table (DBG2)
> > + *
> > + *  Copyright (c) 2019, Pete Batard 
> > + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> > + *
> > + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > + *
> > + **/
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "AcpiTables.h"
> > +
> > +#pragma pack(1)
> > +
> > +#define RPI_UART_INTERFACE_TYPE 
> > EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> > +#define RPI_UART_BASE_ADDRESS   
> > BCM2836_MINI_UART_BASE_ADDRESS
> > +#define RPI_UART_LENGTH 
> > BCM2836_MINI_UART_LENGTH
> > +//
> > +// RPI_UART_STR should match the value used Uart.asl
> > +//
> > +#define RPI_UART_STR{ '\\', '_', 'S', 
> > 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> > +
> > +#define

Re: [edk2-devel] [PATCH v4 1/3] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

2021-06-12 Thread Pete Batard 

No more comments on this series for me.

I have also tested this patch using Putty (serial) on Windows ARM64, to 
validate that COM1: was set to the serial output defined in config.txt, 
be it miniUART or PL011.


The only thing I saw was that the baudrate for PL011 was double the one 
set in Putty, but this is an issue with the Windows drivers using 
hardcoded clocks 
(https://github.com/raspberrypi/windows-drivers/issues/33) and unrelated 
to these changes.


With this:

On 2021.06.07 08:53, Sunny Wang wrote:

Changes:
   1. Add code to ConfigDxe driver and AcpiTables module to dynamically
  build either Mini UART or PL011 UART info in ACPI. This also fixes
  the issue discussed in https://github.com/pftf/RPi4/issues/118.
   2. Cleanup by moving duplicate Debug Port 2 table related defines and
  structures to a newly created header file (RpiDebugPort2Table.h).

Testing Done:
   - Booted to UEFI shell and use acpiview command to check the result of
 the different UART settings in config.txt (enabling either Mini UART
 or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
 changed as expected.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Cc: Mario Bălănică 
Signed-off-by: Sunny Wang 
---
  .../RaspberryPi/AcpiTables/AcpiTables.inf |   8 +-
  .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  81 +
  .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  |  30 +---
  .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  91 ++
  .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  |  10 +-
  Platform/RaspberryPi/AcpiTables/Uart.asl  | 155 +-
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  48 +-
  .../Drivers/ConfigDxe/ConfigDxe.inf   |   1 +
  .../IndustryStandard/RpiDebugPort2Table.h |  33 
  Platform/RaspberryPi/Include/UartSelection.h  |  20 +++
  Platform/RaspberryPi/RPi3/RPi3.dsc|   8 +
  Platform/RaspberryPi/RPi4/RPi4.dsc|   8 +
  Platform/RaspberryPi/RaspberryPi.dec  |   1 +
  13 files changed, 410 insertions(+), 84 deletions(-)
  create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
  rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
  create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
  rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
  create mode 100644 
Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
  create mode 100644 Platform/RaspberryPi/Include/UartSelection.h

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d3363a76a1..1ddc9ca5fe 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
  #
  #  ACPI table data and ASL sources required to boot the platform.
  #
-#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
  #  Copyright (c) 2017, Andrey Warkentin 
  #  Copyright (c) Microsoft Corporation. All rights reserved.
  #
@@ -28,12 +28,14 @@
Emmc.asl
Madt.aslc
Fadt.aslc
-  Dbg2.aslc
+  Dbg2MiniUart.aslc
+  Dbg2Pl011.aslc
Gtdt.aslc
Iort.aslc
Dsdt.asl
Csrt.aslc
-  Spcr.aslc
+  SpcrMiniUart.aslc
+  SpcrPl011.aslc
Pptt.aslc
SsdtThermal.asl
  
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc

new file mode 100644
index 00..be7d96c179
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,81 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard 
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE 
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS   
BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH 
BCM2836_MINI_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR{ '\\', '_', 'S', 'B', 
'.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, 
UartNameStr) {\
+{  
   \
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* UINT8 
Revision */\
+  sizeof (DBG2_DEBUG_DEVICE_INFORMATION), /* 
UINT16Length */  \
+  NumReg,

[edk2-devel] [PATCH v4 1/3] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

2021-06-07 Thread Sunny Wang 
Changes:
  1. Add code to ConfigDxe driver and AcpiTables module to dynamically
 build either Mini UART or PL011 UART info in ACPI. This also fixes
 the issue discussed in https://github.com/pftf/RPi4/issues/118.
  2. Cleanup by moving duplicate Debug Port 2 table related defines and
 structures to a newly created header file (RpiDebugPort2Table.h).

Testing Done:
  - Booted to UEFI shell and use acpiview command to check the result of
the different UART settings in config.txt (enabling either Mini UART
or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
changed as expected.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Cc: Mario Bălănică 
Signed-off-by: Sunny Wang 
---
 .../RaspberryPi/AcpiTables/AcpiTables.inf |   8 +-
 .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  81 +
 .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  |  30 +---
 .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  91 ++
 .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  |  10 +-
 Platform/RaspberryPi/AcpiTables/Uart.asl  | 155 +-
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  48 +-
 .../Drivers/ConfigDxe/ConfigDxe.inf   |   1 +
 .../IndustryStandard/RpiDebugPort2Table.h |  33 
 Platform/RaspberryPi/Include/UartSelection.h  |  20 +++
 Platform/RaspberryPi/RPi3/RPi3.dsc|   8 +
 Platform/RaspberryPi/RPi4/RPi4.dsc|   8 +
 Platform/RaspberryPi/RaspberryPi.dec  |   1 +
 13 files changed, 410 insertions(+), 84 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
 create mode 100644 
Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
 create mode 100644 Platform/RaspberryPi/Include/UartSelection.h

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d3363a76a1..1ddc9ca5fe 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
 #
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
 #  Copyright (c) 2017, Andrey Warkentin 
 #  Copyright (c) Microsoft Corporation. All rights reserved.
 #
@@ -28,12 +28,14 @@
   Emmc.asl
   Madt.aslc
   Fadt.aslc
-  Dbg2.aslc
+  Dbg2MiniUart.aslc
+  Dbg2Pl011.aslc
   Gtdt.aslc
   Iort.aslc
   Dsdt.asl
   Csrt.aslc
-  Spcr.aslc
+  SpcrMiniUart.aslc
+  SpcrPl011.aslc
   Pptt.aslc
   SsdtThermal.asl
 
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
new file mode 100644
index 00..be7d96c179
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,81 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard 
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE 
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS   
BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH 
BCM2836_MINI_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR{ '\\', '_', 'S', 'B', 
'.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, 
UartNameStr) {\
+{  
   \
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* UINT8 
Revision */\
+  sizeof (DBG2_DEBUG_DEVICE_INFORMATION), /* 
UINT16Length */  \
+  NumReg, /* UINT8 
NumberofGenericAddressRegisters */ \
+  RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,/* 
UINT16NameSpaceStringLength */   \
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString), /* 
UINT16NameSpaceStringOffset */   \
+  0,  /* 
UINT16OemDataLength */   \
+  0,  /* 
UINT16OemDataOffset */   \