[edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation

Pete Batard posted 6 patches 6 years, 1 month ago
There is a newer version of this series
[edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation
Posted by Pete Batard 6 years, 1 month ago
Use code derived from JunoPkg to generate our serial tables and
also use PCDs where possible.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +-
 Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 +++++++++++++++++---
 Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ----------
 Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 ++++++++++++++++++
 4 files changed, 183 insertions(+), 63 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
index 50c9f7694d84..6b1155d66444 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
@@ -31,7 +31,7 @@ [Sources]
   Gtdt.aslc
   Dsdt.asl
   Csrt.aslc
-  Spcr.asl
+  Spcr.aslc
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -47,4 +47,6 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
   gArmTokenSpaceGuid.PcdGicDistributorBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
   gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
index 849cf5134793..589a5c2cbd71 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
@@ -2,27 +2,99 @@
  *
  *  Debug Port Table (DBG2)
  *
- *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
+ *  Copyright (c) 2012-2016, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
  **/
 
-UINT8 Dbg2[92] = {
-  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00,
-  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32,
-  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53,
-  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00,
-  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00,
-  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00,
-  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10,
-  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00,
-  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T',
-  'M', 0x00,
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/DebugPort2Table.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+#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             10
+
+//
+// When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
+// which is offset by 0x40 from the actual Bcm2835 base address
+//
+#define RPI_UART_BASE_ADDRESS                           (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
+#define RPI_UART_LENGTH                                 0x70
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+typedef struct {
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
+  EFI_ACPI_5_1_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 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_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
+    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
+    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
+  }
+
+
+STATIC DBG2_TABLE Dbg2 = {
+  {
+    ACPI_HEADER (
+      EFI_ACPI_5_1_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,
+      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
+      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
+// 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/RPi4/AcpiTables/Spcr.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl
deleted file mode 100644
index 4632a4f193e7..000000000000
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl
+++ /dev/null
@@ -1,48 +0,0 @@
-/** @file
- *
- *  Serial Port Console Redirection Table (SPCR)
- *
- *  Copyright (c) 2019, ARM Ltd. All rights reserved.
- *  Copyright (c) 2017-2018, Andrey Warkentin <andrey.warkentin@gmail.com>
- *
- *  SPDX-License-Identifier: BSD-2-Clause-Patent
- *
- **/
-
-[000h 0000   4]                    Signature : "SPCR"    [Serial Port Console Redirection table]
-[004h 0004   4]                 Table Length : 00000050
-[008h 0008   1]                     Revision : 02
-[009h 0009   1]                     Checksum : 00
-[00Ah 0010   6]                       Oem ID : "RPiEFI"
-[010h 0016   8]                 Oem Table ID : "RPi4UEFI"
-[018h 0024   4]                 Oem Revision : 00000001
-[01Ch 0028   4]              Asl Compiler ID : "----"
-[020h 0032   4]        Asl Compiler Revision : 00000000
-
-[024h 0036   1]               Interface Type : 10     // 0x03 = PL011, 0x10 = BCM2835
-[025h 0037   3]                     Reserved : 000000
-
-[028h 0040  12]         Serial Port Register : [Generic Address Structure]
-[028h 0040   1]                     Space ID : 00 [SystemMemory]
-[029h 0041   1]                    Bit Width : 20
-[02Ah 0042   1]                   Bit Offset : 00
-[02Bh 0043   1]         Encoded Access Width : 03 [DWord Access:32]
-[02Ch 0044   8]                      Address : FE215000
-
-[034h 0052   1]               Interrupt Type : 08     // ARMH GIC interrupt
-[035h 0053   1]          PCAT-compatible IRQ : 00
-[036h 0054   4]                    Interrupt : 7D
-[03Ah 0058   1]                    Baud Rate : 07     // 115200
-[03Bh 0059   1]                       Parity : 00
-[03Ch 0060   1]                    Stop Bits : 01
-[03Dh 0061   1]                 Flow Control : 00
-[03Eh 0062   1]                Terminal Type : 02     // VT-UTF8
-[04Ch 0076   1]                     Reserved : 00
-[040h 0064   2]                PCI Device ID : FFFF
-[042h 0066   2]                PCI Vendor ID : FFFF
-[044h 0068   1]                      PCI Bus : 00
-[045h 0069   1]                   PCI Device : 00
-[046h 0070   1]                 PCI Function : 00
-[047h 0071   4]                    PCI Flags : 00000000
-[04Bh 0075   1]                  PCI Segment : 00
-[04Ch 0076   4]                     Reserved : 00000000
diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
new file mode 100644
index 000000000000..65f48ceae688
--- /dev/null
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
@@ -0,0 +1,94 @@
+/** @file
+* SPCR Table
+*
+* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
+* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
+#include <Library/AcpiLib.h>
+#include <Library/PcdLib.h>
+
+#include "AcpiTables.h"
+
+
+#define RPI_UART_FLOW_CONTROL_NONE           0
+
+//
+// When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
+// which is offset by 0x40 from the actual Bcm2835 base address
+//
+#define RPI_UART_BASE_ADDRESS                (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
+#define RPI_UART_INTERRUPT                   0x7D
+
+STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
+  ACPI_HEADER (
+    EFI_ACPI_5_1_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
+    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
+  ),
+  // UINT8                                   InterfaceType;
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
+  // UINT8                                   Reserved1[3];
+  {
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE,
+    EFI_ACPI_RESERVED_BYTE
+  },
+  // EFI_ACPI_5_1_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;
-- 
2.21.0.windows.1


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

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

Re: [edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation
Posted by Philippe Mathieu-Daudé 6 years, 1 month ago
On 12/18/19 12:41 PM, Pete Batard wrote:
> Use code derived from JunoPkg to generate our serial tables and
> also use PCDs where possible.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>   Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +-
>   Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 +++++++++++++++++---
>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ----------
>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 ++++++++++++++++++
>   4 files changed, 183 insertions(+), 63 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
> index 50c9f7694d84..6b1155d66444 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
> @@ -31,7 +31,7 @@ [Sources]
>     Gtdt.aslc
>     Dsdt.asl
>     Csrt.aslc
> -  Spcr.asl
> +  Spcr.aslc
>   
>   [Packages]
>     ArmPkg/ArmPkg.dec
> @@ -47,4 +47,6 @@ [FixedPcd]
>     gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>     gArmTokenSpaceGuid.PcdGicDistributorBase
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>     gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
> index 849cf5134793..589a5c2cbd71 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
> @@ -2,27 +2,99 @@
>    *
>    *  Debug Port Table (DBG2)
>    *
> - *  Copyright (c) Microsoft Corporation. All rights reserved.
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> + *  Copyright (c) 2012-2016, ARM Limited. All rights reserved.
>    *
>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
>    *
>    **/
>   
> -UINT8 Dbg2[92] = {
> -  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00,
> -  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32,
> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53,
> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00,
> -  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00,
> -  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00,
> -  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10,
> -  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00,
> -  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T',
> -  'M', 0x00,
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/DebugPort2Table.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include "AcpiTables.h"
> +
> +#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             10
> +
> +//
> +// When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
> +// which is offset by 0x40 from the actual Bcm2835 base address

Actually this is the other way around, the 8250 is at AUX + 0x40.

Can we clean that up or is it too late?

> +//
> +#define RPI_UART_BASE_ADDRESS                           (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
> +#define RPI_UART_LENGTH                                 0x70
> +//
> +// RPI_UART_STR should match the value used Uart.asl
> +//
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
> +
> +typedef struct {
> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> +  EFI_ACPI_5_1_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 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_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> +  }
> +
> +
> +STATIC DBG2_TABLE Dbg2 = {
> +  {
> +    ACPI_HEADER (
> +      EFI_ACPI_5_1_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,
> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
> +      RPI_UART_BASE_ADDRESS,
> +      RPI_UART_LENGTH,
> +      RPI_UART_STR
> +    ),
> +  }
>   };
[...]


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

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

Re: [edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation
Posted by Pete Batard 6 years, 1 month ago
Hi Philippe,

On 2019.12.18 15:57, Philippe Mathieu-Daudé wrote:
> On 12/18/19 12:41 PM, Pete Batard wrote:
>> Use code derived from JunoPkg to generate our serial tables and
>> also use PCDs where possible.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>>   Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +-
>>   Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 
>> +++++++++++++++++---
>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ----------
>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 
>> ++++++++++++++++++
>>   4 files changed, 183 insertions(+), 63 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf 
>> b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>> index 50c9f7694d84..6b1155d66444 100644
>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>> @@ -31,7 +31,7 @@ [Sources]
>>     Gtdt.aslc
>>     Dsdt.asl
>>     Csrt.aslc
>> -  Spcr.asl
>> +  Spcr.aslc
>>   [Packages]
>>     ArmPkg/ArmPkg.dec
>> @@ -47,4 +47,6 @@ [FixedPcd]
>>     gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
>>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>>     gArmTokenSpaceGuid.PcdGicDistributorBase
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>>     gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc 
>> b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>> index 849cf5134793..589a5c2cbd71 100644
>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>> @@ -2,27 +2,99 @@
>>    *
>>    *  Debug Port Table (DBG2)
>>    *
>> - *  Copyright (c) Microsoft Corporation. All rights reserved.
>> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
>> + *  Copyright (c) 2012-2016, ARM Limited. All rights reserved.
>>    *
>>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
>>    *
>>    **/
>> -UINT8 Dbg2[92] = {
>> -  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00,
>> -  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32,
>> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53,
>> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00,
>> -  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00,
>> -  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00,
>> -  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10,
>> -  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00,
>> -  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T',
>> -  'M', 0x00,
>> +#include <IndustryStandard/Acpi.h>
>> +#include <IndustryStandard/DebugPort2Table.h>
>> +#include <Library/AcpiLib.h>
>> +#include <Library/PcdLib.h>
>> +
>> +#include "AcpiTables.h"
>> +
>> +#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             10
>> +
>> +//
>> +// When using the miniUART, PcdSerialRegisterBase points to the 8250 
>> base address,
>> +// which is offset by 0x40 from the actual Bcm2835 base address
> 
> Actually this is the other way around, the 8250 is at AUX + 0x40.
> 
> Can we clean that up or is it too late?

Yeah, re-reading this I admit the comment is not too clear.

Let me try to rephrase this better in v2.

Regards,

/Pete

> 
>> +//
>> +#define RPI_UART_BASE_ADDRESS                           
>> (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
>> +#define RPI_UART_LENGTH                                 0x70
>> +//
>> +// RPI_UART_STR should match the value used Uart.asl
>> +//
>> +#define RPI_UART_STR                                    { '\\', '_', 
>> 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
>> +
>> +typedef struct {
>> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
>> +  EFI_ACPI_5_1_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 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_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
>> +    UartAddrLen,                                     /* UINT32  
>> AddressSize */                                        \
>> +    UartNameStr                                      /* UINT8   
>> NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
>> +  }
>> +
>> +
>> +STATIC DBG2_TABLE Dbg2 = {
>> +  {
>> +    ACPI_HEADER (
>> +      EFI_ACPI_5_1_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,
>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
>> +      RPI_UART_BASE_ADDRESS,
>> +      RPI_UART_LENGTH,
>> +      RPI_UART_STR
>> +    ),
>> +  }
>>   };
> [...]
> 


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

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

Re: [edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation
Posted by Philippe Mathieu-Daudé 6 years, 1 month ago
On 12/18/19 5:36 PM, Pete Batard wrote:
> Hi Philippe,
> 
> On 2019.12.18 15:57, Philippe Mathieu-Daudé wrote:
>> On 12/18/19 12:41 PM, Pete Batard wrote:
>>> Use code derived from JunoPkg to generate our serial tables and
>>> also use PCDs where possible.
>>>
>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>> ---
>>>   Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +-
>>>   Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 
>>> +++++++++++++++++---
>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ----------
>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 
>>> ++++++++++++++++++
>>>   4 files changed, 183 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf 
>>> b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>> index 50c9f7694d84..6b1155d66444 100644
>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>> @@ -31,7 +31,7 @@ [Sources]
>>>     Gtdt.aslc
>>>     Dsdt.asl
>>>     Csrt.aslc
>>> -  Spcr.asl
>>> +  Spcr.aslc
>>>   [Packages]
>>>     ArmPkg/ArmPkg.dec
>>> @@ -47,4 +47,6 @@ [FixedPcd]
>>>     gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
>>>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>>>     gArmTokenSpaceGuid.PcdGicDistributorBase
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>>>     gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc 
>>> b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>> index 849cf5134793..589a5c2cbd71 100644
>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>> @@ -2,27 +2,99 @@
>>>    *
>>>    *  Debug Port Table (DBG2)
>>>    *
>>> - *  Copyright (c) Microsoft Corporation. All rights reserved.
>>> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
>>> + *  Copyright (c) 2012-2016, ARM Limited. All rights reserved.
>>>    *
>>>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>    *
>>>    **/
>>> -UINT8 Dbg2[92] = {
>>> -  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> -  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32,
>>> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53,
>>> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00,
>>> -  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00,
>>> -  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00,
>>> -  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10,
>>> -  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00,
>>> -  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T',
>>> -  'M', 0x00,
>>> +#include <IndustryStandard/Acpi.h>
>>> +#include <IndustryStandard/DebugPort2Table.h>
>>> +#include <Library/AcpiLib.h>
>>> +#include <Library/PcdLib.h>
>>> +
>>> +#include "AcpiTables.h"
>>> +
>>> +#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             10
>>> +
>>> +//
>>> +// When using the miniUART, PcdSerialRegisterBase points to the 8250 
>>> base address,
>>> +// which is offset by 0x40 from the actual Bcm2835 base address
>>
>> Actually this is the other way around, the 8250 is at AUX + 0x40.
>>
>> Can we clean that up or is it too late?
> 
> Yeah, re-reading this I admit the comment is not too clear.
> 
> Let me try to rephrase this better in v2.

Actually my comment is about the code... Do you think we can clean the 
RPi3/RPi4 codebase by changing 
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase?

If you don't have time for this, maybe you can add a /* TODO FIXME */?

>>
>>> +//
>>> +#define RPI_UART_BASE_ADDRESS (FixedPcdGet64 (PcdSerialRegisterBase) 
>>> - 0x40)
>>> +#define RPI_UART_LENGTH                                 0x70
>>> +//
>>> +// RPI_UART_STR should match the value used Uart.asl
>>> +//
>>> +#define RPI_UART_STR                                    { '\\', '_', 
>>> 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
>>> +
>>> +typedef struct {
>>> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
>>> +  EFI_ACPI_5_1_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 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_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
>>> +    UartAddrLen,                                     /* UINT32 
>>> AddressSize */                                        \
>>> +    UartNameStr                                      /* UINT8 
>>> NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
>>> +  }
>>> +
>>> +
>>> +STATIC DBG2_TABLE Dbg2 = {
>>> +  {
>>> +    ACPI_HEADER (
>>> +      EFI_ACPI_5_1_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,
>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
>>> +      RPI_UART_BASE_ADDRESS,
>>> +      RPI_UART_LENGTH,
>>> +      RPI_UART_STR
>>> +    ),
>>> +  }
>>>   };
>> [...]
>>
> 


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

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

Re: [edk2-devel] [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation
Posted by Pete Batard 6 years, 1 month ago
On 2019.12.18 17:00, Philippe Mathieu-Daudé wrote:
> On 12/18/19 5:36 PM, Pete Batard wrote:
>> Hi Philippe,
>>
>> On 2019.12.18 15:57, Philippe Mathieu-Daudé wrote:
>>> On 12/18/19 12:41 PM, Pete Batard wrote:
>>>> Use code derived from JunoPkg to generate our serial tables and
>>>> also use PCDs where possible.
>>>>
>>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>>> ---
>>>>   Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +-
>>>>   Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 
>>>> +++++++++++++++++---
>>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ----------
>>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 
>>>> ++++++++++++++++++
>>>>   4 files changed, 183 insertions(+), 63 deletions(-)
>>>>
>>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf 
>>>> b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>>> index 50c9f7694d84..6b1155d66444 100644
>>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf
>>>> @@ -31,7 +31,7 @@ [Sources]
>>>>     Gtdt.aslc
>>>>     Dsdt.asl
>>>>     Csrt.aslc
>>>> -  Spcr.asl
>>>> +  Spcr.aslc
>>>>   [Packages]
>>>>     ArmPkg/ArmPkg.dec
>>>> @@ -47,4 +47,6 @@ [FixedPcd]
>>>>     gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
>>>>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>>>>     gArmTokenSpaceGuid.PcdGicDistributorBase
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>>>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>>>>     gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
>>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc 
>>>> b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>>> index 849cf5134793..589a5c2cbd71 100644
>>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
>>>> @@ -2,27 +2,99 @@
>>>>    *
>>>>    *  Debug Port Table (DBG2)
>>>>    *
>>>> - *  Copyright (c) Microsoft Corporation. All rights reserved.
>>>> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
>>>> + *  Copyright (c) 2012-2016, ARM Limited. All rights reserved.
>>>>    *
>>>>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>    *
>>>>    **/
>>>> -UINT8 Dbg2[92] = {
>>>> -  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00,
>>>> -  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32,
>>>> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53,
>>>> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00,
>>>> -  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00,
>>>> -  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00,
>>>> -  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10,
>>>> -  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00,
>>>> -  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T',
>>>> -  'M', 0x00,
>>>> +#include <IndustryStandard/Acpi.h>
>>>> +#include <IndustryStandard/DebugPort2Table.h>
>>>> +#include <Library/AcpiLib.h>
>>>> +#include <Library/PcdLib.h>
>>>> +
>>>> +#include "AcpiTables.h"
>>>> +
>>>> +#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             10
>>>> +
>>>> +//
>>>> +// When using the miniUART, PcdSerialRegisterBase points to the 
>>>> 8250 base address,
>>>> +// which is offset by 0x40 from the actual Bcm2835 base address
>>>
>>> Actually this is the other way around, the 8250 is at AUX + 0x40.
>>>
>>> Can we clean that up or is it too late?
>>
>> Yeah, re-reading this I admit the comment is not too clear.
>>
>> Let me try to rephrase this better in v2.
> 
> Actually my comment is about the code... Do you think we can clean the 
> RPi3/RPi4 codebase by changing 
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase?
> 
> If you don't have time for this, maybe you can add a /* TODO FIXME */?

Well, I'm planning to go with FixedPcdGet64 (PcdBcm283xRegistersAddress) 
+ BCM2836_MINI_UART_OFFSET below in v2. So unless I'm missing something, 
I think this solves the point you raised and will also remove the need 
for the comment.

/Pete

> 
>>>
>>>> +//
>>>> +#define RPI_UART_BASE_ADDRESS (FixedPcdGet64 
>>>> (PcdSerialRegisterBase) - 0x40)
>>>> +#define RPI_UART_LENGTH                                 0x70
>>>> +//
>>>> +// RPI_UART_STR should match the value used Uart.asl
>>>> +//
>>>> +#define RPI_UART_STR                                    { '\\', 
>>>> '_', 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
>>>> +
>>>> +typedef struct {
>>>> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
>>>> +  EFI_ACPI_5_1_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 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_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
>>>> +    UartAddrLen,                                     /* UINT32 
>>>> AddressSize */                                        \
>>>> +    UartNameStr                                      /* UINT8 
>>>> NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
>>>> +  }
>>>> +
>>>> +
>>>> +STATIC DBG2_TABLE Dbg2 = {
>>>> +  {
>>>> +    ACPI_HEADER (
>>>> +      EFI_ACPI_5_1_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,
>>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
>>>> +      RPI_UART_BASE_ADDRESS,
>>>> +      RPI_UART_LENGTH,
>>>> +      RPI_UART_STR
>>>> +    ),
>>>> +  }
>>>>   };
>>> [...]
>>>
>>
> 


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

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