[edk2-devel] [PATCH 2/2] Platform/Sgi: Add serial debug controller to SSDT

Sami Mujawar sami.mujawar at arm.com
Thu Jul 21 12:41:52 UTC 2022


Hi Rohit,

Have you considered moving to use Dynamic Tables Framework? There is 
just too much repetition in this series which can be easily avoided. It 
will also make the code more maintainable.

Apart from this I have a comment marked inline as [SAMI].

Regards,

Sami Mujawar

On 04/07/2022 05:59 pm, Rohit Mathew wrote:
> Add a new device entry in the SSDT ACPI table to describe the serial
> port used as the debug port. On the Neoverse reference design platforms,
> the UART0 port of the SoC is used as the debug port.
>
> Signed-off-by: Rohit Mathew <rohit.mathew at arm.com>
> ---
>   Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf   |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf   |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf       |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf   |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf       |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf     |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf     |  1 +
>   Platform/ARM/SgiPkg/AcpiTables/SsdtRos.asl              | 15 +++++++++++++++
>   9 files changed, 23 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> index d2935f1e73e1..d46ae0274d90 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> @@ -39,6 +39,7 @@ [Packages]
>   [FixedPcd]
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdClusterCount
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>   
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> index 73f47ece7718..4bf681d3bc2e 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> @@ -39,6 +39,7 @@ [Packages]
>   [FixedPcd]
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdClusterCount
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>   
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
> index da14120bde69..89f532217ceb 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
> @@ -41,6 +41,7 @@ [Packages]
>   [FixedPcd]
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdClusterCount
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>   
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
> index 90976250445e..66d5422df36b 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
> @@ -37,6 +37,7 @@ [Packages]
>     Platform/ARM/SgiPkg/SgiPlatform.dec
>   
>   [FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
> index 95fb446c105d..742734ab7348 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
> @@ -37,6 +37,7 @@ [Packages]
>     Platform/ARM/SgiPkg/SgiPlatform.dec
>   
>   [FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdClusterCount
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
> index 3540575dd641..cc41dda1a135 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
> @@ -37,6 +37,7 @@ [Packages]
>     Platform/ARM/SgiPkg/SgiPlatform.dec
>   
>   [FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
> index c6bd69b4a538..ecb42bf3cc33 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
> @@ -39,6 +39,7 @@ [Packages]
>     Platform/ARM/SgiPkg/SgiPlatform.dec
>   
>   [FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> index cb3f3fcdb9b6..379b5c9e6122 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
> @@ -39,6 +39,7 @@ [Packages]
>   [FixedPcd]
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdClusterCount
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgInterrupt
>     gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>     gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>   
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtRos.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtRos.asl
> index fd20c67e1225..ab8578072836 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/SsdtRos.asl
> +++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtRos.asl
> @@ -29,6 +29,21 @@ DefinitionBlock ("SsdtRosTable.aml", "SSDT", 2, "ARMLTD", "ARMSGI",
>         })
>       }
>   
> +    Device (COM1) {
> +      Name (_HID, "ARMH0011")
> +      Name (_CID, "ARMH0011")
[SAMI] Any reason for not using  ARMHB000 (see Section 2.3 of the ACPI 
for Arm Components 1.1 specification)?
> +      Name (_UID, One)
> +      Name (_STA, 0xF)
> +      Name (_CRS, ResourceTemplate () {
> +        Memory32Fixed (
> +          ReadWrite,
> +          FixedPcdGet64 (PcdSerialDbgRegisterBase),
> +          0x1000
> +          )
> +        Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { FixedPcdGet32 (PcdSerialDbgInterrupt) }
> +      })
> +    }
> +
>       // VIRTIO DISK
>       Device (VR00) {
>         Name (_HID, "LNRO0005")


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91637): https://edk2.groups.io/g/devel/message/91637
Mute This Topic: https://groups.io/mt/92169056/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-




More information about the edk2-devel-archive mailing list