| Message ID | 45b51292876f238afe3f6865113cd9d72d33e51a.1613034728.git.gustavo.pimentel@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dmaengine: dw-edma: HDMA support | expand |
Hi Gustavo, [...] > + * Typically this function will be called by the pci driver, which passes It would be "PCI" here. > + * through argument the 'struct pci_dev *' already pointing for the device > + * config space that is associated with the vendor and device ID which will > + * know which ID to search and what to do with it, however, it might be Probably "there might be". > + * cases that this function could be called outside of this scope and > + * therefore is the caller responsibility to check the vendor and/or [...] A suggestion. This commit message is a little hard to read and could be improved. It might just be me (by and large, and I am not a native English speaker), but it's actually easier to figure out what the function does after reading the implementation that from the comment. :) Krzysztof
On Thu, Feb 11, 2021 at 12:50:46, Krzysztof Wilczyński <kw@linux.com> wrote: > Hi Gustavo, > > [...] > > + * Typically this function will be called by the pci driver, which passes > > It would be "PCI" here. Nicely catch. > > > + * through argument the 'struct pci_dev *' already pointing for the device > > + * config space that is associated with the vendor and device ID which will > > + * know which ID to search and what to do with it, however, it might be > > Probably "there might be". I've rephrased it. > > > + * cases that this function could be called outside of this scope and > > + * therefore is the caller responsibility to check the vendor and/or > [...] > > A suggestion. This commit message is a little hard to read and could be > improved. > > It might just be me (by and large, and I am not a native English > speaker), but it's actually easier to figure out what the function does > after reading the implementation that from the comment. :) I have also rephrased it. I hope it would be better, but I don't see how. "Adds another helper to ones that already exist called pci_find_vsec_capability. This helper crawls through the device PCI config space searching for a specific ID on the Vendor-Specific Extended Capabilities section. The Vendor-Specific Extended Capability (VSEC) is a special PCI capability (acts like container) defined by PCI-SIG that allows the one or more proprietary capabilities defined by the vendor which aren't standard or shared between the manufactures. -Gustavo > > Krzysztof
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b9fecc2..1307af6 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -693,6 +693,40 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap) EXPORT_SYMBOL_GPL(pci_find_ht_capability); /** + * pci_find_vsec_capability - Find a vendor-specific extended capability + * @dev: PCI device to query + * @cap: vendor-specific capability ID code + * + * Typically this function will be called by the pci driver, which passes + * through argument the 'struct pci_dev *' already pointing for the device + * config space that is associated with the vendor and device ID which will + * know which ID to search and what to do with it, however, it might be + * cases that this function could be called outside of this scope and + * therefore is the caller responsibility to check the vendor and/or + * device ID first. + * + * Returns the address of the vendor-specific structure that matches the + * requested capability ID code within the device's PCI configuration space + * or 0 if it does not find a match. + */ +u16 pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id) +{ + u16 vsec = 0; + u32 header; + + while ((vsec = pci_find_next_ext_capability(dev, vsec, + PCI_EXT_CAP_ID_VNDR))) { + if (pci_read_config_dword(dev, vsec + PCI_VSEC_HDR, + &header) == PCIBIOS_SUCCESSFUL && + PCI_VSEC_CAP_ID(header) == vsec_cap_id) + return vsec; + } + + return 0; +} +EXPORT_SYMBOL_GPL(pci_find_vsec_capability); + +/** * pci_find_parent_resource - return resource region of parent bus of given * region * @dev: PCI device structure contains resources to be searched diff --git a/include/linux/pci.h b/include/linux/pci.h index b32126d..da6ab6a 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1080,6 +1080,8 @@ struct pci_bus *pci_find_next_bus(const struct pci_bus *from); u64 pci_get_dsn(struct pci_dev *dev); +u16 pci_find_vsec_capability(struct pci_dev *dev, int vsec_cap_id); + struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, struct pci_dev *from); struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index e709ae8..deae275 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -983,6 +983,12 @@ #define PCI_VSEC_HDR 4 /* extended cap - vendor-specific */ #define PCI_VSEC_HDR_LEN_SHIFT 20 /* shift for length field */ +/* Vendor-Specific Extended Capabilities */ +#define PCI_VSEC_HEADER 4 /* Vendor-Specific Header */ +#define PCI_VSEC_CAP_ID(x) ((x) & 0xffff) +#define PCI_VSEC_CAP_REV(x) (((x) >> 16) & 0xf) +#define PCI_VSEC_CAP_LEN(x) (((x) >> 20) & 0xfff) + /* SATA capability */ #define PCI_SATA_REGS 4 /* SATA REGs specifier */ #define PCI_SATA_REGS_MASK 0xF /* location - BAR#/inline */
Add pci_find_vsec_capability() that crawls through the device config space searching in all Vendor-Specific Extended Capabilities for a particular capability ID. Vendor-Specific Extended Capability (VSEC) is a PCIe capability (acts like a wrapper) specified by PCI-SIG that allows the vendor to create their own and specific capability in the device config space. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- drivers/pci/pci.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/pci.h | 2 ++ include/uapi/linux/pci_regs.h | 6 ++++++ 3 files changed, 42 insertions(+)