From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web08.3753.1637230266079073050 for ; Thu, 18 Nov 2021 02:11:06 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3570D1FB; Thu, 18 Nov 2021 02:11:05 -0800 (PST) Received: from [192.168.1.16] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 121173F5A1; Thu, 18 Nov 2021 02:11:03 -0800 (PST) Message-ID: <149839da-fb4f-3004-92d4-1122e64bce7b@arm.com> Date: Thu, 18 Nov 2021 10:11:02 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Subject: Re: [PATCH v1 06/14] DynamicTablesPkg: FdtHwInfoParser: Add Serial port parser To: Sami Mujawar , devel@edk2.groups.io Cc: nd References: <20210623123828.23693-1-Pierre.Gondois@arm.com> <20210623123828.23693-7-Pierre.Gondois@arm.com> <0dfa3844-5e72-6430-10bb-da310ec739fc@arm.com> From: "PierreGondois" In-Reply-To: <0dfa3844-5e72-6430-10bb-da310ec739fc@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Sami, Please find my answer inlined: On 11/5/21 14:27, Sami Mujawar wrote: > > Hi Pierre, > > Please find my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > > On 23/06/2021 01:38 PM, Pierre.Gondois@arm.com wrote: >> From: Pierre Gondois >> >> The Microsoft Debug Port Table 2 (DBG2), the Serial Port Console >> Redirector (SPCR) table are mandatory tables required for booting >> a standards-based operating system. The DBG2 table is used by the >> OS debugger while the SPCR table is used to configure the serial >> terminal. Additionally, the serial ports available on a platform >> for generic use also need to be described in DSDT/SSDT for an OS >> to be able to use the serial ports. >> >> The Arm Base System Architecture 1.0 specification a lists of >> supported serial port hardware for Arm Platforms. This list >> includes the following serial port UARTs: >> - SBSA/Generic UART >> - a fully 16550 compatible UART. >> Along, with these the PL011 UART is the most commonly used serial >> port hardware on Arm platforms. >> >> The serial port hardware information is described in the platform >> Device Tree, the bindings for which can be found at: >> - linux/Documentation/devicetree/bindings/serial/serial.yaml >> - linux/Documentation/devicetree/bindings/serial/8250.txt >> - linux/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt >> - linux/Documentation/devicetree/bindings/serial/pl011.yaml >> >> The FdtHwInfoParser implements a Serial Port Parser that parses >> the platform Device Tree to create CM_ARM_SERIAL_PORT_INFO objects >> with the following IDs: >> - EArmObjSerialConsolePortInfo (for use by SPCR) >> - EArmObjSerialDebugPortInfo (for use by DBG2) >> - EArmObjSerialPortInfo (for use as generic Serial Ports) >> >> The Serial Port for use by SPCR is selected by parsing the Device >> Tree for the '/chosen' node with the 'stdout-path' property. The >> next Serial Port is selected for use as the Debug Serial Port and >> the remaining serial ports are used as generic serial ports. >> >> The CM_ARM_SERIAL_PORT_INFO objects are encapsulated in Configuration >> Manager descriptor objects with the respective IDs and are added to >> the platform information repository. >> >> The platform Configuration Manager can then utilise this information >> when generating the DBG2, SPCR and the SSDT serial port tables. >> >> Signed-off-by: Pierre Gondois >> Signed-off-by: Sami Mujawar >> --- >> .../Serial/ArmSerialPortParser.c | 621 +++++++++++++++++= + >> .../Serial/ArmSerialPortParser.h | 47 ++ >> 2 files changed, 668 insertions(+) >> create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/Serial= /ArmSerialPortParser.c >> create mode 100644 DynamicTablesPkg/Library/FdtHwInfoParserLib/Serial= /ArmSerialPortParser.h [snip] >> + >> + SerialPortInfo->Interrupt =3D FdtGetInterruptId ((CONST UINT32*)Dat= a); >> + >> + // Note: clock-frequency is optional for SBSA UART. >> + Data =3D fdt_getprop (Fdt, SerialPortNode, "clock-frequency", &Data= Size); >> + if (Data !=3D NULL) { >> + if (DataSize < sizeof (UINT32)) { >> + // If error or not enough space. >> + ASSERT (0); >> + return EFI_ABORTED; >> + } else if (fdt_node_offset_by_phandle (Fdt, fdt32_to_cpu (*Data))= >=3D 0) { >> + // "clock-frequency" can be a "clocks phandle to refer to the c= lk used". >> + // This is not supported. >> + ASSERT (0); >> + return EFI_UNSUPPORTED; >> + } >> + SerialPortInfo->Clock =3D fdt32_to_cpu (*(UINT32*)Data); >> + } >> + >> + if (FdtNodeIsCompatible (Fdt, SerialPortNode, &Serial16550Compatibl= eInfo)) { >> + SerialPortInfo->PortSubtype =3D EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL= _FULL_16550; > [SAMI] I think this needs to be set > toEFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_16550_WITH_GAS. [Pierre] From what I can found,the EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_16550_WITH_GAS has only been added recently in the edk2 and linux. I didn't find to what is correspond. In the DynamicTablesPkg, we are generating=C2=A0 the EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_FULL_16550 id. So I think we should keep the non-GAS id. >> + >> + /* reg-io-width: >> + description: | >> + The size (in bytes) of the IO accesses that should be perfor= med on the >> + device. There are some systems that require 32-bit accesses = to the >> + UART. >> + */ >> + Data =3D fdt_getprop (Fdt, SerialPortNode, "reg-io-width", &DataS= ize); >> + if (Data !=3D NULL) { >> + if (DataSize < sizeof (UINT32)) { >> + // If error or not enough space. >> + ASSERT (0); >> + return EFI_ABORTED; >> + } >> >>