lkml.org 
[lkml]   [2012]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs
    On 07/24/2012 12:31 PM, Jiang Liu wrote:
    > From: Jiang Liu<jiang.liu@huawei.com>
    >
    > Introduce five configuration access functions for PCIe capabilities registers
    > to hide differences among PCIe Base Spec versions.
    >
    > Function pci_pcie_capability_read_word/dword() stores the PCIe Capabilities
    > register value by the passed parameter val. If related PCIe Capabilities
    > register is not implemented on the PCIe device, the passed parameter val
    > will be set 0.
    >
    > Function pci_pcie_capability_write_word/dowrd() writes the value to PCIe
    > Capability register.
    >
    > Function pci_pcie_capability_reg_implemeneted() checks whether a Capabilities
    > register is implemented by the PCIe device.
    >
    > Signed-off-by: Jiang Liu<liuj97@gmail.com>
    > Signed-off-by: Yijing Wang<wangyijing@huawei.com>
    > ---
    > drivers/pci/access.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++
    > include/linux/pci.h | 6 ++
    > include/linux/pci_regs.h | 2 +
    > 3 files changed, 165 insertions(+)
    >
    > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
    > index ba91a7e..59409e8 100644
    > --- a/drivers/pci/access.c
    > +++ b/drivers/pci/access.c
    > @@ -469,3 +469,160 @@ void pci_cfg_access_unlock(struct pci_dev *dev)
    > raw_spin_unlock_irqrestore(&pci_lock, flags);
    > }
    > EXPORT_SYMBOL_GPL(pci_cfg_access_unlock);
    > +
    > +static inline int pci_pcie_cap_version(const struct pci_dev *dev)
    > +{
    > + return dev->pcie_flags_reg& PCI_EXP_FLAGS_VERS;
    > +}
    > +
    > +static inline bool pci_pcie_cap_has_devctl(const struct pci_dev *dev)
    > +{
    > + return true;
    > +}
    > +
    > +static inline bool pci_pcie_cap_has_lnkctl(const struct pci_dev *dev)
    > +{
    > + int type = pci_pcie_type(dev);
    > +
    > + return pci_pcie_cap_version(dev)> 1 ||
    > + type == PCI_EXP_TYPE_ROOT_PORT ||
    > + type == PCI_EXP_TYPE_ENDPOINT ||
    > + type == PCI_EXP_TYPE_LEG_END;
    > +}
    > +
    > +static inline bool pci_pcie_cap_has_sltctl(const struct pci_dev *dev)
    > +{
    > + int type = pci_pcie_type(dev);
    > +
    > + return pci_pcie_cap_version(dev)> 1 ||
    > + type == PCI_EXP_TYPE_ROOT_PORT ||
    > + (type == PCI_EXP_TYPE_DOWNSTREAM&&
    > + dev->pcie_flags_reg& PCI_EXP_FLAGS_SLOT);
    > +}
    > +
    > +static inline bool pci_pcie_cap_has_rtctl(const struct pci_dev *dev)
    > +{
    > + int type = pci_pcie_type(dev);
    > +
    > + return pci_pcie_cap_version(dev)> 1 ||
    > + type == PCI_EXP_TYPE_ROOT_PORT ||
    > + type == PCI_EXP_TYPE_RC_EC;
    > +}
    > +
    > +bool pci_pcie_capability_reg_implemented(struct pci_dev *dev, int pos)
    > +{
    > + if (!pci_is_pcie(dev))
    > + return false;
    > +
    > + switch (pos) {
    > + case PCI_EXP_FLAGS_TYPE:
    > + return true;
    > + case PCI_EXP_DEVCAP:
    > + case PCI_EXP_DEVCTL:
    > + case PCI_EXP_DEVSTA:
    > + return pci_pcie_cap_has_devctl(dev);
    > + case PCI_EXP_LNKCAP:
    > + case PCI_EXP_LNKCTL:
    > + case PCI_EXP_LNKSTA:
    > + return pci_pcie_cap_has_lnkctl(dev);
    > + case PCI_EXP_SLTCAP:
    > + case PCI_EXP_SLTCTL:
    > + case PCI_EXP_SLTSTA:
    > + return pci_pcie_cap_has_sltctl(dev);
    > + case PCI_EXP_RTCTL:
    > + case PCI_EXP_RTCAP:
    > + case PCI_EXP_RTSTA:
    > + return pci_pcie_cap_has_rtctl(dev);
    > + case PCI_EXP_DEVCAP2:
    > + case PCI_EXP_DEVCTL2:
    > + case PCI_EXP_LNKCAP2:
    > + case PCI_EXP_LNKCTL2:
    > + case PCI_EXP_LNKSTA2:
    > + return pci_pcie_cap_version(dev)> 1;
    > + default:
    > + return false;
    > + }
    > +}
    > +EXPORT_SYMBOL(pci_pcie_capability_reg_implemented);
    > +
    > +/*
    > + * Quotation from PCIe Base Spec 3.0:
    > + * For Functions that do not implement the Slot Capabilities,
    > + * Slot Status, and Slot Control registers, these spaces must
    > + * be hardwired to 0b, with the exception of the Presence Detect
    > + * State bit in the Slot Status register of Downstream Ports,
    > + * which must be hardwired to 1b.
    > + */
    > +int pci_pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
    > +{
    > + int ret = 0;
    > +
    > + *val = 0;
    > + if (pos& 1)
    > + return -EINVAL;
    > +
    > + if (pci_pcie_capability_reg_implemented(dev, pos)) {
    > + ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
    > + /*
    > + * Reset *val to 0 if pci_read_config_word() fails, it may
    > + * have been written as 0xFFFF if hardware error happens
    > + * during pci_read_config_word().
    > + */
    > + if (ret)
    > + *val = 0;
    > + } else if (pos == PCI_EXP_SLTSTA&&
    > + pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
    > + *val = PCI_EXP_SLTSTA_PDS;
    > + }
    Don't you want the above if check done 1st, and not the
    pci_pcie_capability_reg_implemented(dev, pos) check ?
    Isn't PCI_EXP_SLTCTL an implemented register, looking at this snippet?:
    > + switch (pos) {
    <snip>
    > + case PCI_EXP_SLTCAP:
    > + case PCI_EXP_SLTCTL:
    > + case PCI_EXP_SLTSTA:
    > + return pci_pcie_cap_has_sltctl(dev);
    and the above function is:

    > +static inline bool pci_pcie_cap_has_sltctl(const struct pci_dev *dev)
    > +{
    > + int type = pci_pcie_type(dev);
    > +
    > + return pci_pcie_cap_version(dev)> 1 ||
    > + type == PCI_EXP_TYPE_ROOT_PORT ||
    > + (type == PCI_EXP_TYPE_DOWNSTREAM&&
    > + dev->pcie_flags_reg& PCI_EXP_FLAGS_SLOT);
    > +}

    or is PCI_EXP_FLAGS_SLOT not set when it should be, then the first condition
    fails, and this function forces the val to 1b ?
    > +
    > + return ret;
    > +}
    > +EXPORT_SYMBOL(pci_pcie_capability_read_word);
    > +
    > +int pci_pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
    > +{
    > + int ret = 0;
    > +
    > + *val = 0;
    > + if (pos& 3)
    > + return -EINVAL;
    > +
    > + if (pci_pcie_capability_reg_implemented(dev, pos)) {
    > + ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
    > + /*
    > + * Reset *val to 0 if pci_read_config_dword() fails, it may
    > + * have been written as 0xFFFFFFFF if hardware error happens
    > + * during pci_read_config_dword().
    > + */
    > + if (ret)
    > + *val = 0;
    > + } else if (pos == PCI_EXP_SLTCTL&&
    > + pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
    > + *val = PCI_EXP_SLTSTA_PDS;
    > + }
    ditto above...

    > +
    > + return ret;
    > +}
    > +EXPORT_SYMBOL(pci_pcie_capability_read_dword);
    > +
    > +int pci_pcie_capability_write_word(struct pci_dev *dev, int pos, u16 val)
    > +{
    > + if (pos& 1)
    > + return -EINVAL;
    > + else if (!pci_pcie_capability_reg_implemented(dev, pos))
    > + return 0;
    > +
    > + return pci_write_config_word(dev, pci_pcie_cap(dev) + pos, val);
    > +}
    > +EXPORT_SYMBOL(pci_pcie_capability_write_word);
    > +
    > +int pci_pcie_capability_write_dword(struct pci_dev *dev, int pos, u32 val)
    > +{
    > + if (pos& 3)
    > + return -EINVAL;
    > + else if (!pci_pcie_capability_reg_implemented(dev, pos))
    > + return 0;
    > +
    > + return pci_write_config_dword(dev, pci_pcie_cap(dev) + pos, val);
    > +}
    > +EXPORT_SYMBOL(pci_pcie_capability_write_dword);
    > diff --git a/include/linux/pci.h b/include/linux/pci.h
    > index 9807da5..a9b7605 100644
    > --- a/include/linux/pci.h
    > +++ b/include/linux/pci.h
    > @@ -816,6 +816,12 @@ static inline int pci_write_config_dword(const struct pci_dev *dev, int where,
    > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
    > }
    >
    > +int pci_pcie_capability_read_word(struct pci_dev *dev, int where, u16 *val);
    > +int pci_pcie_capability_read_dword(struct pci_dev *dev, int where, u32 *val);
    > +int pci_pcie_capability_write_word(struct pci_dev *dev, int where, u16 val);
    > +int pci_pcie_capability_write_dword(struct pci_dev *dev, int where, u32 val);
    > +bool pci_pcie_capability_reg_implemented(struct pci_dev *dev, int where);
    > +
    > /* user-space driven config access */
    > int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
    > int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
    > diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
    > index 53274bf..5300fdf 100644
    > --- a/include/linux/pci_regs.h
    > +++ b/include/linux/pci_regs.h
    > @@ -543,7 +543,9 @@
    > #define PCI_EXP_OBFF_MSGB_EN 0x4000 /* OBFF enable with Message type B */
    > #define PCI_EXP_OBFF_WAKE_EN 0x6000 /* OBFF using WAKE# signaling */
    > #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints end here */
    > +#define PCI_EXP_LNKCAP2 44 /* Link Capabilities 2 */
    > #define PCI_EXP_LNKCTL2 48 /* Link Control 2 */
    > +#define PCI_EXP_LNKSTA2 50 /* Link Status 2 */
    > #define PCI_EXP_SLTCTL2 56 /* Slot Control 2 */
    >
    > /* Extended Capabilities (PCI-X 2.0 and Express) */



    \
     
     \ /
      Last update: 2012-07-25 00:01    [W:4.590 / U:0.492 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site