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]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to