Hi Sami,
Thank you for the review.
Regarding the use of Dynamic Tables Framework, there are no short term plans to
migrate to it. Please find my response for your comment inline -
On Thu, Jul 21, 2022 at 01:42 PM, Sami Mujawar wrote:
>
> 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.mat...@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)?
[Rohit]: COM1 is based on PL011. Since PL011 would satisfy SBSA compliance, we
have used PL011's HID.
>
>
>> + 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")
>
>
Regards,
Rohit
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91697): https://edk2.groups.io/g/devel/message/91697
Mute This Topic: https://groups.io/mt/92169056/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
