From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-AM6-obe.outbound.protection.outlook.com (EUR05-AM6-obe.outbound.protection.outlook.com [40.107.22.55]) by mx.groups.io with SMTP id smtpd.web10.26578.1658745727587779314 for ; Mon, 25 Jul 2022 03:42:08 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=QQI6RNcj; spf=pass (domain: arm.com, ip: 40.107.22.55, mailfrom: sami.mujawar@arm.com) ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=ijjIQDlLdkDyZHDsZtbsFF4DFIEXr4k0G2ikfkF0U3hR2XS2xI430sHWcYCaD2jUn6Cg7qnr62ZlwLK/d5bSqamvx4ZbzQ9MysDzJJVZAa4YCMG5keGgk8KBAK63b1YxYzDvUuyPwrJAgXz3tUeFHFTL37vxMq26U4piUxlzmguedK6hSSv92VpnrQNFvmuEvy3d+StjnbkJE58YHbLv3weRANfy+6DFl6TqmQLOcbHYmZKoYQfc1ycSAMcRK07MFT1SWVPXFybv7FLWcDTEGF8Rt+bwqy//g2QmYHdJw8e2Taxc2U9OjQ1kV/zwHmDJtxEOBUszFycUDhLvznVCTw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jyxz4n1PkcwS+qseexVSs5rX5jp5nTCy0heH/cF2Fpg=; b=PIKTdGLpn4FN9fezWSRyCbeQLESudZnblMp7XrcTG2JGQ6vCb8ktJwlRJ3GpTAElV3f6UuVEDaa3ZFDSAEPvWthdSUR5MLXRsIu42bNYFI1yjDc/VJt27vOlgAygbLKCP4YlKH3wQidKN3S6V84XzolSELD9ASMLmmqnauTOGcYLcORsyaZpY+fbNXzuUbspolNfylh7Kn+jAueP4SAYaHeDrWth8hHVKrUEjfITIdEZI1U+VBp6RoXsAz9pFdgAY0bOJc+oBIH7bpPAZomnnHQ3qw+HbhqEuW2dPb6oFqxa10EMhCYzCHk56uuY0qBTo18qtKV/u4TquRihG+q8og== ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=edk2.groups.io smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com]) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jyxz4n1PkcwS+qseexVSs5rX5jp5nTCy0heH/cF2Fpg=; b=QQI6RNcj9gq4BQptG/QRai2GYPIJjYmqR04S7uU6e/j9zMQpg9H6ReAiJj3tR4V2vvMURMJto0TPlgaBENBXMR/VCOTRsjGV48AjPvb16fvmTx3IlM39M6G2J+2ztxuGoGsR9Z8ebFT9O7HrXaQ8BEkPMuQQ68IutCxC8skKDD8= Received: from AS8PR04CA0174.eurprd04.prod.outlook.com (2603:10a6:20b:331::29) by VI1PR08MB3949.eurprd08.prod.outlook.com (2603:10a6:803:e6::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.21; Mon, 25 Jul 2022 10:42:01 +0000 Received: from AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:331:cafe::70) by AS8PR04CA0174.outlook.office365.com (2603:10a6:20b:331::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.24 via Frontend Transport; Mon, 25 Jul 2022 10:42:00 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT005.mail.protection.outlook.com (10.152.16.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.17 via Frontend Transport; Mon, 25 Jul 2022 10:42:00 +0000 Received: ("Tessian outbound fccf984e7173:v123"); Mon, 25 Jul 2022 10:42:00 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 1697fc5dc9e83ebf X-CR-MTA-TID: 64aa7808 Received: from 9768df2bfbfb.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 4284BF45-AE75-4F17-8859-2144B5D5F510.1; Mon, 25 Jul 2022 10:41:54 +0000 Received: from EUR02-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 9768df2bfbfb.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Mon, 25 Jul 2022 10:41:54 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VsnZGHbErgfvTkrxxIaVgLK//OQCwQnoChxo/N+lY9S0pVgdq6MDdlVVC2h9KCA83oXqxPFZlyIMhax2HTnnO9g1smdnl8xLTGOzPlXGKhPgyH3if3bPs19kz5Yosy8uN7NRY0fhKzvb8XW8cMZk32o/02annx7rXbfgl7rFAT0FpdxQ0A0b2fsZ2dC5qmRHbRQk11q4GdCazDyOOFO782TQQw6n4QlBl/Q3NZgmt7XjvyEXzfTeo5YuFMyz1rw5sVbPWO+CcJVVE+r4HqBJdfumdfLiAwBlZrtWHAxGaj+1zctSDhtGuxInfTyl8u4mtMd8DxU4+gSS8JJWmfKW5A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jyxz4n1PkcwS+qseexVSs5rX5jp5nTCy0heH/cF2Fpg=; b=bwkt4DQg5AheMWX4ZN/qzcv3hxljipnIs2psM/USAVRXty7UmJ6rhycYbOpUZfIP3R5HTo64aj160XXLKTQbM3aEet8HLOSiIWS+iG8BnTrflczTO//bLqozOoxgPYs3Ke3bcTXg6bc5RIS0cQ4jIDiEZDCXn4IFYgoYSOMlpuWUca48b69wC0H5DIz+PR+34o8i+BjXRbB/tMmoP8eKx5B5Gp9ReAC1BSWjD8xJeJr5rFgHIxWvp5/wMnJRXD1473UBFFmKOUJ5+4dBDnNEKhYgUe9N8EvWNRfE/UavHaiwUmwtSX5gUcc7Hbayl0QwFxKCuO4ZKEbBlv9+zLi6dw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jyxz4n1PkcwS+qseexVSs5rX5jp5nTCy0heH/cF2Fpg=; b=QQI6RNcj9gq4BQptG/QRai2GYPIJjYmqR04S7uU6e/j9zMQpg9H6ReAiJj3tR4V2vvMURMJto0TPlgaBENBXMR/VCOTRsjGV48AjPvb16fvmTx3IlM39M6G2J+2ztxuGoGsR9Z8ebFT9O7HrXaQ8BEkPMuQQ68IutCxC8skKDD8= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by PAXPR08MB6528.eurprd08.prod.outlook.com (2603:10a6:102:15a::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.18; Mon, 25 Jul 2022 10:41:51 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::d562:5a52:f638:7fe9]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::d562:5a52:f638:7fe9%4]) with mapi id 15.20.5458.024; Mon, 25 Jul 2022 10:41:51 +0000 Message-ID: <18a61e5d-5ed8-7e7c-9cbd-65be05900126@arm.com> Date: Mon, 25 Jul 2022 11:41:50 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [edk2-devel] [PATCH 2/2] Platform/Sgi: Add serial debug controller to SSDT To: Rohit Mathew , devel@edk2.groups.io References: <3602.1658494004063183052@groups.io> From: "Sami Mujawar" Cc: Thanu.Rangarajan@arm.com, nd@arm.com In-Reply-To: <3602.1658494004063183052@groups.io> X-ClientProxiedBy: LO2P265CA0453.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:e::33) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 5242fd03-a3fc-4213-d4aa-08da6e2a4e5e X-MS-TrafficTypeDiagnostic: PAXPR08MB6528:EE_|AM5EUR03FT005:EE_|VI1PR08MB3949:EE_ x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: oH1SLyPE34b70hBs8Lx7pxSfgc+21t50MlvbIpL0D7xoSxWBgrp1VVRvcKpmyzyBOWLXn4sKvqR+N1rTcEeagAdWF73EipvgMHEoqfdU9wBRHMEF1YIr9zkQrenDIiAsqS4wcO+PmEiBliHQywe1Qhk9yvmIeDVXSB09RCFjBVjLEUVQE7K4mp/5unCSMKVrf8fOM0gq0KF7HzhZNNlv4DdI/fBKqgN34DS63pF3kCdoWaBBETyBdhd/PGxq4qm850ZZNUsNhNpcSef20oghk4ELop5TjWE8P6De2c3vr3vxUhVEf1w090G90P/gz8AQYCk/R2rgC53gvXR5MKm+sxbQEVXM4lvXc67N3udoSlIqyWs5U0lPzb+W64iuRRFr/zP2f/H4rbUw1alCEXa1TPPd6Cn9TVlzzWSmstkZ/4qGPWiECPqYQ5biz+WlpnKiWI1n6dAEX+2sHj06qtcrKR3r8Au75gwKeqfb55t7L3OYx9FiNj53+8eek8gbuanMMG+Ay6fq/ZOxY4sdAsE8XyWEF3Uw+AK2aM9iqSQyyAXMOsNdlTdPXTF0QxDX52LX5fDjITSgA+Z1/z7Alfyhq6V5u9ZU7Gacx+cMiO/r+mfaSU9bs8Uyb+o7GsGyIXpJsHNQ/fIycQ6Y7zjsE6yxvIlg+IW0X2Rn7LWat5C3vyh/cfSF3fq/0inAvFd/AJyXv2t9kgkS1rm3rzfmrUaxXZtd6s8wtCuZj/hfIz5tKhFOF9nOTDCwlLN64dFIjlFiYbIMbf0zWLOpndcOrVMBeLrt6uUU56H3v1bGIrvRQ1xYV/snBSpsqpUgKzZ1zzxteoZAGESkYPONwiifRoaEL29utGybGhmQsNvc/VxABFg= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(4636009)(136003)(376002)(39860400002)(396003)(346002)(366004)(38100700002)(44832011)(30864003)(6512007)(166002)(26005)(33964004)(53546011)(6506007)(2616005)(8936002)(5660300002)(2906002)(41300700001)(36756003)(19627235002)(316002)(31696002)(86362001)(478600001)(6486002)(83380400001)(31686004)(66476007)(186003)(8676002)(4326008)(66556008)(66946007)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR08MB6528 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 98598697-7665-4f41-6497-08da6e2a488f X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: whdq5zd3yWCpTH3UdKHj96k+Ns1QNuU1yj6yPiti4AZ4fdaYOj+yWaR0uY6jM/eSdtmLW4Dsth1sA3DreXu+BkTzZrmUQK7xc0jQyuMZGAlPQBOTYdLirjqP5hn9xo2IW3Bpmz4hLQv/4q6qZiCmlsOaquDdlAitmYORBR72E6Vbw2T8ulgwkqxmHgvoeeKc/Kxt4lw11sx5hqC58uPTkQe2DKq0n96O2309woCeRGo9oKlAb8QdY/Ku8vgh8XL+1BTogX0kvnjDOt6IzkfjEdOPcTnw/mRi/59fG0PLt4kfoGnZRmBjqyrfPJ/Is2mfSoSAnUnWWrT6hFmkrbgzPHq0IhGDxfDlw3M1/ePIWo7LJB/83zjXbazLiHn9Mcn1qc5nKrGr+rFrt0jljMUMngSPdGjK5emUJFBgMaMUlC0ms+j63Eyju59jOfFFCY6/QWIWgEqgDAGWWeTPrH/wpVQxJ3i4UkTxUirHkzE4NA8LHEnZTPVnG1lhYefvifi3mrdjx5UWyc6HFy9D7i+w0zwa9tPkXxi5HMT50N0NgxNpRSI6vN5y48yDs+p/tOOb3BE3sdbog/rmBE0igomUkvc7FsvNvhQIxwTV0Crdp5fnsqKvioGsrwOn6q7y14AJrf++zD10amC56pU8fTCTsNj8y3/Ucbc0wQmGWau0TF7+hj8TVjjfoCFlC1q5kuJp+Bi0TVp83Q+bCZmpaVZXfOEfaOZAdOJwJt16w/XbxQPZAMb+j0q/DACICp1OQcEDqu37nulQQPe2TqHK8Ya6pjz9PHRQKu0bWytpG2IOEV7QQkI4yGqTIIGuhR0Q+aUQTX+PkrEu7u7DBiBVht2mtrT3eicpbszhnh2Wxqhhgro= X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230016)(4636009)(136003)(396003)(376002)(39860400002)(346002)(40470700004)(36840700001)(46966006)(47076005)(166002)(336012)(86362001)(186003)(83380400001)(31696002)(36860700001)(82740400003)(40460700003)(8676002)(4326008)(2616005)(316002)(19627235002)(36756003)(70206006)(30864003)(44832011)(8936002)(5660300002)(2906002)(81166007)(478600001)(41300700001)(6486002)(40480700001)(26005)(6512007)(356005)(33964004)(70586007)(6506007)(82310400005)(53546011)(31686004)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jul 2022 10:42:00.8084 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 5242fd03-a3fc-4213-d4aa-08da6e2a4e5e X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3949 Content-Type: multipart/alternative; boundary="------------5ay00VyejpCQAu6SMQU2bbSv" --------------5ay00VyejpCQAu6SMQU2bbSv Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Rohit, Please find my response inline marked [SAMI]. Regards, Sami Mujawar On 22/07/2022 01:46 pm, Rohit Mathew wrote: > 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 > --- > 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. [SAMI] Following is an extract from Section 2.3 of the 'ACPI for Arm Components 1.1' specification. "Some operating systems use the PL011 HID (see Table 5 above) to bind to the Arm Generic UART in the system. While this practice is flawed and not encouraged by Arm, Arm acknowledges that it must be permitted until formal support for the Arm Generic UART HID is made available in these operating systems. Arm strongly recommends use of the Arm Generic UART HID going forward." 1. The Arm Generic UART is a subset of PL011. This means using the ARMHB000 should not be an issue. 2. This file is common to all platforms in SgiPkg, which are infrastructure platforms. 3. Some opreating systems (like Linux) have already integrated support for ARMHB000.     Ref: serial: pl011: Add ACPI SBSA UART match id Considering the above, I think ARMHB000 should be used, here. [/SAMI] > + 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 --------------5ay00VyejpCQAu6SMQU2bbSv Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Rohit,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 22/07/2022 01:46 pm, Rohit Mathew wrote:
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.mathew@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.

[SAMI] Following is an extract from Section 2.3 of the 'ACPI for Arm Components 1.1' specification.

"Some operating systems use the PL011 HID (see Table 5 above)
to bind to the Arm Generic UART in the system. While this
practice is flawed and not encouraged by Arm, Arm
acknowledges that it must be permitted until formal support for
the Arm Generic UART HID is made available in these operating
systems.
Arm strongly recommends use of the Arm Generic UART HID
going forward."

1. The Arm Generic UART is a subset of PL011. This means using the ARMHB000 should not be an issue.

2. This file is common to all platforms in SgiPkg, which are infrastructure platforms.

3. Some opreating systems (like Linux) have already integrated support for ARMHB000.

    Ref: serial: pl011: Add ACPI SBSA UART match id

Considering the above, I think ARMHB000 should be used, here.

[/SAMI]


+ 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
--------------5ay00VyejpCQAu6SMQU2bbSv--