From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 941A37803DA for ; Thu, 4 Jan 2024 16:03:42 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Sdt302SSHtazDnj97WNVYCUiBEKLExzc7UdXveW+0l4=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:Received-SPF:ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:Authentication-Results-Original:Message-ID:Date:User-Agent:Subject:To:Cc:References:From:In-Reply-To:MIME-Version:NoDisclaimer:Original-Authentication-Results:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Language; s=20140610; t=1704384221; v=1; b=FWVjGw2VN2FVHTcoBF7fBIShqWUwvYkrUmDgf5ZPoARLk8cKsQicTIcsKDBMLt0T2421PfbF kHIeRCbiTU4qZY7OMT4fSyw+KCYAK1gy8Wo3R9rEvhTomgxFfgPz43JVLLhiibklNh1KOJVf1wy eagSe/LtLHcfE6IC8OZgq9kQ= X-Received: by 127.0.0.2 with SMTP id DDWVYY7687511xMzX9JiAIu3; Thu, 04 Jan 2024 08:03:41 -0800 X-Received: from EUR05-AM6-obe.outbound.protection.outlook.com (EUR05-AM6-obe.outbound.protection.outlook.com [40.107.22.53]) by mx.groups.io with SMTP id smtpd.web10.57601.1704384220117426123 for ; Thu, 04 Jan 2024 08:03:40 -0800 ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=j4mUQ09QDu8+PfMXfQUnUHq3rLIQ76BKXWJQlr//yeClu9ukifnuMeDIpL+tkovw+VBlXN+MyGU7XK1OLQJQMncL53Ec5GTeeMnm8DrW4G0JSvhuiUyQ/zkdSKJDXRnlI9ZtEQhjRX5csWwLmYLuxkN7M19nnYgLmRDQ1vOv9aCTIzxTmbdVcwoX5o87DkB4W9OpuBsT5SXDljhnSRAL7Fd4kVIhCByL32xHaV+lEJy2Wk31nXxEtt1fOUrSUvtoempszKyE0/wPOePFszLn7DpbuTme4k02B+5KkE1JrrQHDl84sXUMmZwsq4Gt7u7DWX5MMDa/v6WLt84XEo4DHg== 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=+RFKTm09phBk4P1yQsmM7D7n1W8WpGf91rkH/QW2Kfo=; b=i8sgEMZzh0DchJ3S91Kn7HMgbA/RF7EroDf8PqSJbTDk6wT1KOHCeb/rYjwKs1ZnmjOSBehLquyMGLM2xiMvGn0RVHOdaEy+osKY+QoGyWdf+OWh/eBXWRq7OaJMVbqkqTJJxPycQz6UAakMu7rlXLZ6UOE+GeCaEpwCystJ4MXUsYiEI8tUTOysdpJrjXb88PAUTtqda8Q+PbESJfFMBe0wgvFbgohdfljb5+vjxWvgK1RWhDixIodKCQRrfspMZT3x+sjQA2e7F2nFxyrmaB3VcoWqwzFNOZnjUKtjHxXj2ggjuyfu4rtcJ2+xWRlGbP/8hV1Hfo6xH4vdUjv5KA== 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]) X-Received: from AS4P191CA0019.EURP191.PROD.OUTLOOK.COM (2603:10a6:20b:5d9::9) by VI0PR08MB10486.eurprd08.prod.outlook.com (2603:10a6:800:201::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7159.13; Thu, 4 Jan 2024 16:03:33 +0000 X-Received: from AM4PEPF00025F96.EURPRD83.prod.outlook.com (2603:10a6:20b:5d9:cafe::24) by AS4P191CA0019.outlook.office365.com (2603:10a6:20b:5d9::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7159.15 via Frontend Transport; Thu, 4 Jan 2024 16:03:33 +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 X-Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM4PEPF00025F96.mail.protection.outlook.com (10.167.16.5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7181.1 via Frontend Transport; Thu, 4 Jan 2024 16:03:33 +0000 X-Received: ("Tessian outbound a555e524ef42:v239"); Thu, 04 Jan 2024 16:03:32 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: a2b540bdb7d21389 X-CR-MTA-TID: 64aa7808 X-Received: from b0517b9c4f4f.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 71BB5351-E2EC-43C4-A01C-AFFBFDC48B69.1; Thu, 04 Jan 2024 16:03:21 +0000 X-Received: from EUR05-VI1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id b0517b9c4f4f.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 04 Jan 2024 16:03:21 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LJin/8kSZZu15vvd0I8bjuwRfAnNHRP5+zsVfJNd11yZRnbswvGHT8HztCGMCTTzgs8A00IfhHPcw93yGb+IyGKgPu+eRrZJFT1egDiTYcrKNYqudjdCT/gPLjwHiQ3vrJ3O07Q/iHBAgpROfvYriXga2KgxAENaw/rO2mt1oJIwQqom5YI+F4GCHuLZ2w0WBt4khra9GFsomvXQudX4k+avQGpZWXEoj5Gc7ptoWEzE/IQ00cpKt0+T+0MwbHtpaRyU1/6bDhLHNNLDNBqGRULZaarCeFHnKxg6sVm9lUf1YwLrASNple6bbyqq9RDF0ip1hSDFj0KJcYbBLZemKw== 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=+RFKTm09phBk4P1yQsmM7D7n1W8WpGf91rkH/QW2Kfo=; b=Spk7pTT1dGurU3fGfrKydmk8Wwx8ARUr1B9n9j/Hy5LjapQaucfncleJlTJ7Whd3ab/71EYfNnqbz7YvlbPcUVfsLzsRh8njuOBQTyYPVJ87YQ4N0h187Lzpw+LXemaM/VrB2HrMWw4+BPEsvfqQrOEGXnISzG++JfmmEC0IJG+wnJULtZXDaq8tXHLu/dErFJUjEWjzkkCNm9lR22k6Sx6AUQJIKjLQ6Di4qiW+ty0MjDz2lEBVQbO/wCk9xL7e/sy5Tkd5p8IwjSG2riFE6eoJ9dPDMv0x1cLzXeS7Hd7JSTXq2a8/lTXnK1QNk/+/oaWndSssT6Riq2CeVkB+Wg== 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 Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by DU0PR08MB9025.eurprd08.prod.outlook.com (2603:10a6:10:471::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7159.14; Thu, 4 Jan 2024 16:03:17 +0000 X-Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::f36e:3882:2fce:d775]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::f36e:3882:2fce:d775%4]) with mapi id 15.20.7159.015; Thu, 4 Jan 2024 16:03:17 +0000 Message-ID: <746a5922-4e75-4150-9e53-2c9c660055e4@arm.com> Date: Thu, 4 Jan 2024 16:03:15 +0000 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [edk2][PATCH V2 2/2] DynamicTablesPkg/SsdtSerialPortFixupLib: Add Interrupt node for SPIs only To: Himanshu Sharma , devel@edk2.groups.io Cc: Ard Biesheuvel , Leif Lindholm , Pierre Gondois , "nd@arm.com" References: <20240104080257.319631-1-Himanshu.Sharma@arm.com> <20240104080257.319631-3-Himanshu.Sharma@arm.com> From: "Sami Mujawar" In-Reply-To: <20240104080257.319631-3-Himanshu.Sharma@arm.com> X-ClientProxiedBy: LO4P265CA0144.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2c4::17) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: AS8PR08MB6806:EE_|DU0PR08MB9025:EE_|AM4PEPF00025F96:EE_|VI0PR08MB10486:EE_ X-MS-Office365-Filtering-Correlation-Id: 3ca8f744-5ec6-4dc5-df24-08dc0d3eb390 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: dKhZiT8tRxRdLH7a+ZW+A8n1HXayz372GdOuDbrjj3eNniPDPAwuq1IZG4iPch1/TjhpUyUNalEd1FrcgPGhAm/mwH8o4ElvYgxd5sL87vGUsZhD0CLeOB6utgrG4ITMUpajM8qTA8R43ydVtPCukgfVk/mhEAeH74F/DYpnTitvZS2oHO8kGJVM8t5GjUk7hhMxx6t2xEzV89wF3fLR68y/F0NE+K3PWTbxeCd8JpEA2WxRy54Z2mQ+5EzYIYZerCGhLxPY/b2Ut04w2swOuzR7HwMaj6vvV/tn+vWBjHpM20Ex7c1YNlm62aVWKGljHdm/7YclgxC9oLPz/XzTD4h7Tm0MxljmQl0st3whRCpgps7PCI2xkVbLfltEmAxFPzoWBkjL72deRz/K4W/BIShKt1vvvmW1vS2VBIqz1LgLdJYgqPOzork+fJrn5LZdMUxtwhzUkSSQoLA1e/1YLOV4+ag/xVsAQrLo/uVS/Jv9HMG5uA9HKvNZHLD2iuDrJTfsa9/9OvgddD2jpaXy9ucPthjDDda6oyzMaq1Yl470JyCimG3MzZHJKY8sUBmi6cU/QQ0T5dgm0SCSEP59uXFoO1he4VwFVuibpiwTff/iy9skxQ9z5iVCm3FrmVmIWAyIPLupI4Xa4pW1Pe466nwQRXl1v1R6oEEQMcmmry0= 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:(13230031)(136003)(39860400002)(346002)(366004)(376002)(396003)(230922051799003)(451199024)(1800799012)(64100799003)(186009)(38100700002)(166002)(36756003)(31686004)(86362001)(31696002)(83380400001)(26005)(41300700001)(45080400002)(66556008)(66476007)(33964004)(66946007)(8676002)(8936002)(478600001)(6506007)(53546011)(6512007)(54906003)(316002)(6486002)(2616005)(5660300002)(30864003)(2906002)(4326008)(44832011)(213903007)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR08MB9025 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM4PEPF00025F96.EURPRD83.prod.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: e1425b7e-83bc-4508-f38e-08dc0d3eaa48 X-Microsoft-Antispam-Message-Info: B5Hv/nsBjX4TdlXWYxqhLnjHeeUKASzjS1FmOMY9O+4kL+ZPVxGlTFdZkxHk/yAmqUx7aKt5/LVcOu2GVl8CGiEvCbY6l9PYZTY2wl9XNXK0AK70pFuXez9dOP3CMVl/2nOjRbLjZFCS//iUJuUhkSEEWmlYHUkEFTY0/Uf7+HJixUSO/qHweO8LmSLFVGrVvqOXK4Oa9sLJhknBlJ4CeXlCeiH3rg8cFfmTwI07/MghyPNTZ0/aWG4sK0XsbouyoM1ZAnFHCwaELiyrFqSLISCjuEFNgCBIyOPRAyZF333QAeE8CizBPHmyIdbkD+1rgTDl7xW6gNFKIAWS7PR0ogyHu+RYVrlRTXSwFnXvqO/drdJNn/azUPe7thWlagP+/WIiqd/89SNjgZIgrpsXVxmsEbiqnYu5/gKCo+R5TW8ml4m4rVQ7OI4mwTkUdDao8ljWVv7pLWyAp72kYgLOuIOPO8h5KNFBKXuo+jvhCrZjEzUQv2dLytuFT2OoPgD8t/GMjvm5/pcgnz45eKZMRmKkrkxOvpo2NFl9BSDuULbw+myK7S66KXemHFi4HZgtdwCSYyyMap3ztq3a12JRB92YWPu+VEW3FT0hhWXjj/OcLlkTw+6mSMYTf0sg8sHgER4o+tAKNcdi+G2tQReP4r2L/ZimJqccQGdi76iUJMSHl5crHDrBoVPHjBAg4Aslc32HpWPqsMlFyDg/GLldWWLixRO4FGrhDBGw8I0Ad7PlyUzHzRR3idmVkhs73ULHCWfaa9uzJDrdHQTeQNx1yDyN89WvSIV1r5YcMsc4jKQ= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2024 16:03:33.0735 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3ca8f744-5ec6-4dc5-df24-08dc0d3eb390 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: AM4PEPF00025F96.EURPRD83.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI0PR08MB10486 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,sami.mujawar@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: Wl1pFTfcVZnq3ILC7t2YbNADx7686176AA= Content-Type: multipart/alternative; boundary="------------VZUKPN7k70IQQNMpZbPzaL09" Content-Language: en-GB X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=FWVjGw2V; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}"); dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --------------VZUKPN7k70IQQNMpZbPzaL09 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Himanshu, There are some minor comments marked inline as [SAMI], otherwise this patch looks good to me. I can fix those up before merging the patch. With that, Reviewed-by: Sami Mujawar Regards, Sami Mujawar On 04/01/2024 08:02 am, Himanshu Sharma wrote: > Add interrupt node to the AML description of the serial-port only if the > IRQ ID from the Configuration Manager is a valid SPI (shared processor > interrupt) or an extended SPI. So, for DBG2 UART ports where interrupt > is not mandatory, adding of an interrupt node in the AML description > using Serial Port Fixup Library can be ignored if the UART is not > defined with a valid SPI, like in N1SDP. > > This update generates the interrupt node for the valid SPI range using > the AML Codegen API instead of updating it using the AML Fixup API. > > Signed-off-by: Himanshu Sharma > --- > DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf | 3 +- > DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 6 ++- > DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c | 49 ++++++++++++++------ > DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl | 29 +++++++----- > 4 files changed, 58 insertions(+), 29 deletions(-) > > diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf > index 965167bdc4e1..5e2615c961ad 100644 > --- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf > +++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf > @@ -1,7 +1,7 @@ > ## @file > > # SSDT Serial Port fixup Library > > # > > -# Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.
> > +# Copyright (c) 2020 - 2021, 2024, Arm Limited. All rights reserved.
[SAMI] I missed this in my previous review, but we really do not need to extend the copyright here. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > ## > > @@ -19,6 +19,7 @@ > SsdtSerialPortTemplate.asl > > > > [Packages] > > + ArmPkg/ArmPkg.dec > > MdePkg/MdePkg.dec > > MdeModulePkg/MdeModulePkg.dec > > EmbeddedPkg/EmbeddedPkg.dec > > diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > index 8c00bdac20bb..e9df0ec94808 100644 > --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h > @@ -1,6 +1,6 @@ > /** @file > > > > - Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.
> > + Copyright (c) 2017 - 2024, Arm Limited. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -311,7 +311,9 @@ typedef struct CmArmSerialPortInfo { > /// The physical base address for the serial port > > UINT64 BaseAddress; > > > > - /// The serial port interrupt > > + /** The serial port interrupt > > + to be speciifed 0 if the serial port does not have an interrupt wired. [SAMI] There is a typo here. Also would it be better to say ' 0 indicates that the serial port does not       have an interrupt wired. ' [/SAMI] > > + */ > > UINT32 Interrupt; > > > > /// The serial port baud rate > > diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c > index a65c1fe7e30d..d0b1f61fdf85 100644 > --- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c > +++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c > @@ -1,7 +1,7 @@ > /** @file > > SSDT Serial Port Fixup Library. > > > > - Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.
> > + Copyright (c) 2019 - 2021, 2024, Arm Limited. All rights reserved.
[SAMI] the end year needs to be extended, i.e. Copyright (c) 2019 - 2024, Arm Limited. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -9,10 +9,14 @@ > - Arm Server Base Boot Requirements (SBBR), s4.2.1.8 "SPCR". > > - Microsoft Debug Port Table 2 (DBG2) Specification - December 10, 2015. > > - ACPI for Arm Components 1.0 - 2020 > > + - Arm Generic Interrupt Controller Architecture Specification, > > + Issue H, January 2022. > > + (https://developer.arm.com/documentation/ihi0069/) > > **/ > > > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -270,7 +274,6 @@ FixupCrs ( > EFI_STATUS Status; > > AML_OBJECT_NODE_HANDLE NameOpCrsNode; > > AML_DATA_NODE_HANDLE QWordRdNode; > > - AML_DATA_NODE_HANDLE InterruptRdNode; > > > > // Get the "_CRS" object defined by the "Name ()" statement. > > Status = AmlFindNode ( > > @@ -303,20 +306,38 @@ FixupCrs ( > return Status; > > } > > > > - // Get the Interrupt node. > > - // It is the second Resource Data element in the NameOpCrsNode's > > - // variable list of arguments. > > - Status = AmlNameOpGetNextRdNode (QWordRdNode, &InterruptRdNode); > > - if (EFI_ERROR (Status)) { > > - return Status; > > + // Generate an interrupt node as the second Resource Data element in the > > + // NameOpCrsNode, if the interrupt for the serial-port is a valid SPI from > > + // Table 2-1 in Arm Generic Interrupt Controller Architecture Specification. > > + if (((SerialPortInfo->Interrupt >= ARM_GIC_ARCH_SPI_MIN) && > > + (SerialPortInfo->Interrupt <= ARM_GIC_ARCH_SPI_MAX)) || > > + ((SerialPortInfo->Interrupt >= ARM_GIC_ARCH_EXT_SPI_MIN) && > > + (SerialPortInfo->Interrupt <= ARM_GIC_ARCH_EXT_SPI_MAX))) > > + { > > + Status = AmlCodeGenRdInterrupt ( > > + TRUE, // Resource Consumer > > + FALSE, // Level Triggered > > + FALSE, // Active High > > + FALSE, // Exclusive > > + (UINT32 *)&SerialPortInfo->Interrupt, > > + 1, > > + NameOpCrsNode, > > + NULL > > + ); > > + ASSERT_EFI_ERROR (Status); > > + } else if (SerialPortInfo->Interrupt != 0) { > > + // If an interrupt is not wired to the serial port, the > > + // Configuration Manager specifies the interrupt as 0. > > + // Any other value must be treated as an error. > > + DEBUG (( > > + DEBUG_ERROR, > > + "ERROR: SSDT-SERIAL-PORT-FIXUP: Invalid interrupt ID for Serial Port\n" > > + )); > > + ASSERT (0); > > + Status = EFI_INVALID_PARAMETER; > > } > > > > - if (InterruptRdNode == NULL) { > > - return EFI_INVALID_PARAMETER; > > - } > > - > > - // Update the interrupt number. > > - return AmlUpdateRdInterrupt (InterruptRdNode, SerialPortInfo->Interrupt); > > + return Status; > > } > > > > /** Fixup the Serial Port device name. > > diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl > index fcae2160ac3d..22e6c04e020c 100644 > --- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl > +++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl > @@ -1,7 +1,7 @@ > /** @file > > SSDT Serial Template > > > > - Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.
> > + Copyright (c) 2019 - 2020, 2024, Arm Limited. All rights reserved.
[SAMI] Same comment as above. > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -10,6 +10,7 @@ > > > @par Glossary: > > - {template} - Data fixed up using AML Fixup APIs. > > + - {codegen} - Data generated using AML Codegen APIs. > > **/ > > > > DefinitionBlock ("SsdtSerialPortTemplate.aml", "SSDT", 2, "ARMLTD", "SERIAL", 1) { > > @@ -43,17 +44,21 @@ DefinitionBlock ("SsdtSerialPortTemplate.aml", "SSDT", 2, "ARMLTD", "SERIAL", 1) > , // MemoryRangeType > > // TranslationType > > ) // QWordMemory > > - Interrupt ( > > - ResourceConsumer, // ResourceUsage > > - Level, // EdgeLevel > > - ActiveHigh, // ActiveLevel > > - Exclusive, // Shared > > - , // ResourceSourceIndex > > - , // ResourceSource > > - // DescriptorName > > - ) { > > - 0xA5 // {template} > > - } // Interrupt > > + > > + // The Interrupt information is generated using AmlCodegen. > > + // > > + // Interrupt ( // {codegen} > > + // ResourceConsumer, // ResourceUsage > > + // Level, // EdgeLevel > > + // ActiveHigh, // ActiveLevel > > + // Exclusive, // Shared > > + // , // ResourceSourceIndex > > + // , // ResourceSource > > + // // DescriptorName > > + // ) { > > + // // > > + // } // Interrupt > > + > > }) // Name > > } // Device > > } // Scope (_SB) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113186): https://edk2.groups.io/g/devel/message/113186 Mute This Topic: https://groups.io/mt/103518973/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- --------------VZUKPN7k70IQQNMpZbPzaL09 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Himanshu,

There are some minor comments marked inline as [SAMI], otherwise this patch looks good to me.

I can fix those up before merging the patch.

With that,

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 04/01/2024 08:02 am, Himanshu Sharma wrote:
Add interrupt node to the AML description of the serial-port only if the
IRQ ID from the Configuration Manager is a valid SPI (shared processor
interrupt) or an extended SPI. So, for DBG2 UART ports where interrupt
is not mandatory, adding of an interrupt node in the AML description
using Serial Port Fixup Library can be ignored if the UART is not
defined with a valid SPI, like in N1SDP.

This update generates the interrupt node for the valid SPI range using
the AML Codegen API instead of updating it using the AML Fixup API.

Signed-off-by: Himanshu Sharma <Himanshu.Sharma@arm.com>
---
 DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf |  3 +-
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h                                    |  6 ++-
 DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c   | 49 ++++++++++++++------
 DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl | 29 +++++++-----
 4 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
index 965167bdc4e1..5e2615c961ad 100644
--- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
+++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
@@ -1,7 +1,7 @@
 ## @file

 #  SSDT Serial Port fixup Library

 #

-#  Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>

+#  Copyright (c) 2020 - 2021, 2024, Arm Limited. All rights reserved.<BR>
[SAMI] I missed this in my previous review, but we really do not need to extend the copyright here.

 #

 #  SPDX-License-Identifier: BSD-2-Clause-Patent

 ##

@@ -19,6 +19,7 @@
   SsdtSerialPortTemplate.asl

 

 [Packages]

+  ArmPkg/ArmPkg.dec

   MdePkg/MdePkg.dec

   MdeModulePkg/MdeModulePkg.dec

   EmbeddedPkg/EmbeddedPkg.dec

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 8c00bdac20bb..e9df0ec94808 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -1,6 +1,6 @@
 /** @file

 

-  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>

+  Copyright (c) 2017 - 2024, Arm Limited. All rights reserved.<BR>

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -311,7 +311,9 @@ typedef struct CmArmSerialPortInfo {
   /// The physical base address for the serial port

   UINT64    BaseAddress;

 

-  /// The serial port interrupt

+  /** The serial port interrupt

+      to be speciifed 0 if the serial port does not have an interrupt wired.

[SAMI] There is a typo here. Also would it be better to say

'            0 indicates that the serial port does not

      have an interrupt wired.

'

[/SAMI]


+  */

   UINT32    Interrupt;

 

   /// The serial port baud rate

diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c
index a65c1fe7e30d..d0b1f61fdf85 100644
--- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c
+++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.c
@@ -1,7 +1,7 @@
 /** @file

   SSDT Serial Port Fixup Library.

 

-  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>

+  Copyright (c) 2019 - 2021, 2024, Arm Limited. All rights reserved.<BR>
[SAMI] the end year needs to be extended, i.e. Copyright (c) 2019 - 2024, Arm Limited. All rights reserved.<BR> 

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -9,10 +9,14 @@
   - Arm Server Base Boot Requirements (SBBR), s4.2.1.8 "SPCR".

   - Microsoft Debug Port Table 2 (DBG2) Specification - December 10, 2015.

   - ACPI for Arm Components 1.0 - 2020

+  - Arm Generic Interrupt Controller Architecture Specification,

+    Issue H, January 2022.

+    (https://developer.arm.com/documentation/ihi0069/)

 **/

 

 #include <IndustryStandard/DebugPort2Table.h>

 #include <Library/AcpiLib.h>

+#include <Library/ArmGicArchLib.h>

 #include <Library/BaseLib.h>

 #include <Library/BaseMemoryLib.h>

 #include <Library/DebugLib.h>

@@ -270,7 +274,6 @@ FixupCrs (
   EFI_STATUS              Status;

   AML_OBJECT_NODE_HANDLE  NameOpCrsNode;

   AML_DATA_NODE_HANDLE    QWordRdNode;

-  AML_DATA_NODE_HANDLE    InterruptRdNode;

 

   // Get the "_CRS" object defined by the "Name ()" statement.

   Status = AmlFindNode (

@@ -303,20 +306,38 @@ FixupCrs (
     return Status;

   }

 

-  // Get the Interrupt node.

-  // It is the second Resource Data element in the NameOpCrsNode's

-  // variable list of arguments.

-  Status = AmlNameOpGetNextRdNode (QWordRdNode, &InterruptRdNode);

-  if (EFI_ERROR (Status)) {

-    return Status;

+  // Generate an interrupt node as the second Resource Data element in the

+  // NameOpCrsNode, if the interrupt for the serial-port is a valid SPI from

+  // Table 2-1 in Arm Generic Interrupt Controller Architecture Specification.

+  if (((SerialPortInfo->Interrupt >= ARM_GIC_ARCH_SPI_MIN) &&

+       (SerialPortInfo->Interrupt <= ARM_GIC_ARCH_SPI_MAX)) ||

+      ((SerialPortInfo->Interrupt >= ARM_GIC_ARCH_EXT_SPI_MIN) &&

+       (SerialPortInfo->Interrupt <= ARM_GIC_ARCH_EXT_SPI_MAX)))

+  {

+    Status = AmlCodeGenRdInterrupt (

+               TRUE,                  // Resource Consumer

+               FALSE,                 // Level Triggered

+               FALSE,                 // Active High

+               FALSE,                 // Exclusive

+               (UINT32 *)&SerialPortInfo->Interrupt,

+               1,

+               NameOpCrsNode,

+               NULL

+               );

+    ASSERT_EFI_ERROR (Status);

+  } else if (SerialPortInfo->Interrupt != 0) {

+    // If an interrupt is not wired to the serial port, the

+    // Configuration Manager specifies the interrupt as 0.

+    // Any other value must be treated as an error.

+    DEBUG ((

+      DEBUG_ERROR,

+      "ERROR: SSDT-SERIAL-PORT-FIXUP: Invalid interrupt ID for Serial Port\n"

+      ));

+    ASSERT (0);

+    Status = EFI_INVALID_PARAMETER;

   }

 

-  if (InterruptRdNode == NULL) {

-    return EFI_INVALID_PARAMETER;

-  }

-

-  // Update the interrupt number.

-  return AmlUpdateRdInterrupt (InterruptRdNode, SerialPortInfo->Interrupt);

+  return Status;

 }

 

 /** Fixup the Serial Port device name.

diff --git a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl
index fcae2160ac3d..22e6c04e020c 100644
--- a/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl
+++ b/DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortTemplate.asl
@@ -1,7 +1,7 @@
 /** @file

   SSDT Serial Template

 

-  Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>

+  Copyright (c) 2019 - 2020, 2024, Arm Limited. All rights reserved.<BR>
[SAMI] Same comment as above.

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -10,6 +10,7 @@
 

   @par Glossary:

     - {template} - Data fixed up using AML Fixup APIs.

+    - {codegen}  - Data generated using AML Codegen APIs.

 **/

 

 DefinitionBlock ("SsdtSerialPortTemplate.aml", "SSDT", 2, "ARMLTD", "SERIAL", 1) {

@@ -43,17 +44,21 @@ DefinitionBlock ("SsdtSerialPortTemplate.aml", "SSDT", 2, "ARMLTD", "SERIAL", 1)
           ,                   // MemoryRangeType

                               // TranslationType

         ) // QWordMemory

-        Interrupt (

-          ResourceConsumer,   // ResourceUsage

-          Level,              // EdgeLevel

-          ActiveHigh,         // ActiveLevel

-          Exclusive,          // Shared

-          ,                   // ResourceSourceIndex

-          ,                   // ResourceSource

-                              // DescriptorName

-          ) {

-            0xA5                                          // {template}

-        } // Interrupt

+

+        // The Interrupt information is generated using AmlCodegen.

+        //

+        // Interrupt (                                    // {codegen}

+        //  ResourceConsumer, // ResourceUsage

+        //  Level,            // EdgeLevel

+        //  ActiveHigh,       // ActiveLevel

+        //  Exclusive,        // Shared

+        //  ,                 // ResourceSourceIndex

+        //  ,                 // ResourceSource

+        //                    // DescriptorName

+        //  ) {

+        //    <IRQ>           // <spi>

+        // } // Interrupt

+

       }) // Name

     } // Device

   } // Scope (_SB)
_._,_._,_
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#113186) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------VZUKPN7k70IQQNMpZbPzaL09--