My response in line.
Thanks
Ashraf
> -----Original Message-----
> From: Ni, Ray <ray...@intel.com>
> Sent: Tuesday, December 17, 2019 7:08 AM
> 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 03/12]
> PciBusDxe: Separation of the PCI device registration and start
>
> Ashraf,
> The change looks good.
>
> 2 minor comments:
> 1. StartPciRootPortsOnBridge()
> Can it be renamed to EnablePciDevicesOnBridge()?
> Because it basically calls PciIo.Attribute() to enable the devices. And
> I am not
> sure the enable only applies to PCI root ports. There could be PCI devices
> behind
> P2P bridge.
It enables only Type 1 PCI devices (Root Port, PCIe-to-PCI Bridge, PCIe switch
upstream/downstream ports), and no endpoint (Type 0) devices.
>
> 2. + if (EFI_ERROR (Status) == EFI_NOT_FOUND) {
> Should be "if (EFI_ERROR (Status)) {"
Good catch! Will fix this.
>
> > > EFI_STATUS
> > > -StartPciDevicesOnBridge (
> > > +StartPciRootPortsOnBridge (
> > > + IN EFI_HANDLE Controller,
> > > + IN PCI_IO_DEVICE *RootBridge
> > > + )
> > + if (EFI_ERROR (Status) == EFI_NOT_FOUND) {
> > > + return Status;
> > > + } else {
> > > + //
> > > + // finally start those PCI bridge port devices only
> > > + //
> > > + return StartPciRootPortsOnBridge (
> > > + Controller,
> > > + RootBridge
> > > + );
> > > + }
> > > +}
> > > +
> > > /**
> > > Start to manage all the PCI devices it found previously under
> > > the entire host bridge.
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52275): https://edk2.groups.io/g/devel/message/52275
Mute This Topic: https://groups.io/mt/55155883/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
