On Thu, Nov 06, 2014 at 05:59:40PM -0800, Yinghai Lu wrote: > With commit 6cd33649fa83 ("PCI: Add pci_configure_device() during enumeration") > and following commits, we apply _HPX for pci devices. > > that is called via pci_scan_slot > ==>pci_scan_single_device > ==>pci_device_add > ==>pci_configure_device > ==>program_hpp_type2 > and dev->subordinate is not allocated yet. > > So that link control setup get skipped. > > Acoording to ACPI spec, there is no requirement that need to be set to pcie > root port or downstream port. > > We could just change to pcie_cap_has_lnkctl to cover all pcie devices with > link control cap. > > Fixes: 6cd33649fa83 ("PCI: Add pci_configure_device() during enumeration") > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> > > --- > drivers/pci/access.c | 13 ------------- > drivers/pci/pci.h | 13 +++++++++++++ > drivers/pci/probe.c | 2 +- > 3 files changed, 14 insertions(+), 14 deletions(-) > > Index: linux-2.6/drivers/pci/probe.c > =================================================================== > --- linux-2.6.orig/drivers/pci/probe.c > +++ linux-2.6/drivers/pci/probe.c > @@ -1323,7 +1323,7 @@ static void program_hpp_type2(struct pci > ~hpp->pci_exp_devctl_and, hpp->pci_exp_devctl_or); > > /* Initialize Link Control Register */ > - if (dev->subordinate) > + if (pcie_cap_has_lnkctl(dev)) > pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL, > ~hpp->pci_exp_lnkctl_and, hpp->pci_exp_lnkctl_or); Nice catch! It's much cleaner to do this based on pcie_cap_has_lnkctl(). > Index: linux-2.6/drivers/pci/access.c > =================================================================== > --- linux-2.6.orig/drivers/pci/access.c > +++ linux-2.6/drivers/pci/access.c > @@ -444,19 +444,6 @@ static inline int pcie_cap_version(const > return pcie_caps_reg(dev) & PCI_EXP_FLAGS_VERS; > } > > -static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev) > -{ > - int type = pci_pcie_type(dev); > - > - return type == PCI_EXP_TYPE_ENDPOINT || > - type == PCI_EXP_TYPE_LEG_END || > - type == PCI_EXP_TYPE_ROOT_PORT || > - type == PCI_EXP_TYPE_UPSTREAM || > - type == PCI_EXP_TYPE_DOWNSTREAM || > - type == PCI_EXP_TYPE_PCI_BRIDGE || > - type == PCI_EXP_TYPE_PCIE_BRIDGE; > -} > - > static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev) > { > int type = pci_pcie_type(dev); > Index: linux-2.6/drivers/pci/pci.h > =================================================================== > --- linux-2.6.orig/drivers/pci/pci.h > +++ linux-2.6/drivers/pci/pci.h > @@ -6,6 +6,19 @@ > > extern const unsigned char pcie_link_speed[]; > > +static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev) > +{ > + int type = pci_pcie_type(dev); > + > + return type == PCI_EXP_TYPE_ENDPOINT || > + type == PCI_EXP_TYPE_LEG_END || > + type == PCI_EXP_TYPE_ROOT_PORT || > + type == PCI_EXP_TYPE_UPSTREAM || > + type == PCI_EXP_TYPE_DOWNSTREAM || > + type == PCI_EXP_TYPE_PCI_BRIDGE || > + type == PCI_EXP_TYPE_PCIE_BRIDGE; > +} But I don't want to move pcie_cap_has_lnkctl() to pci.h and leave pcie_cap_has_sltctl() and pcie_cap_has_rtctl() in access.c. They're all related, so I think they should stay together. What if we left them in access.c, made them non-static, non-inline, and just added prototypes to pci.h? Or I guess we could add a prototype just for pcie_cap_has_lnkctl(), since that's the only one we need so far. Bjorn > + > /* Functions internal to the PCI core code */ > > int pci_create_sysfs_dev_files(struct pci_dev *pdev); -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html