On Mon, Oct 26, 2020 at 07:57:05PM -0700, Kuppuswamy Sathyanarayanan wrote: > pcie_ports_native is set only if user requests native handling > of PCIe capabilities via pcie_port_setup command line option. > User input takes precedence over _OSC based control negotiation > result. So consider the _OSC negotiated result only if > pcie_ports_native is unset. > > Also, since struct pci_host_bridge ->native_* members caches the > ownership status of various PCIe capabilities, use them instead > of distributed checks for pcie_ports_native. > diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c > index 50a9522ab07d..ccd5e0ce5605 100644 > --- a/drivers/pci/pcie/portdrv_core.c > +++ b/drivers/pci/pcie/portdrv_core.c > @@ -208,8 +208,7 @@ static int get_port_device_capability(struct pci_dev *dev) > struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); > int services = 0; > > - if (dev->is_hotplug_bridge && > - (pcie_ports_native || host->native_pcie_hotplug)) { > + if (dev->is_hotplug_bridge && host->native_pcie_hotplug) { This is a nit, but I think this and similar checks should be reordered so we do the most generic test first, i.e., if (host->native_pcie_hotplug && dev->is_hotplug_bridge) Logically there's no point in looking at the device things if we don't have native control. > services |= PCIE_PORT_SERVICE_HP; > > /* > @@ -221,8 +220,7 @@ static int get_port_device_capability(struct pci_dev *dev) > } > > #ifdef CONFIG_PCIEAER > - if (dev->aer_cap && pci_aer_available() && > - (pcie_ports_native || host->native_aer)) { > + if (dev->aer_cap && pci_aer_available() && host->native_aer) { Can't we clear host->native_aer when pci_aer_available() returns false? I'd like to have all the checks about whether we have native control to be in one place instead of being scattered. Something like this above: OSC_OWNER(ctrl, OSC_PCI_EXPRESS_AER_CONTROL, host_bridge->native_aer); if (!pci_aer_available()) host_bridge->native_aer = 0; So this test would become: if (host->native_aer && dev->aer_cap) > services |= PCIE_PORT_SERVICE_AER; > > /* > @@ -238,8 +236,7 @@ static int get_port_device_capability(struct pci_dev *dev) > * Event Collectors can also generate PMEs, but we don't handle > * those yet. > */ > - if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && > - (pcie_ports_native || host->native_pme)) { > + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && host->native_pme) { > services |= PCIE_PORT_SERVICE_PME; Also here: if (host->native_pme && pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)