On Sun, Sep 13, 2020 at 01:49:26PM -0700, Kuppuswamy, Sathyanarayanan wrote: > On 9/10/20 2:00 PM, Kuppuswamy, Sathyanarayanan wrote: > > On 9/10/20 12:49 PM, Bjorn Helgaas wrote: > > > On Fri, Jul 24, 2020 at 08:58:52PM -0700, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote: > > > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > > > > > > > > If CONFIG_PCIEPORTBUS is not enabled in kernel then initialing > > > > struct pci_host_bridge PCIe specific native_* members to "1" is > > > > incorrect. So protect the PCIe specific member initialization > > > > with CONFIG_PCIEPORTBUS. > > > > > > s/initialing/initializing/ > > will fix it in next version. > > > > > > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > > > > --- > > > > drivers/pci/probe.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > > > index 2f66988cea25..a94b97564ceb 100644 > > > > --- a/drivers/pci/probe.c > > > > +++ b/drivers/pci/probe.c > > > > @@ -588,12 +588,14 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) > > > > * may implement its own AER handling and use _OSC to prevent the > > > > * OS from interfering. > > > > */ > > > > +#ifdef CONFIG_PCIEPORTBUS > > > > bridge->native_aer = 1; > > > > bridge->native_pcie_hotplug = 1; > > > > - bridge->native_shpc_hotplug = 1; > > > > bridge->native_pme = 1; > > > > bridge->native_ltr = 1; > > > > > > native_ltr isn't dependent on PCIEPORTBUS either, is it? It's only > > > used for ASPM. > > Agreed. I was confused due to a comment in include/linux/pci.h > > > > unsigned int native_ltr:1; /* OS may use PCIe LTR */ > Currently there is no code dependency between LTR and CONFIG_PCIEPORTBUS. > > But I am wondering whether its correct to move LTR code under > CONFIG_PCIEPORTBUS?. As per PCIe spec v5.0 sec 7.8.2, LTR is a > optional PCIe extended capability. So why is not moved under > drivers/pci/pcie/*. What is the criteria for code to be placed under > drivers/pci/pcie/* Some folks think drivers/pci/pcie/ should not exist, and I tend to agree, but it's a fair bit of churn to remove it. We do have quite a bit of PCIe extended capability support in drivers/pci -- ats.c, iov.c, vc.c. There's no need to move LTR under CONFIG_PCIEPORTBUS because CONFIG_PCIEPORTBUS enables portdrv, and AFAIK there's nothing LTR-related that relies on portdrv. The stuff currently in drivers/pci/pcie is mostly just portdrv and services that depend on it. aspm.c and ptm.c are exceptions and really should be in drivers/pci. > > > > bridge->native_dpc = 1; > > > > +#endif > > > > + bridge->native_shpc_hotplug = 1; > > > > device_initialize(&bridge->dev); > > > > } > > > > -- > > > > 2.17.1 > > > > > > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer
