public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Dynamically build UARTs info in ACPI
@ 2021-05-31  8:22 Sunny Wang
  2021-05-31  8:22 ` [PATCH v3 1/2] Platform/RaspberryPi: " Sunny Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sunny Wang @ 2021-05-31  8:22 UTC (permalink / raw)
  To: devel; +Cc: Sunny Wang

In v3: Address comments given by Jeremy and Matio on v2.
In v2: Address comments given by Pete on v1.

Dynamically build UARTs info in ACPI so that it can match the UART
related settings defined in config.txt 

Sunny Wang (2):
  Platform/RaspberryPi: Dynamically build UARTs info in ACPI
  Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

 .../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      | 158 +++++++++++++-----
 .../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, 413 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

-- 
2.31.0.windows.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI
  2021-05-31  8:22 [PATCH v3 0/2] Dynamically build UARTs info in ACPI Sunny Wang
@ 2021-05-31  8:22 ` Sunny Wang
  2021-05-31  8:22 ` [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
  2021-06-02 12:11 ` [edk2-devel] [PATCH v3 0/2] Dynamically build UARTs info in ACPI Samer El-Haj-Mahmoud
  2 siblings, 0 replies; 10+ messages in thread
From: Sunny Wang @ 2021-05-31  8:22 UTC (permalink / raw)
  To: devel
  Cc: Sunny Wang, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
	Pete Batard, Ard Biesheuvel, Mario Bălănică,
	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 alos 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 <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Mario Bălănică <mariobalanica02@gmail.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
 .../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 <andrey.warkentin@gmail.com>
 #  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 0000000000..be7d96c179
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,81 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#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),                         /* UINT16    Length */                          \
+      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
+      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
+      0,                                                              /* UINT16    OemDataLength */                   \
+      0,                                                              /* UINT16    OemDataOffset */                   \
+      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
+      SubType,                                                        /* UINT16    Port Subtype */                    \
+      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
+      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
+    },                                                                                                                \
+    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
+    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
+    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
+  }
+
+
+STATIC DBG2_TABLE Dbg2 = {
+  {
+    ACPI_HEADER (
+      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+      DBG2_TABLE,
+      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
+    ),
+    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
+    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
+  },
+  {
+    /*
+     * Kernel Debug Port
+     */
+    DBG2_DEBUG_PORT_DDI (
+      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
+      RPI_UART_INTERFACE_TYPE,
+      RPI_UART_BASE_ADDRESS,
+      RPI_UART_LENGTH,
+      RPI_UART_STR
+    ),
+  }
+};
+
+#pragma pack()
+
+//
+// Reference the table being generated to prevent the optimizer from removing
+// the data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Dbg2;
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
similarity index 72%
rename from Platform/RaspberryPi/AcpiTables/Dbg2.aslc
rename to Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
index e3f2adae7e..e07869b027 100644
--- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
@@ -3,7 +3,7 @@
  *  Debug Port Table (DBG2)
  *
  *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
- *  Copyright (c) 2012-2020, ARM Limited. All rights reserved.
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -11,7 +11,7 @@
 
 #include <IndustryStandard/Acpi.h>
 #include <IndustryStandard/Bcm2836.h>
-#include <IndustryStandard/DebugPort2Table.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
 #include <Library/AcpiLib.h>
 #include <Library/PcdLib.h>
 
@@ -19,37 +19,13 @@
 
 #pragma pack(1)
 
-#define RPI_DBG2_NUM_DEBUG_PORTS                        1
-#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
-#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
-
-#if (RPI_MODEL == 4)
 #define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
 #define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
 #define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
-#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
-#else
-#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 }
-#endif
-
-typedef struct {
-  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
-  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
-  UINT32                                                AddressSize;
-  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
-} DBG2_DEBUG_DEVICE_INFORMATION;
-
-typedef struct {
-  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
-  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
-} DBG2_TABLE;
-
+#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
 
 #define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
     {                                                                                                                 \
diff --git a/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
new file mode 100644
index 0000000000..2f638c61a5
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
@@ -0,0 +1,91 @@
+/** @file
+* SPCR Table
+*
+* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
+* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#define RPI_UART_FLOW_CONTROL_NONE           0
+
+#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
+
+STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
+  ACPI_HEADER (
+    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
+  ),
+  // UINT8                                   InterfaceType;
+  RPI_UART_INTERFACE_TYPE,
+  // UINT8                                   Reserved1[3];
+  {
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE
+  },
+  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
+  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
+  // UINT8                                   InterruptType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
+  // UINT8                                   Irq;
+  0,                                         // Not used on ARM
+  // UINT32                                  GlobalSystemInterrupt;
+  RPI_UART_INTERRUPT,
+  // UINT8                                   BaudRate;
+#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
+#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
+#else
+#error Unsupported SPCR Baud Rate
+#endif
+  // UINT8                                   Parity;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
+  // UINT8                                   StopBits;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
+  // UINT8                                   FlowControl;
+  RPI_UART_FLOW_CONTROL_NONE,
+  // UINT8                                   TerminalType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
+  // UINT8                                   Reserved2;
+  EFI_ACPI_RESERVED_BYTE,
+  // UINT16                                  PciDeviceId;
+  0xFFFF,
+  // UINT16                                  PciVendorId;
+  0xFFFF,
+  // UINT8                                   PciBusNumber;
+  0x00,
+  // UINT8                                   PciDeviceNumber;
+  0x00,
+  // UINT8                                   PciFunctionNumber;
+  0x00,
+  // UINT32                                  PciFlags;
+  0x00000000,
+  // UINT8                                   PciSegment;
+  0x00,
+  // UINT32                                  Reserved3;
+  EFI_ACPI_RESERVED_DWORD
+};
+
+//
+// Reference the table being generated to prevent the optimizer from removing the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Spcr;
diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
similarity index 87%
rename from Platform/RaspberryPi/AcpiTables/Spcr.aslc
rename to Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
index 07df3a718d..06e19c1245 100644
--- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
+++ b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
@@ -2,7 +2,7 @@
 * SPCR Table
 *
 * Copyright (c) 2019 Pete Batard <pete@akeo.ie>
-* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
+* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
 *
 * SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -18,16 +18,10 @@
 
 #define RPI_UART_FLOW_CONTROL_NONE           0
 
-// Prefer PL011 serial output on the Raspberry Pi 4
-#if (RPI_MODEL == 4)
 #define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
 #define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
 #define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
-#else
-#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
-#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
-#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
-#endif
+
 STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
   ACPI_HEADER (
     EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index 81ae6711af..bac9d791eb 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -2,6 +2,7 @@
  *
  *  [DSDT] Serial devices (UART).
  *
+ *  Copyright (c) 2021, ARM Limited. All rights reserved.
  *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
  *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) Microsoft Corporation. All rights reserved.
@@ -93,57 +94,125 @@ Device(BTH0)
 {
   Name (_HID, "BCM2EA6")
   Name (_CID, "BCM2EA6")
+
+  //
+  // UART In Use will be dynamically updated during boot
+  // 0x55 0x52 0x49 0x55 0xA 0x2 (Value must > 1)
+  //
+  Name (URIU, 0x2)
+
   Method (_STA)
   {
     Return (0xf)
   }
+
+  //
+  // Resource for URT0 (PL011)
+  //
+  Name (BTPL, ResourceTemplate ()
+  {
+    UARTSerialBus(
+      115200,        // InitialBaudRate: in BPS
+      ,              // BitsPerByte: default to 8 bits
+      ,              // StopBits: Defaults to one bit
+      0x00,          // LinesInUse: 8 1-bit flags to
+                    //   declare enabled control lines.
+                    //   Raspberry Pi does not exposed
+                    //   HW control signals -> not supported.
+                    //   Optional bits:
+                    //   - Bit 7 (0x80) Request To Send (RTS)
+                    //   - Bit 6 (0x40) Clear To Send (CTS)
+                    //   - Bit 5 (0x20) Data Terminal Ready (DTR)
+                    //   - Bit 4 (0x10) Data Set Ready (DSR)
+                    //   - Bit 3 (0x08) Ring Indicator (RI)
+                    //   - Bit 2 (0x04) Data Carrier Detect (DTD)
+                    //   - Bit 1 (0x02) Reserved. Must be 0.
+                    //   - Bit 0 (0x01) Reserved. Must be 0.
+      ,              // IsBigEndian:
+                    //   default to LittleEndian.
+      ,              // Parity: Defaults to no parity
+      ,              // FlowControl: Defaults to
+                    //   no flow control.
+      16,            // ReceiveBufferSize
+      16,            // TransmitBufferSize
+      "\\_SB.GDV0.URT0",  // ResourceSource:
+                    //   UART bus controller name
+      ,              // ResourceSourceIndex: assumed to be 0
+      ,              // ResourceUsage: assumed to be
+                    //   ResourceConsumer
+      UAR0,          // DescriptorName: creates name
+                    //   for offset of resource descriptor
+    )                // Vendor data
+    //
+    // RPIQ connection for BT_ON/OFF
+    //
+    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
+  })
+
+  //
+  // Resource for URTM (miniUART)
+  //
+  Name (BTMN, ResourceTemplate ()
+  {
+    //
+    // BT UART: ResourceSource will be dynamically updated to
+    // either URT0 (PL011) or URTM (miniUART) during boot
+    //
+    UARTSerialBus(
+      115200,        // InitialBaudRate: in BPS
+      ,              // BitsPerByte: default to 8 bits
+      ,              // StopBits: Defaults to one bit
+      0x00,          // LinesInUse: 8 1-bit flags to
+                    //   declare enabled control lines.
+                    //   Raspberry Pi does not exposed
+                    //   HW control signals -> not supported.
+                    //   Optional bits:
+                    //   - Bit 7 (0x80) Request To Send (RTS)
+                    //   - Bit 6 (0x40) Clear To Send (CTS)
+                    //   - Bit 5 (0x20) Data Terminal Ready (DTR)
+                    //   - Bit 4 (0x10) Data Set Ready (DSR)
+                    //   - Bit 3 (0x08) Ring Indicator (RI)
+                    //   - Bit 2 (0x04) Data Carrier Detect (DTD)
+                    //   - Bit 1 (0x02) Reserved. Must be 0.
+                    //   - Bit 0 (0x01) Reserved. Must be 0.
+      ,              // IsBigEndian:
+                    //   default to LittleEndian.
+      ,              // Parity: Defaults to no parity
+      ,              // FlowControl: Defaults to
+                    //   no flow control.
+      16,            // ReceiveBufferSize
+      16,            // TransmitBufferSize
+      "\\_SB.GDV0.URTM",  // ResourceSource:
+                    //   UART bus controller name
+      ,              // ResourceSourceIndex: assumed to be 0
+      ,              // ResourceUsage: assumed to be
+                    //   ResourceConsumer
+      UARM,          // DescriptorName: creates name
+                    //   for offset of resource descriptor
+    )                // Vendor data
+    //
+    // RPIQ connection for BT_ON/OFF
+    //
+    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
+  })
+
   Method (_CRS, 0x0, Serialized)
   {
-    Name (RBUF, ResourceTemplate ()
+    if (URIU == 0)
     {
-      // BT UART: URT0 (PL011) or URTM (miniUART)
-      UARTSerialBus(
-        115200,        // InitialBaudRate: in BPS
-        ,              // BitsPerByte: default to 8 bits
-        ,              // StopBits: Defaults to one bit
-        0x00,          // LinesInUse: 8 1-bit flags to
-                       //   declare enabled control lines.
-                       //   Raspberry Pi does not exposed
-                       //   HW control signals -> not supported.
-                       //   Optional bits:
-                       //   - Bit 7 (0x80) Request To Send (RTS)
-                       //   - Bit 6 (0x40) Clear To Send (CTS)
-                       //   - Bit 5 (0x20) Data Terminal Ready (DTR)
-                       //   - Bit 4 (0x10) Data Set Ready (DSR)
-                       //   - Bit 3 (0x08) Ring Indicator (RI)
-                       //   - Bit 2 (0x04) Data Carrier Detect (DTD)
-                       //   - Bit 1 (0x02) Reserved. Must be 0.
-                       //   - Bit 0 (0x01) Reserved. Must be 0.
-        ,              // IsBigEndian:
-                       //   default to LittleEndian.
-        ,              // Parity: Defaults to no parity
-        ,              // FlowControl: Defaults to
-                       //   no flow control.
-        16,            // ReceiveBufferSize
-        16,            // TransmitBufferSize
-#if (RPI_MODEL == 4)
-        "\\_SB.GDV0.URTM",  // ResourceSource:
-#else
-        "\\_SB.GDV0.URT0",  // ResourceSource:
-#endif
-                       //   UART bus controller name
-        ,              // ResourceSourceIndex: assumed to be 0
-        ,              // ResourceUsage: assumed to be
-                       //   ResourceConsumer
-        UAR0,          // DescriptorName: creates name
-                       //   for offset of resource descriptor
-      )                // Vendor data
-
       //
-      // RPIQ connection for BT_ON/OFF
+      // PL011 UART is configured for console output
+      // Return Mini UART for Bluetooth
       //
-      GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
-    })
-    Return (RBUF)
+      return (^BTMN)
+    }
+    else
+    {
+      //
+      // Mini UART is configured for console output
+      // Return PL011 UART for Bluetooth
+      //
+      return (^BTPL)
+    }
   }
 }
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index d3c5869949..d6efb59793 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -12,6 +12,10 @@
 #include <IndustryStandard/Bcm2836.h>
 #include <IndustryStandard/Bcm2836Gpio.h>
 #include <IndustryStandard/RpiMbox.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <IndustryStandard/RpiDebugPort2Table.h>
+#include <UartSelection.h>
+
 #include <Library/AcpiLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
@@ -23,6 +27,7 @@
 #include <Library/NetLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/PcdLib.h>
 #include <Protocol/AcpiTable.h>
 #include <Protocol/BcmGenetPlatformDevice.h>
 #include <Protocol/RpiFirmware.h>
@@ -743,6 +748,11 @@ STATIC CONST AML_NAME_OP_REPLACE SsdtEmmcNameOpReplace[] = {
   { }
 };
 
+STATIC CONST AML_NAME_OP_REPLACE DsdtNameOpReplace[] = {
+  { "URIU", PcdToken (PcdUartInUse) },
+  { }
+};
+
 STATIC CONST NAMESPACE_TABLES SdtTables[] = {
   {
     SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 'A', 'N'),
@@ -756,11 +766,11 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
     PcdToken(PcdSdIsArasan),
     SsdtEmmcNameOpReplace
   },
-  {
+  { // DSDT
     SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),
     0,
     0,
-    NULL
+    DsdtNameOpReplace
   },
   { }
 };
@@ -779,6 +789,9 @@ HandleDynamicNamespace (
 {
   UINTN Tables;
 
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *SpcrTable;
+  DBG2_TABLE                                     *Dbg2Table;
+
   switch (AcpiHeader->Signature) {
   case SIGNATURE_32 ('D', 'S', 'D', 'T'):
   case SIGNATURE_32 ('S', 'S', 'D', 'T'):
@@ -788,14 +801,37 @@ HandleDynamicNamespace (
       }
     }
     DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
-
     return FALSE;
+
   case SIGNATURE_32 ('I', 'O', 'R', 'T'):
     // only enable the IORT on machines with >3G and no limit
     // to avoid problems with rhel/centos and other older OSs
     if (PcdGet32 (PcdRamLimitTo3GB) || !PcdGet32 (PcdRamMoreThan3GB)) {
       return FALSE;
     }
+    return TRUE;
+
+  case SIGNATURE_32 ('S', 'P', 'C', 'R'):
+    SpcrTable = (EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *)AcpiHeader;
+    if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE) &&
+        (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART)) {
+      return TRUE;
+    } else if ((PcdGet32 (PcdUartInUse) == MINI_UART_IN_USE) &&
+               (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART)) {
+      return TRUE;
+    }
+    return FALSE;
+
+  case SIGNATURE_32 ('D', 'B', 'G', '2'):
+    Dbg2Table = (DBG2_TABLE *)AcpiHeader;
+    if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE) &&
+        (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)) {
+      return TRUE;
+    } else if ((PcdGet32 (PcdUartInUse) == MINI_UART_IN_USE) &&
+               (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART)) {
+      return TRUE;
+    }
+    return FALSE;
   }
 
   return TRUE;
@@ -812,6 +848,12 @@ ConfigInitialize (
   EFI_STATUS                      Status;
   EFI_EVENT                       EndOfDxeEvent;
 
+  if ((MmioRead32(GPIO_GPFSEL1) & GPFSEL1_UART_MASK) == PL011_UART_IN_USE_REG_VALUE) {
+    PcdSet32S (PcdUartInUse, PL011_UART_IN_USE);
+  } else if ((MmioRead32(GPIO_GPFSEL1) & GPFSEL1_UART_MASK) == MINI_UART_IN_USE_REG_VALUE) {
+    PcdSet32S (PcdUartInUse, MINI_UART_IN_USE);
+  }
+
   Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
                   NULL, (VOID**)&mFwProtocol);
   ASSERT_EFI_ERROR (Status);
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 032e40b0c3..597e1b4205 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -94,6 +94,7 @@
   gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
   gRaspberryPiTokenSpaceGuid.PcdFanTemp
   gRaspberryPiTokenSpaceGuid.PcdBootPolicy
+  gRaspberryPiTokenSpaceGuid.PcdUartInUse
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
new file mode 100644
index 0000000000..73134dfdc2
--- /dev/null
+++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
@@ -0,0 +1,33 @@
+/** @file
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ **/
+#ifndef __RPI_DEBUG_PORT_2_H__
+#define __RPI_DEBUG_PORT_2_H__
+
+#include <IndustryStandard/DebugPort2Table.h>
+
+#define RPI_DBG2_NUM_DEBUG_PORTS                        1
+#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
+#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
+
+#pragma pack(1)
+
+
+typedef struct {
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
+  UINT32                                                AddressSize;
+  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
+} DBG2_DEBUG_DEVICE_INFORMATION;
+
+typedef struct {
+  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
+  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
+} DBG2_TABLE;
+
+#pragma pack()
+#endif  //__RPI_DEBUG_PORT_2_H__
diff --git a/Platform/RaspberryPi/Include/UartSelection.h b/Platform/RaspberryPi/Include/UartSelection.h
new file mode 100644
index 0000000000..803bf860bb
--- /dev/null
+++ b/Platform/RaspberryPi/Include/UartSelection.h
@@ -0,0 +1,20 @@
+/** @file
+ *
+ *  Copyright (c) 2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#ifndef UART_SELECTION_H
+#define UART_SELECTION_H
+
+#define GPFSEL1_UART_MASK             (BIT17 | BIT16 | BIT15 | BIT14 | BIT13 | BIT12)
+
+#define PL011_UART_IN_USE_REG_VALUE   (BIT17 | BIT14)      // ALT0 - 0x24000
+#define MINI_UART_IN_USE_REG_VALUE    (BIT16 | BIT13)      // ALT5 - 0x12000
+
+#define PL011_UART_IN_USE             0
+#define MINI_UART_IN_USE              1
+
+#endif /* UART_SELECTION_H */
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 425c7ff9ec..53825bcf62 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -552,6 +552,14 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
 
+  #
+  # UART in use
+  # This value will be synchronized with the setting in config.txt
+  # 0  - PL011_UART_IN_USE
+  # 1  - MINI_UART_IN_USE
+  #
+  gRaspberryPiTokenSpaceGuid.PcdUartInUse|1
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index d8c6fdd4bd..fd73c4d14b 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -568,6 +568,14 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
 
+  #
+  # UART in use
+  # This value will be synchronized with the setting in config.txt
+  # 0  - PL011_UART_IN_USE
+  # 1  - MINI_UART_IN_USE
+  #
+  gRaspberryPiTokenSpaceGuid.PcdUartInUse|0
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index 8eb1c2bac7..f1dd8ac0ed 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -72,3 +72,4 @@
   gRaspberryPiTokenSpaceGuid.PcdPlatformResetDelay|0|UINT32|0x0000001E
   gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F
   gRaspberryPiTokenSpaceGuid.PcdBootPolicy|0|UINT32|0x00000020
+  gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
-- 
2.31.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31  8:22 [PATCH v3 0/2] Dynamically build UARTs info in ACPI Sunny Wang
  2021-05-31  8:22 ` [PATCH v3 1/2] Platform/RaspberryPi: " Sunny Wang
@ 2021-05-31  8:22 ` Sunny Wang
  2021-05-31 12:40   ` Mario Bălănică
  2021-06-02 12:11 ` [edk2-devel] [PATCH v3 0/2] Dynamically build UARTs info in ACPI Samer El-Haj-Mahmoud
  2 siblings, 1 reply; 10+ messages in thread
From: Sunny Wang @ 2021-05-31  8:22 UTC (permalink / raw)
  To: devel
  Cc: Sunny Wang, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
	Pete Batard, Ard Biesheuvel, Mario Bălănică,
	Sunny Wang

This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows IOT.

Note that PL011 UART still doesn't work with Windows 10 IOT with this
change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
Therefore, there should be no issue with PL011 UART related changes,
and we will still need a Windows expert to take a further look in the
future. Making PL011 UART work properly with Windows 10 IOT may require
additional changes to Windows driver or firmware's ACPI table.

Testing Done:
  - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by WOR) with
    the RPi-Windows-Drivers release ver 0.5 downloaded from
    https://github.com/worproject/RPi-Windows-Drivers/releases
    and checked that both Bluetooth and serial port (Mini UART) can
    work fine.
  - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
    console connection (PL011 UART).

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Mario Bălănică <mariobalanica02@gmail.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
 Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index bac9d791eb..cb99086d39 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -79,6 +85,11 @@ Device (URTM)
     // from muxing the pins away.
 
     // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0, ResourceConsumer, , ) { 14, 15 }
+    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -143,10 +154,6 @@ Device(BTH0)
       UAR0,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
   })
 
   //
@@ -190,10 +197,6 @@ Device(BTH0)
       UARM,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, ResourceConsumer, , ) { 128 }
   })
 
   Method (_CRS, 0x0, Serialized)
-- 
2.31.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31  8:22 ` [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
@ 2021-05-31 12:40   ` Mario Bălănică
  2021-05-31 13:26     ` [edk2-devel] " Sunny Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Mario Bălănică @ 2021-05-31 12:40 UTC (permalink / raw)
  To: Sunny Wang
  Cc: devel, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
	Pete Batard, Ard Biesheuvel

[-- Attachment #1: Type: text/plain, Size: 5008 bytes --]

Hi Sunny,

What issues are you seeing with the PL011 UART in Windows? Last time I
checked, it worked fine, and the fact that Bluetooth works also confirms
this.
It won't show up as a COM port (like mini UART does) as it's built using
SerCx2.

@@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +


@@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
>

What is the reason for trying to mux both UARTs to the BT chip? If PL011 is
used for Bluetooth and the mini UART driver loads *after* it, wouldn't it
mux away the pins and break Bluetooth?




În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com> a scris:

> This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
> in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
> Bluetooth and serial port (Mini UART) in Windows IOT.
>
> Note that PL011 UART still doesn't work with Windows 10 IOT with this
> change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
> Therefore, there should be no issue with PL011 UART related changes,
> and we will still need a Windows expert to take a further look in the
> future. Making PL011 UART work properly with Windows 10 IOT may require
> additional changes to Windows driver or firmware's ACPI table.
>
> Testing Done:
>   - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by
> WOR) with
>     the RPi-Windows-Drivers release ver 0.5 downloaded from
>     https://github.com/worproject/RPi-Windows-Drivers/releases
>     and checked that both Bluetooth and serial port (Mini UART) can
>     work fine.
>   - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
>     console connection (PL011 UART).
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Mario Bălănică <mariobalanica02@gmail.com>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl
> b/Platform/RaspberryPi/AcpiTables/Uart.asl
> index bac9d791eb..cb99086d39 100644
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> @@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -143,10 +154,6 @@ Device(BTH0)
>        UAR0,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    //
> @@ -190,10 +197,6 @@ Device(BTH0)
>        UARM,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    Method (_CRS, 0x0, Serialized)
> --
> 2.31.0.windows.1
>
>

[-- Attachment #2: Type: text/html, Size: 6798 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31 12:40   ` Mario Bălănică
@ 2021-05-31 13:26     ` Sunny Wang
  2021-05-31 15:56       ` Mario Bălănică
  0 siblings, 1 reply; 10+ messages in thread
From: Sunny Wang @ 2021-05-31 13:26 UTC (permalink / raw)
  To: devel@edk2.groups.io, mariobalanica02@gmail.com
  Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton, Pete Batard,
	Ard Biesheuvel, Sunny Wang

[-- Attachment #1: Type: text/plain, Size: 8911 bytes --]

Hi Mario,

Thanks for checking this.
Yeah, the problem is NO COM port for PL011 UART in Windows IOT’s device manager, so I thought it doesn’t work. If there is no COM port for PL011 UART, how can I check PL011 UART’s functionality? How can I use a telnet tool like PuTTY to send messages from RPi4 to my laptop? How to use SerCx2? Is there a guideline for using PL011 UART on RPi4 with Windows IoT?

As for why I mux both UARTs to the BT chip, I was trying to address your comment “This always assumes that PL011 is used for Bluetooth”. Apparently, it didn’t address your comment. No matter whether I add “PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011 UART as the primary serial console. Bluetooth only works when I configure Mini UART as the Primary serial console. I also confirmed that this problem also exists with RPi4 Release FW 1.26 + edk2-platforms-raspberrypi-pl011-bth-noflow.diff in https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any idea to make the Bluetooth work with configuring PL011 UART as the primary serial console? By the way, it is good to know the loading sequence. Do you know where I can quickly find this information without using WinDbg?


Actually, the original code change (Patch 2/2) is the patch (edk2-platforms-raspberrypi-pl011-bth-noflow.diff) on  https://github.com/worproject/RPi-Bluetooth-Testing/. I’m not familiar with Windows drivers’ behavior, so I had no idea about why we need to change them and couldn’t find more information about the changes’ background either. It looks like you know the details. Could you share with me more information about the background of these changes? At least, I have some questions below:
1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did we add this line in the beginning? I just removed it by your request and according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still worried about if we did this for some other purposes.
2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth? Where can I find the information?
3.  As for your comment about GPIO 31, I was confused, so I didn’t update anything for addressing this comment. My understanding was that It is intended to override the BCM_ALT2’s default setting from PullLow to PullNone to fack the CTS signal. If we change it to PullLow, it will be used for HW flow control, which is unwanted, isn’t it? What did you want me to change? Could you directly paste the code change you wanted here for my reference?

Moreover, for the changes you want, could you also paste the code change?

Best Regards,
Sunny Wang

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Mario Balanica via groups.io
Sent: Monday, May 31, 2021 8:41 PM
To: Sunny Wang <Sunny.Wang@arm.com>
Cc: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

Hi Sunny,

What issues are you seeing with the PL011 UART in Windows? Last time I checked, it worked fine, and the fact that Bluetooth works also confirms this.
It won't show up as a COM port (like mini UART does) as it's built using SerCx2.

@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+

@@ -79,6 +85,11 @@ Device (URTM)
     // from muxing the pins away.

     // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0<file://_SB.GPI0>", 0, ResourceConsumer, , ) { 14, 15 }
+    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+

What is the reason for trying to mux both UARTs to the BT chip? If PL011 is used for Bluetooth and the mini UART driver loads after it, wouldn't it mux away the pins and break Bluetooth?




În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com<mailto:Sunny.Wang@arm.com>> a scris:
This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows IOT.

Note that PL011 UART still doesn't work with Windows 10 IOT with this
change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
Therefore, there should be no issue with PL011 UART related changes,
and we will still need a Windows expert to take a further look in the
future. Making PL011 UART work properly with Windows 10 IOT may require
additional changes to Windows driver or firmware's ACPI table.

Testing Done:
  - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by WOR) with
    the RPi-Windows-Drivers release ver 0.5 downloaded from
    https://github.com/worproject/RPi-Windows-Drivers/releases
    and checked that both Bluetooth and serial port (Mini UART) can
    work fine.
  - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
    console connection (PL011 UART).

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com<mailto:samer.el-haj-mahmoud@arm.com>>
Cc: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
Cc: Jeremy Linton <jeremy.linton@arm.com<mailto:jeremy.linton@arm.com>>
Cc: Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb%2Btianocore@kernel.org>>
Cc: Mario Bălănică <mariobalanica02@gmail.com<mailto:mariobalanica02@gmail.com>>
Signed-off-by: Sunny Wang <sunny.wang@arm.com<mailto:sunny.wang@arm.com>>
---
 Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index bac9d791eb..cb99086d39 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -79,6 +85,11 @@ Device (URTM)
     // from muxing the pins away.

     // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0<file://_SB.GPI0>", 0, ResourceConsumer, , ) { 14, 15 }
+    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -143,10 +154,6 @@ Device(BTH0)
       UAR0,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ<file://_SB.GDV0.RPIQ>", 0, ResourceConsumer, , ) { 128 }
   })

   //
@@ -190,10 +197,6 @@ Device(BTH0)
       UARM,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ<file://_SB.GDV0.RPIQ>", 0, ResourceConsumer, , ) { 128 }
   })

   Method (_CRS, 0x0, Serialized)
--
2.31.0.windows.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 15889 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31 13:26     ` [edk2-devel] " Sunny Wang
@ 2021-05-31 15:56       ` Mario Bălănică
  2021-05-31 16:35         ` Mario Bălănică
  0 siblings, 1 reply; 10+ messages in thread
From: Mario Bălănică @ 2021-05-31 15:56 UTC (permalink / raw)
  To: Sunny Wang
  Cc: devel@edk2.groups.io, Samer El-Haj-Mahmoud, Sami Mujawar,
	Jeremy Linton, Pete Batard, Ard Biesheuvel

[-- Attachment #1: Type: text/plain, Size: 13644 bytes --]

>
> If there is no COM port for PL011 UART, how can I check PL011 UART’s
> functionality? How can I use a telnet tool like PuTTY to send messages from
> RPi4 to my laptop?


You can use: samples/MinComm at develop · ms-iot/samples (github.com)
<https://github.com/ms-iot/samples/tree/develop/MinComm> (let me know if
you need a precompiled binary)

As for why I mux both UARTs to the BT chip, I was trying to address your
> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
> it didn’t address your comment. No matter whether I add “PinFunction
> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, ,
> ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011
> UART as the primary serial console. Bluetooth only works when I configure
> Mini UART as the Primary serial console. I also confirmed that this problem
> also exists with RPi4 Release FW 1.26 +
> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
> idea to make the Bluetooth work with configuring PL011 UART as the primary
> serial console?


Bluetooth doesn't really work with the mini UART driver at the moment. The
chip stalls after a few transfers, as far as I remember. Haven't looked
much into it.

By the way, it is good to know the loading sequence. Do you know where I
> can quickly find this information without using WinDbg?


I'm not aware of any way to see the driver loading order without WinDbg.
Also, verbose mode must be enabled before boot: View Verbose Output -
Windows drivers | Microsoft Docs
<https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/view---verbose-output>

You could also test if Bluetooth will break by uninstalling / re-installing
the mini UART driver while the system is running, then try to scan for
nearby BT devices.

1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did
> we add this line in the beginning? I just removed it by your request and
> according to the change in worproject/RPi-Bluetooth-Testing/, but I’m
> still worried about if we did this for some other purposes.


That is a leftover from the MS-IOT tables. It tries to toggle BT_ON from
the GPIO expander (I2C) through RPIQ (mailbox), but the driver has no
knowledge of it. And since the firmware blob turns on Bluetooth by default,
the line is completely useless.

We may want to have something like this in the future, for power management
of the BT chip (will require some driver changes too).

2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using GPIO
> 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth? Where
> can I find the information?


Yes, the Bluetooth chip is available on pins 32, 33 (TX / RX). Also 30, 31
for hardware flow control (CTS / RTS). More info can be found in the BCM2711
datasheet
<https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf> (5.3.
Alternative Function Assignments)

3.  As for your comment about GPIO 31, I was confused, so I didn’t update
> anything for addressing this comment. My understanding was that It is
> intended to override the BCM_ALT2’s default setting from PullLow to
> PullNone to fack the CTS signal. If we change it to PullLow, it will be
> used for HW flow control, which is unwanted, isn’t it? What did you want me
> to change? Could you directly paste the code change you wanted here for my
> reference?


The BCM_ALT2 thing is a dirty hack. Pin 31 must be held LOW so that we can
talk to the BT chip without flow control. It seems this can't be described
with PinFunction, so I've relied on the default value of ALT2 for pin 31
(which turns out to be always LOW).

My proposal is to move the pin muxing stuff in ConfigDxe, like this:

STATIC VOID
>
> ApplyVariables (
>
>   VOID
>
>   )
>
> {
>
> ...
>
>
>>   if (FanOnGpio) {
>
>     DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
>
>     GpioPinFuncSet (FanOnGpio, GPIO_FSEL_OUTPUT);
>
>   }
>
>
>>   /*
>
>    * Bluetooth pin muxing
>
>    */
>
>   GpioPinFuncSet (31, GPIO_FSEL_OUTPUT);
>
>   GpioPinSet (31, 0);
>
>
>>   if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE)) {
>
>     GpioPinFuncSet (32, GPIO_FSEL_ALT3);
>
>     GpioPinFuncSet (33, GPIO_FSEL_ALT3);
>
>   } else {
>
>     GpioPinFuncSet (32, GPIO_FSEL_ALT5);
>
>     GpioPinFuncSet (33, GPIO_FSEL_ALT5);
>
>   }
>
> }
>
>
--Mario

În lun., 31 mai 2021 la 16:26, Sunny Wang <Sunny.Wang@arm.com> a scris:

> Hi Mario,
>
>
>
> Thanks for checking this.
>
> Yeah, the problem is NO COM port for PL011 UART in Windows IOT’s device
> manager, so I thought it doesn’t work. If there is no COM port for PL011
> UART, how can I check PL011 UART’s functionality? How can I use a
> telnet tool like PuTTY to send messages from RPi4 to my laptop? How to use
> SerCx2? Is there a guideline for using PL011 UART on RPi4 with Windows IoT?
>
>
>
> As for why I mux both UARTs to the BT chip, I was trying to address your
> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
> it didn’t address your comment. No matter whether I add “PinFunction
> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, ,
> ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011
> UART as the primary serial console. Bluetooth only works when I configure
> Mini UART as the Primary serial console. I also confirmed that this problem
> also exists with RPi4 Release FW 1.26 +
> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
> idea to make the Bluetooth work with configuring PL011 UART as the primary
> serial console? By the way, it is good to know the loading sequence. Do you
> know where I can quickly find this information without using WinDbg?
>
>
>
>
>
> Actually, the original code change (Patch 2/2) is the patch
> (edk2-platforms-raspberrypi-pl011-bth-noflow.diff) on
> https://github.com/worproject/RPi-Bluetooth-Testing/. I’m not familiar
> with Windows drivers’ behavior, so I had no idea about why we need to
> change them and couldn’t find more information about the changes’
> background either. It looks like you know the details. Could you share with
> me more information about the background of these changes? At least, I have
> some questions below:
>
> 1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did
> we add this line in the beginning? I just removed it by your request and
> according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still
> worried about if we did this for some other purposes.
>
> 2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using
> GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth?
> Where can I find the information?
>
> 3.  As for your comment about GPIO 31, I was confused, so I didn’t update
> anything for addressing this comment. My understanding was that It is
> intended to override the BCM_ALT2’s default setting from PullLow to
> PullNone to fack the CTS signal. If we change it to PullLow, it will be
> used for HW flow control, which is unwanted, isn’t it? What did you want me
> to change? Could you directly paste the code change you wanted here for my
> reference?
>
>
>
> Moreover, for the changes you want, could you also paste the code change?
>
>
> Best Regards,
>
> Sunny Wang
>
>
>
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Mario
> Balanica via groups.io
> *Sent:* Monday, May 31, 2021 8:41 PM
> *To:* Sunny Wang <Sunny.Wang@arm.com>
> *Cc:* devel@edk2.groups.io; Samer El-Haj-Mahmoud <
> Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>;
> Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>
> *Subject:* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable
> Bluetooth and UART in Windows OS
>
>
>
> Hi Sunny,
>
>
>
> What issues are you seeing with the PL011 UART in Windows? Last time I
> checked, it worked fine, and the fact that Bluetooth works also confirms
> this.
>
> It won't show up as a COM port (like mini UART does) as it's built using
> SerCx2.
>
>
>
> @@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
>
>
>
> @@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
>
>
>
> What is the reason for trying to mux both UARTs to the BT chip? If PL011
> is used for Bluetooth and the mini UART driver loads *after* it, wouldn't
> it mux away the pins and break Bluetooth?
>
>
>
>
>
>
>
>
>
> În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com> a scris:
>
> This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
> in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
> Bluetooth and serial port (Mini UART) in Windows IOT.
>
> Note that PL011 UART still doesn't work with Windows 10 IOT with this
> change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
> Therefore, there should be no issue with PL011 UART related changes,
> and we will still need a Windows expert to take a further look in the
> future. Making PL011 UART work properly with Windows 10 IOT may require
> additional changes to Windows driver or firmware's ACPI table.
>
> Testing Done:
>   - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by
> WOR) with
>     the RPi-Windows-Drivers release ver 0.5 downloaded from
>     https://github.com/worproject/RPi-Windows-Drivers/releases
>     and checked that both Bluetooth and serial port (Mini UART) can
>     work fine.
>   - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
>     console connection (PL011 UART).
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Mario Bălănică <mariobalanica02@gmail.com>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl
> b/Platform/RaspberryPi/AcpiTables/Uart.asl
> index bac9d791eb..cb99086d39 100644
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> @@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -143,10 +154,6 @@ Device(BTH0)
>        UAR0,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    //
> @@ -190,10 +197,6 @@ Device(BTH0)
>        UARM,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    Method (_CRS, 0x0, Serialized)
> --
> 2.31.0.windows.1
>
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>

[-- Attachment #2: Type: text/html, Size: 24350 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31 15:56       ` Mario Bălănică
@ 2021-05-31 16:35         ` Mario Bălănică
  2021-06-07  8:31           ` Sunny Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Mario Bălănică @ 2021-05-31 16:35 UTC (permalink / raw)
  To: Sunny Wang
  Cc: devel@edk2.groups.io, Samer El-Haj-Mahmoud, Sami Mujawar,
	Jeremy Linton, Pete Batard, Ard Biesheuvel

[-- Attachment #1: Type: text/plain, Size: 14556 bytes --]

edit: "GpioPinSet (31, 0);" should be "GpioPinSet (31, FALSE);" and maybe
add the "fake the CTS signal as we don't support HW flow control yet"
comment too.

When this patch gets merged, I'll add my Bluetooth changes to
raspberrypi/windows-drivers:
Windows IOT drivers (github.com)
<https://github.com/raspberrypi/windows-drivers> and submit another patch
here to enable hardware flow control.

--Mario

În lun., 31 mai 2021 la 18:56, Mario Bălănică <mariobalanica02@gmail.com> a
scris:

> If there is no COM port for PL011 UART, how can I check PL011 UART’s
>> functionality? How can I use a telnet tool like PuTTY to send messages from
>> RPi4 to my laptop?
>
>
> You can use: samples/MinComm at develop · ms-iot/samples (github.com)
> <https://github.com/ms-iot/samples/tree/develop/MinComm> (let me know if
> you need a precompiled binary)
>
> As for why I mux both UARTs to the BT chip, I was trying to address your
>> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
>> it didn’t address your comment. No matter whether I add “PinFunction
>> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer,
>> , ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure
>> PL011 UART as the primary serial console. Bluetooth only works when I
>> configure Mini UART as the Primary serial console. I also confirmed that
>> this problem also exists with RPi4 Release FW 1.26 +
>> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
>> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
>> idea to make the Bluetooth work with configuring PL011 UART as the primary
>> serial console?
>
>
> Bluetooth doesn't really work with the mini UART driver at the moment. The
> chip stalls after a few transfers, as far as I remember. Haven't looked
> much into it.
>
> By the way, it is good to know the loading sequence. Do you know where I
>> can quickly find this information without using WinDbg?
>
>
> I'm not aware of any way to see the driver loading order without WinDbg.
> Also, verbose mode must be enabled before boot: View Verbose Output -
> Windows drivers | Microsoft Docs
> <https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/view---verbose-output>
>
> You could also test if Bluetooth will break by uninstalling /
> re-installing the mini UART driver while the system is running, then try to
> scan for nearby BT devices.
>
> 1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did
>> we add this line in the beginning? I just removed it by your request and
>> according to the change in worproject/RPi-Bluetooth-Testing/, but I’m
>> still worried about if we did this for some other purposes.
>
>
> That is a leftover from the MS-IOT tables. It tries to toggle BT_ON from
> the GPIO expander (I2C) through RPIQ (mailbox), but the driver has no
> knowledge of it. And since the firmware blob turns on Bluetooth by default,
> the line is completely useless.
>
> We may want to have something like this in the future, for power
> management of the BT chip (will require some driver changes too).
>
> 2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using
>> GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth?
>> Where can I find the information?
>
>
> Yes, the Bluetooth chip is available on pins 32, 33 (TX / RX). Also 30, 31
> for hardware flow control (CTS / RTS). More info can be found in the BCM2711
> datasheet
> <https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf> (5.3.
> Alternative Function Assignments)
>
> 3.  As for your comment about GPIO 31, I was confused, so I didn’t update
>> anything for addressing this comment. My understanding was that It is
>> intended to override the BCM_ALT2’s default setting from PullLow to
>> PullNone to fack the CTS signal. If we change it to PullLow, it will be
>> used for HW flow control, which is unwanted, isn’t it? What did you want me
>> to change? Could you directly paste the code change you wanted here for my
>> reference?
>
>
> The BCM_ALT2 thing is a dirty hack. Pin 31 must be held LOW so that we can
> talk to the BT chip without flow control. It seems this can't be described
> with PinFunction, so I've relied on the default value of ALT2 for pin 31
> (which turns out to be always LOW).
>
> My proposal is to move the pin muxing stuff in ConfigDxe, like this:
>
> STATIC VOID
>>
>> ApplyVariables (
>>
>>   VOID
>>
>>   )
>>
>> {
>>
>> ...
>>
>>
>>>   if (FanOnGpio) {
>>
>>     DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
>>
>>     GpioPinFuncSet (FanOnGpio, GPIO_FSEL_OUTPUT);
>>
>>   }
>>
>>
>>>   /*
>>
>>    * Bluetooth pin muxing
>>
>>    */
>>
>>   GpioPinFuncSet (31, GPIO_FSEL_OUTPUT);
>>
>>   GpioPinSet (31, 0);
>>
>>
>>>   if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE)) {
>>
>>     GpioPinFuncSet (32, GPIO_FSEL_ALT3);
>>
>>     GpioPinFuncSet (33, GPIO_FSEL_ALT3);
>>
>>   } else {
>>
>>     GpioPinFuncSet (32, GPIO_FSEL_ALT5);
>>
>>     GpioPinFuncSet (33, GPIO_FSEL_ALT5);
>>
>>   }
>>
>> }
>>
>>
> --Mario
>
> În lun., 31 mai 2021 la 16:26, Sunny Wang <Sunny.Wang@arm.com> a scris:
>
>> Hi Mario,
>>
>>
>>
>> Thanks for checking this.
>>
>> Yeah, the problem is NO COM port for PL011 UART in Windows IOT’s device
>> manager, so I thought it doesn’t work. If there is no COM port for PL011
>> UART, how can I check PL011 UART’s functionality? How can I use a
>> telnet tool like PuTTY to send messages from RPi4 to my laptop? How to use
>> SerCx2? Is there a guideline for using PL011 UART on RPi4 with Windows IoT?
>>
>>
>>
>> As for why I mux both UARTs to the BT chip, I was trying to address your
>> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
>> it didn’t address your comment. No matter whether I add “PinFunction
>> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer,
>> , ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure
>> PL011 UART as the primary serial console. Bluetooth only works when I
>> configure Mini UART as the Primary serial console. I also confirmed that
>> this problem also exists with RPi4 Release FW 1.26 +
>> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
>> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
>> idea to make the Bluetooth work with configuring PL011 UART as the primary
>> serial console? By the way, it is good to know the loading sequence. Do you
>> know where I can quickly find this information without using WinDbg?
>>
>>
>>
>>
>>
>> Actually, the original code change (Patch 2/2) is the patch
>> (edk2-platforms-raspberrypi-pl011-bth-noflow.diff) on
>> https://github.com/worproject/RPi-Bluetooth-Testing/. I’m not familiar
>> with Windows drivers’ behavior, so I had no idea about why we need to
>> change them and couldn’t find more information about the changes’
>> background either. It looks like you know the details. Could you share with
>> me more information about the background of these changes? At least, I have
>> some questions below:
>>
>> 1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why
>> did we add this line in the beginning? I just removed it by your request
>> and according to the change in worproject/RPi-Bluetooth-Testing/, but I’m
>> still worried about if we did this for some other purposes.
>>
>> 2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using
>> GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth?
>> Where can I find the information?
>>
>> 3.  As for your comment about GPIO 31, I was confused, so I didn’t update
>> anything for addressing this comment. My understanding was that It is
>> intended to override the BCM_ALT2’s default setting from PullLow to
>> PullNone to fack the CTS signal. If we change it to PullLow, it will be
>> used for HW flow control, which is unwanted, isn’t it? What did you want me
>> to change? Could you directly paste the code change you wanted here for my
>> reference?
>>
>>
>>
>> Moreover, for the changes you want, could you also paste the code change?
>>
>>
>> Best Regards,
>>
>> Sunny Wang
>>
>>
>>
>> *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Mario
>> Balanica via groups.io
>> *Sent:* Monday, May 31, 2021 8:41 PM
>> *To:* Sunny Wang <Sunny.Wang@arm.com>
>> *Cc:* devel@edk2.groups.io; Samer El-Haj-Mahmoud <
>> Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>;
>> Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard
>> Biesheuvel <ardb+tianocore@kernel.org>
>> *Subject:* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable
>> Bluetooth and UART in Windows OS
>>
>>
>>
>> Hi Sunny,
>>
>>
>>
>> What issues are you seeing with the PL011 UART in Windows? Last time I
>> checked, it worked fine, and the fact that Bluetooth works also confirms
>> this.
>>
>> It won't show up as a COM port (like mini UART does) as it's built using
>> SerCx2.
>>
>>
>>
>> @@ -30,6 +30,12 @@ Device (URT0)
>>    {
>>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
>> BCM2836_PL011_UART_INTERRUPT }
>> +
>> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 32, 33 }
>> +
>>
>>
>>
>> @@ -79,6 +85,11 @@ Device (URTM)
>>      // from muxing the pins away.
>>
>>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
>> ResourceConsumer, , ) { 14, 15 }
>> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 32, 33 }
>> +
>>
>>
>>
>> What is the reason for trying to mux both UARTs to the BT chip? If PL011
>> is used for Bluetooth and the mini UART driver loads *after* it,
>> wouldn't it mux away the pins and break Bluetooth?
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com> a scris:
>>
>> This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
>> in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
>> Bluetooth and serial port (Mini UART) in Windows IOT.
>>
>> Note that PL011 UART still doesn't work with Windows 10 IOT with this
>> change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
>> Therefore, there should be no issue with PL011 UART related changes,
>> and we will still need a Windows expert to take a further look in the
>> future. Making PL011 UART work properly with Windows 10 IOT may require
>> additional changes to Windows driver or firmware's ACPI table.
>>
>> Testing Done:
>>   - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by
>> WOR) with
>>     the RPi-Windows-Drivers release ver 0.5 downloaded from
>>     https://github.com/worproject/RPi-Windows-Drivers/releases
>>     and checked that both Bluetooth and serial port (Mini UART) can
>>     work fine.
>>   - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
>>     console connection (PL011 UART).
>>
>> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Jeremy Linton <jeremy.linton@arm.com>
>> Cc: Pete Batard <pete@akeo.ie>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Mario Bălănică <mariobalanica02@gmail.com>
>> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
>> ---
>>  Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl
>> b/Platform/RaspberryPi/AcpiTables/Uart.asl
>> index bac9d791eb..cb99086d39 100644
>> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
>> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
>> @@ -30,6 +30,12 @@ Device (URT0)
>>    {
>>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
>> BCM2836_PL011_UART_INTERRUPT }
>> +
>> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 32, 33 }
>> +
>> +    // fake the CTS signal as we don't support HW flow control yet
>> +    // BCM_ALT2 is set as output (low) by default
>> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 31 }
>>    })
>>    Method (_CRS, 0x0, Serialized)
>>    {
>> @@ -79,6 +85,11 @@ Device (URTM)
>>      // from muxing the pins away.
>>
>>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
>> ResourceConsumer, , ) { 14, 15 }
>> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 32, 33 }
>> +
>> +    // fake the CTS signal as we don't support HW flow control yet
>> +    // BCM_ALT2 is set as output (low) by default
>> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
>> ResourceConsumer, , ) { 31 }
>>    })
>>    Method (_CRS, 0x0, Serialized)
>>    {
>> @@ -143,10 +154,6 @@ Device(BTH0)
>>        UAR0,          // DescriptorName: creates name
>>                      //   for offset of resource descriptor
>>      )                // Vendor data
>> -    //
>> -    // RPIQ connection for BT_ON/OFF
>> -    //
>> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
>> 0, ResourceConsumer, , ) { 128 }
>>    })
>>
>>    //
>> @@ -190,10 +197,6 @@ Device(BTH0)
>>        UARM,          // DescriptorName: creates name
>>                      //   for offset of resource descriptor
>>      )                // Vendor data
>> -    //
>> -    // RPIQ connection for BT_ON/OFF
>> -    //
>> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
>> 0, ResourceConsumer, , ) { 128 }
>>    })
>>
>>    Method (_CRS, 0x0, Serialized)
>> --
>> 2.31.0.windows.1
>>
>> 
>> IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose the
>> contents to any other person, use it for any purpose, or store or copy the
>> information in any medium. Thank you.
>>
>

[-- Attachment #2: Type: text/html, Size: 25321 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/2] Dynamically build UARTs info in ACPI
  2021-05-31  8:22 [PATCH v3 0/2] Dynamically build UARTs info in ACPI Sunny Wang
  2021-05-31  8:22 ` [PATCH v3 1/2] Platform/RaspberryPi: " Sunny Wang
  2021-05-31  8:22 ` [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
@ 2021-06-02 12:11 ` Samer El-Haj-Mahmoud
  2 siblings, 0 replies; 10+ messages in thread
From: Samer El-Haj-Mahmoud @ 2021-06-02 12:11 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Sunny Wang, Sami Mujawar, Jeremy Linton, Pete Batard,
	ardb+tianocore@kernel.org,
	Andrei Warkentin (awarkentin@vmware.com),
	Mario Bălănică, Samer El-Haj-Mahmoud

CCing the maintainers and reviewers

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sunny
> Wang via groups.io
> Sent: Monday, May 31, 2021 4:23 AM
> To: devel@edk2.groups.io
> Cc: Sunny Wang <Sunny.Wang@arm.com>
> Subject: [edk2-devel] [PATCH v3 0/2] Dynamically build UARTs info in ACPI
>
> In v3: Address comments given by Jeremy and Matio on v2.
> In v2: Address comments given by Pete on v1.
>
> Dynamically build UARTs info in ACPI so that it can match the UART
> related settings defined in config.txt
>
> Sunny Wang (2):
>   Platform/RaspberryPi: Dynamically build UARTs info in ACPI
>   Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
>
>  .../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      | 158 +++++++++++++-----
>  .../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, 413 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
>
> --
> 2.31.0.windows.1
>
>
>
> 
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-05-31 16:35         ` Mario Bălănică
@ 2021-06-07  8:31           ` Sunny Wang
  2021-06-07 14:58             ` Mario Bălănică
  0 siblings, 1 reply; 10+ messages in thread
From: Sunny Wang @ 2021-06-07  8:31 UTC (permalink / raw)
  To: devel@edk2.groups.io, mariobalanica02@gmail.com
  Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton, Pete Batard,
	Ard Biesheuvel, Sunny Wang

[-- Attachment #1: Type: text/plain, Size: 15818 bytes --]

Thanks for all the valuable comments, Mario. I learned a lot from your comments.
I just sent v4 for addressing your comments.

By the way, for samples/MinComm at develop · ms-iot/samples (github.com)<https://github.com/ms-iot/samples/tree/develop/MinComm>

  1.  I successfully build it. However, when I ran MinComm.exe on RPi4, I saw the “vcruntime140.dll was not found” error message, and couldn’t fix it by installing the latest Microsoft Visual C++ Redistributable (vc_redist.arm64.exe<https://aka.ms/vs/16/release/VC_redist.arm64.exe>) from https://support.microsoft.com/en-us/topic/the-latest-supported-visual-c-downloads-2647da03-1eea-4433-9aff-95f26a218cc0.
  2.  Just a suggestion. I think it would be good to upload the precompiled binary somewhere.


Best Regards,
Sunny Wang
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Mario Balanica via groups.io
Sent: Tuesday, June 1, 2021 12:36 AM
To: Sunny Wang <Sunny.Wang@arm.com>
Cc: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

edit: "GpioPinSet (31, 0);" should be "GpioPinSet (31, FALSE);" and maybe add the "fake the CTS signal as we don't support HW flow control yet" comment too.

When this patch gets merged, I'll add my Bluetooth changes to raspberrypi/windows-drivers: Windows IOT drivers (github.com)<https://github.com/raspberrypi/windows-drivers> and submit another patch here to enable hardware flow control.

--Mario

În lun., 31 mai 2021 la 18:56, Mario Bălănică <mariobalanica02@gmail.com<mailto:mariobalanica02@gmail.com>> a scris:
If there is no COM port for PL011 UART, how can I check PL011 UART’s functionality? How can I use a telnet tool like PuTTY to send messages from RPi4 to my laptop?

You can use: samples/MinComm at develop · ms-iot/samples (github.com)<https://github.com/ms-iot/samples/tree/develop/MinComm> (let me know if you need a precompiled binary)

As for why I mux both UARTs to the BT chip, I was trying to address your comment “This always assumes that PL011 is used for Bluetooth”. Apparently, it didn’t address your comment. No matter whether I add “PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011 UART as the primary serial console. Bluetooth only works when I configure Mini UART as the Primary serial console. I also confirmed that this problem also exists with RPi4 Release FW 1.26 + edk2-platforms-raspberrypi-pl011-bth-noflow.diff in https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any idea to make the Bluetooth work with configuring PL011 UART as the primary serial console?

Bluetooth doesn't really work with the mini UART driver at the moment. The chip stalls after a few transfers, as far as I remember. Haven't looked much into it.

By the way, it is good to know the loading sequence. Do you know where I can quickly find this information without using WinDbg?

I'm not aware of any way to see the driver loading order without WinDbg. Also, verbose mode must be enabled before boot: View Verbose Output - Windows drivers | Microsoft Docs<https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/view---verbose-output>

You could also test if Bluetooth will break by uninstalling / re-installing the mini UART driver while the system is running, then try to scan for nearby BT devices.

1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did we add this line in the beginning? I just removed it by your request and according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still worried about if we did this for some other purposes.

That is a leftover from the MS-IOT tables. It tries to toggle BT_ON from the GPIO expander (I2C) through RPIQ (mailbox), but the driver has no knowledge of it. And since the firmware blob turns on Bluetooth by default, the line is completely useless.

We may want to have something like this in the future, for power management of the BT chip (will require some driver changes too).

2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth? Where can I find the information?

Yes, the Bluetooth chip is available on pins 32, 33 (TX / RX). Also 30, 31 for hardware flow control (CTS / RTS). More info can be found in the BCM2711 datasheet<https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf> (5.3. Alternative Function Assignments)

3.  As for your comment about GPIO 31, I was confused, so I didn’t update anything for addressing this comment. My understanding was that It is intended to override the BCM_ALT2’s default setting from PullLow to PullNone to fack the CTS signal. If we change it to PullLow, it will be used for HW flow control, which is unwanted, isn’t it? What did you want me to change? Could you directly paste the code change you wanted here for my reference?

The BCM_ALT2 thing is a dirty hack. Pin 31 must be held LOW so that we can talk to the BT chip without flow control. It seems this can't be described with PinFunction, so I've relied on the default value of ALT2 for pin 31 (which turns out to be always LOW).

My proposal is to move the pin muxing stuff in ConfigDxe, like this:

STATIC VOID
ApplyVariables (
  VOID
  )
{
...

  if (FanOnGpio) {
    DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
    GpioPinFuncSet (FanOnGpio, GPIO_FSEL_OUTPUT);
  }

  /*
   * Bluetooth pin muxing
   */
  GpioPinFuncSet (31, GPIO_FSEL_OUTPUT);
  GpioPinSet (31, 0);

  if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE)) {
    GpioPinFuncSet (32, GPIO_FSEL_ALT3);
    GpioPinFuncSet (33, GPIO_FSEL_ALT3);
  } else {
    GpioPinFuncSet (32, GPIO_FSEL_ALT5);
    GpioPinFuncSet (33, GPIO_FSEL_ALT5);
  }
}

--Mario

În lun., 31 mai 2021 la 16:26, Sunny Wang <Sunny.Wang@arm.com<mailto:Sunny.Wang@arm.com>> a scris:
Hi Mario,

Thanks for checking this.
Yeah, the problem is NO COM port for PL011 UART in Windows IOT’s device manager, so I thought it doesn’t work. If there is no COM port for PL011 UART, how can I check PL011 UART’s functionality? How can I use a telnet tool like PuTTY to send messages from RPi4 to my laptop? How to use SerCx2? Is there a guideline for using PL011 UART on RPi4 with Windows IoT?

As for why I mux both UARTs to the BT chip, I was trying to address your comment “This always assumes that PL011 is used for Bluetooth”. Apparently, it didn’t address your comment. No matter whether I add “PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011 UART as the primary serial console. Bluetooth only works when I configure Mini UART as the Primary serial console. I also confirmed that this problem also exists with RPi4 Release FW 1.26 + edk2-platforms-raspberrypi-pl011-bth-noflow.diff in https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any idea to make the Bluetooth work with configuring PL011 UART as the primary serial console? By the way, it is good to know the loading sequence. Do you know where I can quickly find this information without using WinDbg?


Actually, the original code change (Patch 2/2) is the patch (edk2-platforms-raspberrypi-pl011-bth-noflow.diff) on  https://github.com/worproject/RPi-Bluetooth-Testing/. I’m not familiar with Windows drivers’ behavior, so I had no idea about why we need to change them and couldn’t find more information about the changes’ background either. It looks like you know the details. Could you share with me more information about the background of these changes? At least, I have some questions below:
1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did we add this line in the beginning? I just removed it by your request and according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still worried about if we did this for some other purposes.
2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth? Where can I find the information?
3.  As for your comment about GPIO 31, I was confused, so I didn’t update anything for addressing this comment. My understanding was that It is intended to override the BCM_ALT2’s default setting from PullLow to PullNone to fack the CTS signal. If we change it to PullLow, it will be used for HW flow control, which is unwanted, isn’t it? What did you want me to change? Could you directly paste the code change you wanted here for my reference?

Moreover, for the changes you want, could you also paste the code change?

Best Regards,
Sunny Wang

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Mario Balanica via groups.io<http://groups.io>
Sent: Monday, May 31, 2021 8:41 PM
To: Sunny Wang <Sunny.Wang@arm.com<mailto:Sunny.Wang@arm.com>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>; Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; Jeremy Linton <Jeremy.Linton@arm.com<mailto:Jeremy.Linton@arm.com>>; Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb%2Btianocore@kernel.org>>
Subject: Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

Hi Sunny,

What issues are you seeing with the PL011 UART in Windows? Last time I checked, it worked fine, and the fact that Bluetooth works also confirms this.
It won't show up as a COM port (like mini UART does) as it's built using SerCx2.

@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+

@@ -79,6 +85,11 @@ Device (URTM)
     // from muxing the pins away.

     // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0<file://_SB.GPI0>", 0, ResourceConsumer, , ) { 14, 15 }
+    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+

What is the reason for trying to mux both UARTs to the BT chip? If PL011 is used for Bluetooth and the mini UART driver loads after it, wouldn't it mux away the pins and break Bluetooth?




În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com<mailto:Sunny.Wang@arm.com>> a scris:
This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows IOT.

Note that PL011 UART still doesn't work with Windows 10 IOT with this
change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
Therefore, there should be no issue with PL011 UART related changes,
and we will still need a Windows expert to take a further look in the
future. Making PL011 UART work properly with Windows 10 IOT may require
additional changes to Windows driver or firmware's ACPI table.

Testing Done:
  - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by WOR) with
    the RPi-Windows-Drivers release ver 0.5 downloaded from
    https://github.com/worproject/RPi-Windows-Drivers/releases
    and checked that both Bluetooth and serial port (Mini UART) can
    work fine.
  - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
    console connection (PL011 UART).

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com<mailto:samer.el-haj-mahmoud@arm.com>>
Cc: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
Cc: Jeremy Linton <jeremy.linton@arm.com<mailto:jeremy.linton@arm.com>>
Cc: Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb%2Btianocore@kernel.org>>
Cc: Mario Bălănică <mariobalanica02@gmail.com<mailto:mariobalanica02@gmail.com>>
Signed-off-by: Sunny Wang <sunny.wang@arm.com<mailto:sunny.wang@arm.com>>
---
 Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
index bac9d791eb..cb99086d39 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
   {
     MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
     Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_PL011_UART_INTERRUPT }
+
+    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -79,6 +85,11 @@ Device (URTM)
     // from muxing the pins away.

     // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0<file://_SB.GPI0>", 0, ResourceConsumer, , ) { 14, 15 }
+    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 32, 33 }
+
+    // fake the CTS signal as we don't support HW flow control yet
+    // BCM_ALT2 is set as output (low) by default
+    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0<file://_SB.GDV0.GPI0>", 0, ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -143,10 +154,6 @@ Device(BTH0)
       UAR0,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ<file://_SB.GDV0.RPIQ>", 0, ResourceConsumer, , ) { 128 }
   })

   //
@@ -190,10 +197,6 @@ Device(BTH0)
       UARM,          // DescriptorName: creates name
                     //   for offset of resource descriptor
     )                // Vendor data
-    //
-    // RPIQ connection for BT_ON/OFF
-    //
-    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ<file://_SB.GDV0.RPIQ>", 0, ResourceConsumer, , ) { 128 }
   })

   Method (_CRS, 0x0, Serialized)
--
2.31.0.windows.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 38822 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS
  2021-06-07  8:31           ` Sunny Wang
@ 2021-06-07 14:58             ` Mario Bălănică
  0 siblings, 0 replies; 10+ messages in thread
From: Mario Bălănică @ 2021-06-07 14:58 UTC (permalink / raw)
  To: Sunny Wang
  Cc: devel@edk2.groups.io, Samer El-Haj-Mahmoud, Sami Mujawar,
	Jeremy Linton, Pete Batard, Ard Biesheuvel

[-- Attachment #1: Type: text/plain, Size: 16399 bytes --]

This version of MiniComm will work without vcredist (compiled with /MT):
https://drive.google.com/file/d/1v04RHUEIqlhrg2TUU_d_G4Z6lbZ1l44g/view?usp=sharing

--Mario

În lun., 7 iun. 2021 la 11:31, Sunny Wang <Sunny.Wang@arm.com> a scris:

> Thanks for all the valuable comments, Mario. I learned a lot from your
> comments.
>
> I just sent v4 for addressing your comments.
>
>
>
> By the way, for samples/MinComm at develop · ms-iot/samples (github.com)
> <https://github.com/ms-iot/samples/tree/develop/MinComm>
>
>    1. I successfully build it. However, when I ran MinComm.exe on RPi4, I
>    saw the “vcruntime140.dll was not found” error message, and couldn’t fix it
>    by installing the latest Microsoft Visual C++ Redistributable (
>    vc_redist.arm64.exe <https://aka.ms/vs/16/release/VC_redist.arm64.exe>)
>    from
>    https://support.microsoft.com/en-us/topic/the-latest-supported-visual-c-downloads-2647da03-1eea-4433-9aff-95f26a218cc0.
>
>    2. Just a suggestion. I think it would be good to upload the
>    precompiled binary somewhere.
>
>
>
>
> Best Regards,
>
> Sunny Wang
>
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> * On Behalf Of *Mario
> Balanica via groups.io
> *Sent:* Tuesday, June 1, 2021 12:36 AM
> *To:* Sunny Wang <Sunny.Wang@arm.com>
> *Cc:* devel@edk2.groups.io; Samer El-Haj-Mahmoud <
> Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>;
> Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>
> *Subject:* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable
> Bluetooth and UART in Windows OS
>
>
>
> edit: "GpioPinSet (31, 0);" should be "GpioPinSet (31, FALSE);" and maybe
> add the "fake the CTS signal as we don't support HW flow control yet"
> comment too.
>
>
>
> When this patch gets merged, I'll add my Bluetooth changes to raspberrypi/windows-drivers:
> Windows IOT drivers (github.com)
> <https://github.com/raspberrypi/windows-drivers> and submit another patch
> here to enable hardware flow control.
>
>
>
> --Mario
>
>
>
> În lun., 31 mai 2021 la 18:56, Mario Bălănică <mariobalanica02@gmail.com>
> a scris:
>
> If there is no COM port for PL011 UART, how can I check PL011 UART’s
> functionality? How can I use a telnet tool like PuTTY to send messages from
> RPi4 to my laptop?
>
>
>
> You can use: samples/MinComm at develop · ms-iot/samples (github.com)
> <https://github.com/ms-iot/samples/tree/develop/MinComm> (let me know if
> you need a precompiled binary)
>
>
>
> As for why I mux both UARTs to the BT chip, I was trying to address your
> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
> it didn’t address your comment. No matter whether I add “PinFunction
> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, ,
> ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011
> UART as the primary serial console. Bluetooth only works when I configure
> Mini UART as the Primary serial console. I also confirmed that this problem
> also exists with RPi4 Release FW 1.26 +
> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
> idea to make the Bluetooth work with configuring PL011 UART as the primary
> serial console?
>
>
>
> Bluetooth doesn't really work with the mini UART driver at the moment. The
> chip stalls after a few transfers, as far as I remember. Haven't looked
> much into it.
>
>
>
> By the way, it is good to know the loading sequence. Do you know where I
> can quickly find this information without using WinDbg?
>
>
>
> I'm not aware of any way to see the driver loading order without WinDbg.
> Also, verbose mode must be enabled before boot: View Verbose Output -
> Windows drivers | Microsoft Docs
> <https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/view---verbose-output>
>
>
>
> You could also test if Bluetooth will break by uninstalling /
> re-installing the mini UART driver while the system is running, then try to
> scan for nearby BT devices.
>
>
>
> 1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did
> we add this line in the beginning? I just removed it by your request and
> according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still
> worried about if we did this for some other purposes.
>
>
>
> That is a leftover from the MS-IOT tables. It tries to toggle BT_ON from
> the GPIO expander (I2C) through RPIQ (mailbox), but the driver has no
> knowledge of it. And since the firmware blob turns on Bluetooth by default,
> the line is completely useless.
>
>
>
> We may want to have something like this in the future, for power
> management of the BT chip (will require some driver changes too).
>
>
>
> 2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using
> GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth?
> Where can I find the information?
>
>
>
> Yes, the Bluetooth chip is available on pins 32, 33 (TX / RX). Also 30, 31
> for hardware flow control (CTS / RTS). More info can be found in the BCM2711
> datasheet
> <https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf> (5.3.
> Alternative Function Assignments)
>
>
>
> 3.  As for your comment about GPIO 31, I was confused, so I didn’t update
> anything for addressing this comment. My understanding was that It is
> intended to override the BCM_ALT2’s default setting from PullLow to
> PullNone to fack the CTS signal. If we change it to PullLow, it will be
> used for HW flow control, which is unwanted, isn’t it? What did you want me
> to change? Could you directly paste the code change you wanted here for my
> reference?
>
>
>
> The BCM_ALT2 thing is a dirty hack. Pin 31 must be held LOW so that we can
> talk to the BT chip without flow control. It seems this can't be described
> with PinFunction, so I've relied on the default value of ALT2 for pin 31
> (which turns out to be always LOW).
>
>
>
> My proposal is to move the pin muxing stuff in ConfigDxe, like this:
>
>
>
> STATIC VOID
>
> ApplyVariables (
>
>   VOID
>
>   )
>
> {
>
> ...
>
>
>
>   if (FanOnGpio) {
>
>     DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
>
>     GpioPinFuncSet (FanOnGpio, GPIO_FSEL_OUTPUT);
>
>   }
>
>
>
>   /*
>
>    * Bluetooth pin muxing
>
>    */
>
>   GpioPinFuncSet (31, GPIO_FSEL_OUTPUT);
>
>   GpioPinSet (31, 0);
>
>
>
>   if ((PcdGet32 (PcdUartInUse) == PL011_UART_IN_USE)) {
>
>     GpioPinFuncSet (32, GPIO_FSEL_ALT3);
>
>     GpioPinFuncSet (33, GPIO_FSEL_ALT3);
>
>   } else {
>
>     GpioPinFuncSet (32, GPIO_FSEL_ALT5);
>
>     GpioPinFuncSet (33, GPIO_FSEL_ALT5);
>
>   }
>
> }
>
>
>
> --Mario
>
>
>
> În lun., 31 mai 2021 la 16:26, Sunny Wang <Sunny.Wang@arm.com> a scris:
>
> Hi Mario,
>
>
>
> Thanks for checking this.
>
> Yeah, the problem is NO COM port for PL011 UART in Windows IOT’s device
> manager, so I thought it doesn’t work. If there is no COM port for PL011
> UART, how can I check PL011 UART’s functionality? How can I use a
> telnet tool like PuTTY to send messages from RPi4 to my laptop? How to use
> SerCx2? Is there a guideline for using PL011 UART on RPi4 with Windows IoT?
>
>
>
> As for why I mux both UARTs to the BT chip, I was trying to address your
> comment “This always assumes that PL011 is used for Bluetooth”. Apparently,
> it didn’t address your comment. No matter whether I add “PinFunction
> (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, ,
> ) { 32, 33 }” or not, the Bluetooth has NOT worked when I configure PL011
> UART as the primary serial console. Bluetooth only works when I configure
> Mini UART as the Primary serial console. I also confirmed that this problem
> also exists with RPi4 Release FW 1.26 +
> edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
> https://github.com/worproject/RPi-Bluetooth-Testing/. Do you have any
> idea to make the Bluetooth work with configuring PL011 UART as the primary
> serial console? By the way, it is good to know the loading sequence. Do you
> know where I can quickly find this information without using WinDbg?
>
>
>
>
>
> Actually, the original code change (Patch 2/2) is the patch
> (edk2-platforms-raspberrypi-pl011-bth-noflow.diff) on
> https://github.com/worproject/RPi-Bluetooth-Testing/. I’m not familiar
> with Windows drivers’ behavior, so I had no idea about why we need to
> change them and couldn’t find more information about the changes’
> background either. It looks like you know the details. Could you share with
> me more information about the background of these changes? At least, I have
> some questions below:
>
> 1.  What is the connection between GPIO 128 and Bluetooth (BT_ON)? Why did
> we add this line in the beginning? I just removed it by your request and
> according to the change in worproject/RPi-Bluetooth-Testing/, but I’m still
> worried about if we did this for some other purposes.
>
> 2.  Why do we need to configure GPIO 32 and 33? Is the Bluetooth using
> GPIO 32 and 33? Or Is Windows OS checking GPIO 32 and 33 for Bluetooth?
> Where can I find the information?
>
> 3.  As for your comment about GPIO 31, I was confused, so I didn’t update
> anything for addressing this comment. My understanding was that It is
> intended to override the BCM_ALT2’s default setting from PullLow to
> PullNone to fack the CTS signal. If we change it to PullLow, it will be
> used for HW flow control, which is unwanted, isn’t it? What did you want me
> to change? Could you directly paste the code change you wanted here for my
> reference?
>
>
>
> Moreover, for the changes you want, could you also paste the code change?
>
>
> Best Regards,
>
> Sunny Wang
>
>
>
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Mario
> Balanica via groups.io
> *Sent:* Monday, May 31, 2021 8:41 PM
> *To:* Sunny Wang <Sunny.Wang@arm.com>
> *Cc:* devel@edk2.groups.io; Samer El-Haj-Mahmoud <
> Samer.El-Haj-Mahmoud@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>;
> Jeremy Linton <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>
> *Subject:* Re: [edk2-devel] [PATCH v3 2/2] Platform/RaspberryPi: Enable
> Bluetooth and UART in Windows OS
>
>
>
> Hi Sunny,
>
>
>
> What issues are you seeing with the PL011 UART in Windows? Last time I
> checked, it worked fine, and the fact that Bluetooth works also confirms
> this.
>
> It won't show up as a COM port (like mini UART does) as it's built using
> SerCx2.
>
>
>
> @@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
>
>
>
> @@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
>
>
>
> What is the reason for trying to mux both UARTs to the BT chip? If PL011
> is used for Bluetooth and the mini UART driver loads *after* it, wouldn't
> it mux away the pins and break Bluetooth?
>
>
>
>
>
>
>
>
>
> În lun., 31 mai 2021 la 11:23, Sunny Wang <Sunny.Wang@arm.com> a scris:
>
> This change is based on edk2-platforms-raspberrypi-pl011-bth-noflow.diff
> in https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
> Bluetooth and serial port (Mini UART) in Windows IOT.
>
> Note that PL011 UART still doesn't work with Windows 10 IOT with this
> change, but PL011 UART works fine with VMware ESXi-Arm Fling v1.3.
> Therefore, there should be no issue with PL011 UART related changes,
> and we will still need a Windows expert to take a further look in the
> future. Making PL011 UART work properly with Windows 10 IOT may require
> additional changes to Windows driver or firmware's ACPI table.
>
> Testing Done:
>   - Successfully booted Windows Windows 10 IOT (20279.1) on SD (made by
> WOR) with
>     the RPi-Windows-Drivers release ver 0.5 downloaded from
>     https://github.com/worproject/RPi-Windows-Drivers/releases
>     and checked that both Bluetooth and serial port (Mini UART) can
>     work fine.
>   - Successfully booted VMware ESXi-Arm Fling v1.3 with only serial
>     console connection (PL011 UART).
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Mario Bălănică <mariobalanica02@gmail.com>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Uart.asl | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl
> b/Platform/RaspberryPi/AcpiTables/Uart.asl
> index bac9d791eb..cb99086d39 100644
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> @@ -30,6 +30,12 @@ Device (URT0)
>    {
>      MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
>      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
> BCM2836_PL011_UART_INTERRUPT }
> +
> +    PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -79,6 +85,11 @@ Device (URTM)
>      // from muxing the pins away.
>
>      // PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GPI0", 0,
> ResourceConsumer, , ) { 14, 15 }
> +    PinFunction (Exclusive, PullDown, BCM_ALT5, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 32, 33 }
> +
> +    // fake the CTS signal as we don't support HW flow control yet
> +    // BCM_ALT2 is set as output (low) by default
> +    PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0,
> ResourceConsumer, , ) { 31 }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -143,10 +154,6 @@ Device(BTH0)
>        UAR0,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    //
> @@ -190,10 +197,6 @@ Device(BTH0)
>        UARM,          // DescriptorName: creates name
>                      //   for offset of resource descriptor
>      )                // Vendor data
> -    //
> -    // RPIQ connection for BT_ON/OFF
> -    //
> -    GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ",
> 0, ResourceConsumer, , ) { 128 }
>    })
>
>    Method (_CRS, 0x0, Serialized)
> --
> 2.31.0.windows.1
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>

[-- Attachment #2: Type: text/html, Size: 34498 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-06-07 14:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-31  8:22 [PATCH v3 0/2] Dynamically build UARTs info in ACPI Sunny Wang
2021-05-31  8:22 ` [PATCH v3 1/2] Platform/RaspberryPi: " Sunny Wang
2021-05-31  8:22 ` [PATCH v3 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
2021-05-31 12:40   ` Mario Bălănică
2021-05-31 13:26     ` [edk2-devel] " Sunny Wang
2021-05-31 15:56       ` Mario Bălănică
2021-05-31 16:35         ` Mario Bălănică
2021-06-07  8:31           ` Sunny Wang
2021-06-07 14:58             ` Mario Bălănică
2021-06-02 12:11 ` [edk2-devel] [PATCH v3 0/2] Dynamically build UARTs info in ACPI Samer El-Haj-Mahmoud

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox