From: "Samer El-Haj-Mahmoud" <samer.el-haj-mahmoud@arm.com>
To: Joey Gouly <Joey.Gouly@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Joey Gouly <Joey.Gouly@arm.com>,
"ardb+tianocore@kernel.org" <ardb+tianocore@kernel.org>,
Sami Mujawar <Sami.Mujawar@arm.com>,
"Jeff Brasen (jbrasen@nvidia.com)" <jbrasen@nvidia.com>,
"ipark@nvidia.com" <ipark@nvidia.com>, nd <nd@arm.com>
Subject: Re: [PATCH v1 2/2] DynamicTablesPkg: Add an override for 16550 HID in SSDT
Date: Wed, 20 Jan 2021 18:27:38 +0000 [thread overview]
Message-ID: <DB7PR08MB3260A44F65D321419111633E90A20@DB7PR08MB3260.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20210120182005.2580-3-joey.gouly@arm.com>
> -----Original Message-----
> From: Joey Gouly <joey.gouly@arm.com>
> Sent: Wednesday, January 20, 2021 1:20 PM
> To: devel@edk2.groups.io
> Cc: Joey Gouly <Joey.Gouly@arm.com>; ardb+tianocore@kernel.org; Sami
> Mujawar <Sami.Mujawar@arm.com>; Jeff Brasen (jbrasen@nvidia.com)
> <jbrasen@nvidia.com>; ipark@nvidia.com; Samer El-Haj-Mahmoud
> <Samer.El-Haj-Mahmoud@arm.com>; nd <nd@arm.com>
> Subject: [PATCH v1 2/2] DynamicTablesPkg: Add an override for 16550 HID in
> SSDT
>
> Some platforms advertise support for a 16550 UART, but are not compatible
> with the PNP0500 HID. Allow them to override the HID by setting
> PcdNonSbsaCompliantSerialHid.
>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
> DynamicTablesPkg/DynamicTablesPkg.dec | 3 +++
>
> DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFix
> upLib.inf | 4 +++-
>
> DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFix
> upLib.c | 14 +++++++++++---
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec
> b/DynamicTablesPkg/DynamicTablesPkg.dec
> index
> 291a45a69679ae82219ecd2f26dfabfbab1f7f65..3ec4fff116a8f538be331edf34
> 1867948c025116 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
> @@ -44,5 +44,8 @@ [PcdsFixedAtBuild]
> # Maximum number of Custom DT Generators
>
> gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdMaxCustomDTGenerators|1|UI
> NT16|0xC0000003
>
> + # Non SBSA Compliant Serial HID
> +
> +
> gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdNonSbsaCompliantSerialHid|""|
> V
> + OID*|0x40000008
> +
> [Guids]
> gEdkiiDynamicTablesPkgTokenSpaceGuid = { 0xab226e66, 0x31d8, 0x4613, {
> 0x87, 0x9d, 0xd2, 0xfa, 0xb6, 0x10, 0x26, 0x3c } } diff --git
> a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.inf
> b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.inf
> index
> af3d404393f5f1385ab2d40f45f7222ab66f9b3a..b64825982e8fb7aaf78f3fd68
> 992e1c78d20c408 100644
> ---
> a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.inf
> +++
> b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialP
> +++ ortFixupLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # SSDT Serial Port fixup Library
> #
> -# Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
> +# Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent ## @@ -28,3 +28,5 @@
> [LibraryClasses]
> AmlLib
> BaseLib
>
> +[Pcd]
> + gEdkiiDynamicTablesPkgTokenSpaceGuid.PcdNonSbsaCompliantSerialHid
> diff --git
> a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.c
> b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.c
> index
> 0ff071485ef25f4ca63de0eeab5120d1beece4db..73a8087ed8a8ff84b64531a3c
> 73d319585dfb6cf 100644
> ---
> a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPort
> FixupLib.c
> +++
> b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialP
> +++ ortFixupLib.c
> @@ -1,7 +1,7 @@
> /** @file
> SSDT Serial Port Fixup Library.
>
> - Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>
> + Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -139,13 +139,21 @@ FixupIds (
> AML_OBJECT_NODE_HANDLE NameOpIdNode;
> CONST CHAR8 * HidString;
> CONST CHAR8 * CidString;
> + CONST CHAR8 * NonSbsaHid;
>
> // Get the _CID and _HID value to write.
> switch (SerialPortInfo->PortSubtype) {
> case EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_FULL_16550:
> {
> - HidString = "PNP0501";
> - CidString = "PNP0500";
> + // If there is a non-SBSA compliant HID, use that.
> + NonSbsaHid = (CONST CHAR8*)PcdGetPtr
> (PcdNonSbsaCompliantSerialHid);
> + if ((NonSbsaHid != NULL) && (AsciiStrLen (NonSbsaHid) != 0)) {
> + HidString = NonSbsaHid;
> + CidString = "";
> + } else {
> + HidString = "PNP0501";
> + CidString = "PNP0500";
> + }
> break;
> }
> case EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART:
> --
Since you are using PcdNonSbsaCompliantSerialHid to indicate that this is a non-BSA compliant 16550 UART, maybe rename the PCD to reflect that? The name PcdNonSbsaCompliantSerialHid may imply that this is not a PL011/ Arm SBSA Generic UART. BSA 1.0 allows both PL011/Generic UART (the definition moved from SBSA spec to the BSA spec) OR a 16550 standard UART. In this case, we are saying the UART is a 16550-like UART, but not exactly standard (i.e. do not use the standard 16550 IDs)
Maybe PcdNon16550CompliantSerialHid or PcdNonBsa16550CompliantSerialHid is a better name that matches what the code is doing?
The comments will need to change as well.
next prev parent reply other threads:[~2021-01-20 18:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-20 18:20 [PATCH v1 0/2] Add a Pcd to override the 16650 HID in SSDT Joey Gouly
2021-01-20 18:20 ` [PATCH v1 1/2] DynamicTablesPkg: Don't use gEfiMdeModulePkgTokenSpaceGuid Joey Gouly
2021-01-21 1:40 ` 回复: [edk2-devel] " gaoliming
2021-01-20 18:20 ` [PATCH v1 2/2] DynamicTablesPkg: Add an override for 16550 HID in SSDT Joey Gouly
2021-01-20 18:27 ` Samer El-Haj-Mahmoud [this message]
2021-02-01 14:27 ` [PATCH v1 0/2] Add a Pcd to override the 16650 " Samer El-Haj-Mahmoud
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB7PR08MB3260A44F65D321419111633E90A20@DB7PR08MB3260.eurprd08.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox