| Message ID | 20211110221456.11977-5-jim2101024@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: brcmstb: have portdrv turn on sub-device power | expand |
On 11/10/21 2:14 PM, Jim Quinlan wrote: > The function will be needed elsewhere in a few commits. > > Signed-off-by: Jim Quinlan <jim2101024@gmail.com> > --- > drivers/pci/pci.h | 2 ++ > drivers/pci/pcie/portdrv_pci.c | 23 ++++++++++++++++++----- > 2 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 1cce56c2aea0..c2bd1995d3a9 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -744,4 +744,6 @@ extern const struct attribute_group aspm_ctrl_attr_group; > > extern const struct attribute_group pci_dev_reset_method_attr_group; > > +bool pcie_is_port_dev(struct pci_dev *dev); Looks like you need an inline stub here when CONFIG_PCIEPORTBUS is disabled to avoid the linking failure reported by the kbuild robot: static inline bool pcie_is_port_dev(struct pci_dev *dev) { return false; } Thanks! -- Florian
On Thu, Nov 11, 2021 at 3:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 11/10/21 2:14 PM, Jim Quinlan wrote: > > The function will be needed elsewhere in a few commits. > > > > Signed-off-by: Jim Quinlan <jim2101024@gmail.com> > > --- > > drivers/pci/pci.h | 2 ++ > > drivers/pci/pcie/portdrv_pci.c | 23 ++++++++++++++++++----- > > 2 files changed, 20 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > > index 1cce56c2aea0..c2bd1995d3a9 100644 > > --- a/drivers/pci/pci.h > > +++ b/drivers/pci/pci.h > > @@ -744,4 +744,6 @@ extern const struct attribute_group aspm_ctrl_attr_group; > > > > extern const struct attribute_group pci_dev_reset_method_attr_group; > > > > +bool pcie_is_port_dev(struct pci_dev *dev); > > Looks like you need an inline stub here when CONFIG_PCIEPORTBUS is > disabled to avoid the linking failure reported by the kbuild robot: Probably always an inline function. It has nothing to do with the driver, so portdrv_pci.c is not the right place. Rob
Hi Jim, [...] > +bool pcie_is_port_dev(struct pci_dev *dev) > +{ > + int type; > + > + if (!dev) > + return false; > + > + type = pci_pcie_type(dev); > + > + return pci_is_pcie(dev) && > + ((type == PCI_EXP_TYPE_ROOT_PORT) || > + (type == PCI_EXP_TYPE_UPSTREAM) || > + (type == PCI_EXP_TYPE_DOWNSTREAM) || > + (type == PCI_EXP_TYPE_RC_EC)); > +} > +EXPORT_SYMBOL_GPL(pcie_is_port_dev); It would be really nice to document what the above function does (not that some of the logic has been extracted from other function). You know, for the future generations of kernel hackers. Krzysztof
On Thu, Nov 11, 2021 at 6:50 PM Krzysztof Wilczyński <kw@linux.com> wrote: > > Hi Jim, > > [...] > > +bool pcie_is_port_dev(struct pci_dev *dev) > > +{ > > + int type; > > + > > + if (!dev) > > + return false; > > + > > + type = pci_pcie_type(dev); > > + > > + return pci_is_pcie(dev) && > > + ((type == PCI_EXP_TYPE_ROOT_PORT) || > > + (type == PCI_EXP_TYPE_UPSTREAM) || > > + (type == PCI_EXP_TYPE_DOWNSTREAM) || > > + (type == PCI_EXP_TYPE_RC_EC)); > > +} > > +EXPORT_SYMBOL_GPL(pcie_is_port_dev); > > It would be really nice to document what the above function does (not that > some of the logic has been extracted from other function). You know, for > the future generations of kernel hackers. Hi Krzysztof and others, I gave this a second look and realized that the portdrv's pci_device_id list for the probe is doing filtering that is not included in the function. So perhaps the code must be the following in order to live up to its name: static inline bool pci_is_port_dev(struct pci_dev *dev) { int type, class; if (!dev || !pci_is_pcie(dev)) return false; class = dev->class; /* This must be kept in sync with port_pci_ids[] of protdev_pci.c */ if (!(class == ((PCI_CLASS_BRIDGE_PCI << 8) | 0x00) || class == ((PCI_CLASS_BRIDGE_PCI << 8) | 0x01) || class == ((PCI_CLASS_SYSTEM_RCEC << 8) | 0x00))) return false; type = pci_pcie_type(dev); return ((type == PCI_EXP_TYPE_ROOT_PORT) || (type == PCI_EXP_TYPE_UPSTREAM) || (type == PCI_EXP_TYPE_DOWNSTREAM) || (type == PCI_EXP_TYPE_RC_EC)); } Kind of large for an inline, plus the code must be kept in sync with the device list. Suggestions? As for a description, my understanding is that the code identifies a pci_dev that is directly under a host bridge device. I'm not really sure about the PCI_CLASS_SYSTEM_RCEC though. Regards, Jim Quinlan Broadcom STB > > Krzysztof
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 1cce56c2aea0..c2bd1995d3a9 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -744,4 +744,6 @@ extern const struct attribute_group aspm_ctrl_attr_group; extern const struct attribute_group pci_dev_reset_method_attr_group; +bool pcie_is_port_dev(struct pci_dev *dev); + #endif /* DRIVERS_PCI_H */ diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c index c7ff1eea225a..63f2a87e9db8 100644 --- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c @@ -90,6 +90,23 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = { #define PCIE_PORTDRV_PM_OPS NULL #endif /* !PM */ +bool pcie_is_port_dev(struct pci_dev *dev) +{ + int type; + + if (!dev) + return false; + + type = pci_pcie_type(dev); + + return pci_is_pcie(dev) && + ((type == PCI_EXP_TYPE_ROOT_PORT) || + (type == PCI_EXP_TYPE_UPSTREAM) || + (type == PCI_EXP_TYPE_DOWNSTREAM) || + (type == PCI_EXP_TYPE_RC_EC)); +} +EXPORT_SYMBOL_GPL(pcie_is_port_dev); + /* * pcie_portdrv_probe - Probe PCI-Express port devices * @dev: PCI-Express port device being probed @@ -104,11 +121,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, int type = pci_pcie_type(dev); int status; - if (!pci_is_pcie(dev) || - ((type != PCI_EXP_TYPE_ROOT_PORT) && - (type != PCI_EXP_TYPE_UPSTREAM) && - (type != PCI_EXP_TYPE_DOWNSTREAM) && - (type != PCI_EXP_TYPE_RC_EC))) + if (!pcie_is_port_dev(dev)) return -ENODEV; if (type == PCI_EXP_TYPE_RC_EC)
The function will be needed elsewhere in a few commits. Signed-off-by: Jim Quinlan <jim2101024@gmail.com> --- drivers/pci/pci.h | 2 ++ drivers/pci/pcie/portdrv_pci.c | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-)