From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web10.18933.1581323178454143168 for ; Mon, 10 Feb 2020 00:26:18 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: ashraf.javeed@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Feb 2020 00:26:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,424,1574150400"; d="scan'208";a="405509570" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga005.jf.intel.com with ESMTP; 10 Feb 2020 00:26:16 -0800 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 10 Feb 2020 00:26:16 -0800 Received: from bgsmsx154.gar.corp.intel.com (10.224.48.47) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 10 Feb 2020 00:26:16 -0800 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.159]) by BGSMSX154.gar.corp.intel.com ([169.254.7.238]) with mapi id 14.03.0439.000; Mon, 10 Feb 2020 13:56:13 +0530 From: "Javeed, Ashraf" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Wang, Jian J" , "Wu, Hao A" Subject: Re: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] MdeModulePkg/PciBusDxe: Setup for PCI Express features Thread-Topic: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] MdeModulePkg/PciBusDxe: Setup for PCI Express features Thread-Index: AQHV3fIMf9YTCUzmnk+VcR6U2vCJ+qgQKrMggAOCQQCAAGtwgA== Date: Mon, 10 Feb 2020 08:26:12 +0000 Message-ID: <95C5C2B113DE604FB208120C742E9824579AB45A@BGSMSX101.gar.corp.intel.com> References: <20200207200447.10536-1-ashraf.javeed@intel.com> <15F1377EA7D1AA4F.7869@groups.io> <95C5C2B113DE604FB208120C742E9824579AAF65@BGSMSX101.gar.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C429202@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C429202@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjFlOTFlZTgtNTk1YS00MjkwLTg0M2QtYjcyYmVlMjZjYTg0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNUw1OEZqaUJZd2p0c0ZkbXQwaUtaTDIxWWRnb1J6Nkp5WHpBOWpXcHUzUU9CR213MnRHTmY3NHM4Zm1SdE5ZcSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Return-Path: ashraf.javeed@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable My comments below. Thanks Ashraf > -----Original Message----- > From: Ni, Ray > Sent: Monday, February 10, 2020 12:50 PM > To: Javeed, Ashraf ; devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A > Subject: RE: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] > MdeModulePkg/PciBusDxe: Setup for PCI Express features >=20 > > > + PCI_CAPABILITY_PCIEXP PciExpressCapabilityStru= cture; > 1. To Align with existing field "Pci", how about rename it to > "PciExpressCapability" (no "Structure" suffix)? >=20 As per PCI Express Base Specification, the feature name is "PCI Express Cap= ability Structure" and I have used the same type although the data structur= e defined for the same has not used it. Is this really important that I hav= e to remove the "Structure" from its name? >=20 > 2. I see that only GetPciExpressProtocol() in PciPlatformSupport.c is use= d in this > patch. > All other functions in PciFeatureSupport.c andPciPlatformSupport.c ar= e not > used. > It makes the reviewers confused about how those unused functions can = be > used. > You should remove these unused functions in this patch and add them i= n later > patches > when the code logic calls them. >=20 All the other main routines of the PciPlatformSupport.c are called from the= code of PciFeatureSupport.c. All the routines of the PciFeatureSupport.c are called from within itself, = and its main routine that defines the integration of PCI Express feature in= itialization is defined in the following second patch. If required, I can combine both the patches into one patch, which gives the= basic structure of the PCI Express feature initialization in the PciBusDxe= . > Thanks, > Ray