My comments below. Thanks Ashraf
> -----Original Message----- > From: Ni, Ray <ray...@intel.com> > Sent: Monday, February 10, 2020 12:50 PM > To: Javeed, Ashraf <ashraf.jav...@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com> > Subject: RE: [edk2-devel] [edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 01/12] > MdeModulePkg/PciBusDxe: Setup for PCI Express features > > > > + PCI_CAPABILITY_PCIEXP > > > PciExpressCapabilityStructure; > 1. To Align with existing field "Pci", how about rename it to > "PciExpressCapability" (no "Structure" suffix)? > As per PCI Express Base Specification, the feature name is "PCI Express Capability Structure" and I have used the same type although the data structure defined for the same has not used it. Is this really important that I have to remove the "Structure" from its name? > > 2. I see that only GetPciExpressProtocol() in PciPlatformSupport.c is used in > this > patch. > All other functions in PciFeatureSupport.c andPciPlatformSupport.c are 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 in > later > patches > when the code logic calls them. > 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 initialization 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54116): https://edk2.groups.io/g/devel/message/54116 Mute This Topic: https://groups.io/mt/71063360/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-