linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers
@ 2012-07-24 16:31 Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register Jiang Liu
                   ` (23 more replies)
  0 siblings, 24 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci

From: Jiang Liu <liuj97@gmail.com>

As suggested by Bjorn Helgaas and Don Dutile in threads
http://www.spinics.net/lists/linux-pci/msg15663.html, we could improve access
to PCIe capabilities register in to way:
1) cache content of PCIe Capabilities Register into struct pce_dev to avoid
   repeatedly reading this register because it's read only.
2) provide access functions for PCIe Capabilities registers to hide differences
   among PCIe base specifications, so the caller don't need to handle those
   differences.

This patch set applies to
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci-next

And you could pull the change set from
https://github.com/jiangliu/linux.git topic/pcie-cap

These patch set is still RFC. It provides the new interfaces and has made the
major changes to adopt those new interfaces. But there are still several device
drivers left untouched. Any comments about the new interfaces are welcomed,
especially about function names:). Once we reach an agreement, I will send out
a formal version with all needed work done.

v2: 1) change return value to 0 when the register is not implemented by
       V1 PCIe devices.
    2) Change all driver in the source tree to use the new interfaces.

Jiang Liu (29):
  PCI: add PCIe capabilities access functions to hide differences among
    PCIe specs
  PCI/core: use PCIe capabilities access functions to simplify
    implementation
  PCI/hotplug: use PCIe capabilities access functions to simplify
    implementation
  PCI/portdrv: use PCIe capabilities access functions to simplify
    implementation
  PCI/pciehp: use PCIe capabilities access functions to simplify
    implementation
  PCI/PME: use PCIe capabilities access functions to simplify
    implementation
  PCI/AER: use PCIe capabilities access functions to simplify
    implementation
  PCI/ASPM: use PCIe capabilities access functions to simplify
    implementation
  PCI/ARM: use PCIe capabilities access functions to simplify
    implementation
  PCI/MIPS: use PCIe capabilities access functions to simplify
    implementation
  PCI/tile: use PCIe capabilities access functions to simplify
    implementation
  PCI/r8169: use PCIe capabilities access functions to simplify
    implementation
  PCI/broadcom: use PCIe capabilities access functions to simplify
    implementation
  PCI/igb: use PCIe capabilities access functions to simplify
    implementation
  PCI/vxge: use PCIe capabilities access functions to simplify
    implementation
  PCI/mlx4: use PCIe capabilities access functions to simplify
    implementation
  PCI/niu: use PCIe capabilities access functions to simplify
    implementation
  PCI/myri10ge: use PCIe capabilities access functions to simplify
    implementation
  PCI/chelsio: use PCIe capabilities access functions to simplify
    implementation
  PCI/atl1c: use PCIe capabilities access functions to simplify
    implementation
  PCI/ath9k: use PCIe capabilities access functions to simplify
    implementation
  PCI/iwl: use PCIe capabilities access functions to simplify
    implementation
  PCI/mthca: use PCIe capabilities access functions to simplify
    implementation
  PCI/qib: use PCIe capabilities access functions to simplify
    implementation
  PCI/qla: use PCIe capabilities access functions to simplify
    implementation
  PCI/radeon: use PCIe capabilities access functions to simplify
    implementation
  PCI/tsi721: use PCIe capabilities access functions to simplify
    implementation
  PCI/et131x: use PCIe capabilities access functions to simplify
    implementation
  PCI/rtl8192e: use PCIe capabilities access functions to simplify
    implementation

Yijing Wang (3):
  PCI: add pcie_flags_reg into struct pci_dev to cache PCIe
    capabilities register
  PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type
  PCI: remove unused field pcie_type from struct pci_dev

 arch/arm/mach-tegra/pcie.c                         |    7 +-
 arch/mips/pci/pci-octeon.c                         |    7 +-
 arch/powerpc/platforms/powernv/pci-ioda.c          |    2 +-
 arch/tile/kernel/pci.c                             |   17 +-
 drivers/gpu/drm/radeon/evergreen.c                 |    9 +-
 drivers/infiniband/hw/mthca/mthca_reset.c          |    8 +-
 drivers/infiniband/hw/qib/qib_pcie.c               |   40 ++-
 drivers/iommu/intel-iommu.c                        |    6 +-
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c    |    2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   |   24 +-
 drivers/net/ethernet/broadcom/tg3.c                |   46 ++--
 drivers/net/ethernet/chelsio/cxgb3/t3_hw.c         |   19 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    |   10 +-
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c         |    7 +-
 drivers/net/ethernet/intel/e1000e/netdev.c         |   20 +-
 drivers/net/ethernet/intel/igb/igb_main.c          |   12 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |    2 +-
 drivers/net/ethernet/mellanox/mlx4/reset.c         |    8 +-
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |   21 +-
 drivers/net/ethernet/neterion/vxge/vxge-config.c   |    4 +-
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |    2 +-
 drivers/net/ethernet/realtek/r8169.c               |   35 +--
 drivers/net/ethernet/sun/niu.c                     |    9 +-
 drivers/net/wireless/ath/ath9k/pci.c               |   18 +-
 drivers/net/wireless/iwlegacy/common.h             |    5 +-
 drivers/net/wireless/iwlwifi/iwl-trans-pcie.c      |    4 +-
 drivers/net/wireless/rtlwifi/pci.c                 |    8 +-
 drivers/pci/access.c                               |  157 ++++++++++++
 drivers/pci/hotplug/pciehp_acpi.c                  |    6 +-
 drivers/pci/hotplug/pciehp_hpc.c                   |   10 +-
 drivers/pci/hotplug/pcihp_slot.c                   |   12 +-
 drivers/pci/iov.c                                  |    6 +-
 drivers/pci/pci.c                                  |  262 +++++---------------
 drivers/pci/pcie/aer/aer_inject.c                  |    2 +-
 drivers/pci/pcie/aer/aerdrv.c                      |   23 +-
 drivers/pci/pcie/aer/aerdrv_acpi.c                 |    2 +-
 drivers/pci/pcie/aer/aerdrv_core.c                 |   27 +-
 drivers/pci/pcie/aspm.c                            |  108 ++++----
 drivers/pci/pcie/pme.c                             |   23 +-
 drivers/pci/pcie/portdrv_bus.c                     |    2 +-
 drivers/pci/pcie/portdrv_core.c                    |   24 +-
 drivers/pci/pcie/portdrv_pci.c                     |   15 +-
 drivers/pci/probe.c                                |   29 +--
 drivers/pci/quirks.c                               |    9 +-
 drivers/pci/search.c                               |    2 +-
 drivers/rapidio/devices/tsi721.c                   |   13 +-
 drivers/scsi/qla2xxx/qla_init.c                    |    4 +-
 drivers/scsi/qla2xxx/qla_nx.c                      |    8 +-
 drivers/scsi/qla4xxx/ql4_nx.c                      |    4 +-
 drivers/staging/et131x/et131x.c                    |    9 +-
 drivers/staging/rtl8192e/rtl8192e/rtl_pci.c        |    8 +-
 include/linux/pci.h                                |   17 +-
 include/linux/pci_regs.h                           |    2 +
 53 files changed, 510 insertions(+), 626 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-25 15:12   ` Don Dutile
  2012-07-24 16:31 ` [RFC PATCH v2 02/32] PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type Jiang Liu
                   ` (22 subsequent siblings)
  23 siblings, 1 reply; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Yijing Wang, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, linux-kernel, linux-pci, Jiang Liu

From: Yijing Wang <wangyijing@huawei.com>

From: Yijing Wang <wangyijing@huawei.com>

Since PCI Express Capabilities Register is read only, cache its value
into struct pci_dev to avoid repeatedly calling pci_read_config_*().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/probe.c |    1 +
 include/linux/pci.h |   10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6c143b4..6fd58df 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -929,6 +929,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
 	pdev->is_pcie = 1;
 	pdev->pcie_cap = pos;
 	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
+	pdev->pcie_flags_reg = reg16;
 	pdev->pcie_type = (reg16 & PCI_EXP_FLAGS_TYPE) >> 4;
 	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
 	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5faa831..95662b2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -258,6 +258,7 @@ struct pci_dev {
 	u8		pcie_mpss:3;	/* PCI-E Max Payload Size Supported */
 	u8		rom_base_reg;	/* which config register controls the ROM */
 	u8		pin;  		/* which interrupt pin this device uses */
+	u16		pcie_flags_reg;	/* cached PCI-E Capabilities Register */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this
@@ -1650,6 +1651,15 @@ static inline bool pci_is_pcie(struct pci_dev *dev)
 	return !!pci_pcie_cap(dev);
 }
 
+/**
+ * pci_pcie_type - get the PCIe device/port type
+ * @dev: PCI device
+ */
+static inline int pci_pcie_type(const struct pci_dev *dev)
+{
+	return (dev->pcie_flags_reg & PCI_EXP_FLAGS_TYPE) >> 4;
+}
+
 void pci_request_acs(void);
 bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
 bool pci_acs_path_enabled(struct pci_dev *start,
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 02/32] PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 03/32] PCI: remove unused field pcie_type from struct pci_dev Jiang Liu
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Yijing Wang, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, linux-kernel, linux-pci, Jiang Liu

From: Yijing Wang <wangyijing@huawei.com>

From: Yijing Wang <wangyijing@huawei.com>

Introduce an inline function pci_pcie_type(dev) to extract PCIe
device type from pci_dev->pcie_flags_reg field, and prepare for
removing pci_dev->pcie_type.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c          |    2 +-
 drivers/iommu/intel-iommu.c                        |    6 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |    2 +-
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |    2 +-
 drivers/pci/iov.c                                  |    6 +--
 drivers/pci/pci.c                                  |   26 ++++++------
 drivers/pci/pcie/aer/aer_inject.c                  |    2 +-
 drivers/pci/pcie/aer/aerdrv.c                      |    7 ++--
 drivers/pci/pcie/aer/aerdrv_acpi.c                 |    2 +-
 drivers/pci/pcie/aer/aerdrv_core.c                 |    2 +-
 drivers/pci/pcie/aspm.c                            |   42 ++++++++++----------
 drivers/pci/pcie/pme.c                             |    6 +--
 drivers/pci/pcie/portdrv_bus.c                     |    2 +-
 drivers/pci/pcie/portdrv_core.c                    |    4 +-
 drivers/pci/pcie/portdrv_pci.c                     |    8 ++--
 drivers/pci/probe.c                                |    9 +++--
 drivers/pci/search.c                               |    2 +-
 17 files changed, 67 insertions(+), 63 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 9cda6a1..b46e1da 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -855,7 +855,7 @@ static void __devinit pnv_ioda_setup_PEs(struct pci_bus *bus)
 		if (pe == NULL)
 			continue;
 		/* Leaving the PCIe domain ... single PE# */
-		if (dev->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE)
+		if (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)
 			pnv_ioda_setup_bus_PE(dev, pe);
 		else if (dev->subordinate)
 			pnv_ioda_setup_PEs(dev->subordinate);
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 2fb7d15..8612271 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2350,7 +2350,7 @@ static int iommu_should_identity_map(struct pci_dev *pdev, int startup)
 			return 0;
 		if (pdev->class >> 8 == PCI_CLASS_BRIDGE_PCI)
 			return 0;
-	} else if (pdev->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE)
+	} else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_PCI_BRIDGE)
 		return 0;
 
 	/* 
@@ -3545,10 +3545,10 @@ found:
 		struct pci_dev *bridge = bus->self;
 
 		if (!bridge || !pci_is_pcie(bridge) ||
-		    bridge->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE)
+		    pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)
 			return 0;
 
-		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
+		if (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT) {
 			for (i = 0; i < atsru->devices_cnt; i++)
 				if (atsru->devices[i] == bridge)
 					return 1;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bf20457..565772e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7470,7 +7470,7 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
 		goto skip_bad_vf_detection;
 
 	bdev = pdev->bus->self;
-	while (bdev && (bdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT))
+	while (bdev && (pci_pcie_type(bdev) != PCI_EXP_TYPE_ROOT_PORT))
 		bdev = bdev->bus->self;
 
 	if (!bdev)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 342b3a7..01d6141 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -1382,7 +1382,7 @@ static void netxen_mask_aer_correctable(struct netxen_adapter *adapter)
 		adapter->ahw.board_type != NETXEN_BRDTYPE_P3_10G_TP)
 		return;
 
-	if (root->pcie_type != PCI_EXP_TYPE_ROOT_PORT)
+	if (pci_pcie_type(root) != PCI_EXP_TYPE_ROOT_PORT)
 		return;
 
 	aer_pos = pci_find_ext_capability(root, PCI_EXT_CAP_ID_ERR);
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 74bbaf8..aeccc91 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -433,8 +433,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	struct resource *res;
 	struct pci_dev *pdev;
 
-	if (dev->pcie_type != PCI_EXP_TYPE_RC_END &&
-	    dev->pcie_type != PCI_EXP_TYPE_ENDPOINT)
+	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
+	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
 		return -ENODEV;
 
 	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
@@ -503,7 +503,7 @@ found:
 	iov->self = dev;
 	pci_read_config_dword(dev, pos + PCI_SRIOV_CAP, &iov->cap);
 	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
-	if (dev->pcie_type == PCI_EXP_TYPE_RC_END)
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
 		iov->link = PCI_DEVFN(PCI_SLOT(dev->devfn), iov->link);
 
 	if (pdev)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f3ea977..28eb55b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -885,7 +885,7 @@ static struct pci_cap_saved_state *pci_find_saved_cap(
 
 static int pci_save_pcie_state(struct pci_dev *dev)
 {
-	int pos, i = 0;
+	int type, pos, i = 0;
 	struct pci_cap_saved_state *save_state;
 	u16 *cap;
 	u16 flags;
@@ -903,13 +903,14 @@ static int pci_save_pcie_state(struct pci_dev *dev)
 
 	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags);
 
-	if (pcie_cap_has_devctl(dev->pcie_type, flags))
+	type = pci_pcie_type(dev);
+	if (pcie_cap_has_devctl(type, flags))
 		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &cap[i++]);
-	if (pcie_cap_has_lnkctl(dev->pcie_type, flags))
+	if (pcie_cap_has_lnkctl(type, flags))
 		pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]);
-	if (pcie_cap_has_sltctl(dev->pcie_type, flags))
+	if (pcie_cap_has_sltctl(type, flags))
 		pci_read_config_word(dev, pos + PCI_EXP_SLTCTL, &cap[i++]);
-	if (pcie_cap_has_rtctl(dev->pcie_type, flags))
+	if (pcie_cap_has_rtctl(type, flags))
 		pci_read_config_word(dev, pos + PCI_EXP_RTCTL, &cap[i++]);
 
 	pos = pci_pcie_cap2(dev);
@@ -924,7 +925,7 @@ static int pci_save_pcie_state(struct pci_dev *dev)
 
 static void pci_restore_pcie_state(struct pci_dev *dev)
 {
-	int i = 0, pos;
+	int i = 0, pos, type;
 	struct pci_cap_saved_state *save_state;
 	u16 *cap;
 	u16 flags;
@@ -937,13 +938,14 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
 
 	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags);
 
-	if (pcie_cap_has_devctl(dev->pcie_type, flags))
+	type = pci_pcie_type(dev);
+	if (pcie_cap_has_devctl(type, flags))
 		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, cap[i++]);
-	if (pcie_cap_has_lnkctl(dev->pcie_type, flags))
+	if (pcie_cap_has_lnkctl(type, flags))
 		pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]);
-	if (pcie_cap_has_sltctl(dev->pcie_type, flags))
+	if (pcie_cap_has_sltctl(type, flags))
 		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
-	if (pcie_cap_has_rtctl(dev->pcie_type, flags))
+	if (pcie_cap_has_rtctl(type, flags))
 		pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
 
 	pos = pci_pcie_cap2(dev);
@@ -2459,8 +2461,8 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
 		acs_flags &= (PCI_ACS_RR | PCI_ACS_CR |
 			      PCI_ACS_EC | PCI_ACS_DT);
 
-	if (pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM ||
-	    pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT ||
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM ||
+	    pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
 	    pdev->multifunction) {
 		pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ACS);
 		if (!pos)
diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c
index 5222986..4e24cb8 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -288,7 +288,7 @@ static struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
 	while (1) {
 		if (!pci_is_pcie(dev))
 			break;
-		if (dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+		if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
 			return dev;
 		if (!dev->bus->self)
 			break;
diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 58ad791..f7c6245 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -81,10 +81,11 @@ bool pci_aer_available(void)
 static int set_device_error_reporting(struct pci_dev *dev, void *data)
 {
 	bool enable = *((bool *)data);
+	int type = pci_pcie_type(dev);
 
-	if ((dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) ||
-	    (dev->pcie_type == PCI_EXP_TYPE_UPSTREAM) ||
-	    (dev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM)) {
+	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
+	    (type == PCI_EXP_TYPE_UPSTREAM) ||
+	    (type == PCI_EXP_TYPE_DOWNSTREAM)) {
 		if (enable)
 			pci_enable_pcie_error_reporting(dev);
 		else
diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index 124f20f..5194a7d 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -60,7 +60,7 @@ static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data)
 	p = (struct acpi_hest_aer_common *)(hest_hdr + 1);
 	if (p->flags & ACPI_HEST_GLOBAL) {
 		if ((pci_is_pcie(info->pci_dev) &&
-		     info->pci_dev->pcie_type == pcie_type) || bridge)
+		     pci_pcie_type(info->pci_dev) == pcie_type) || bridge)
 			ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST);
 	} else
 		if (hest_match_pci(p, info->pci_dev))
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 0ca0535..f551534 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -465,7 +465,7 @@ static pci_ers_result_t reset_link(struct pci_dev *dev)
 
 	if (driver && driver->reset_link) {
 		status = driver->reset_link(udev);
-	} else if (udev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM) {
+	} else if (pci_pcie_type(udev) == PCI_EXP_TYPE_DOWNSTREAM) {
 		status = default_downstream_reset_link(udev);
 	} else {
 		dev_printk(KERN_DEBUG, &dev->dev,
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index b500840..2591603 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -412,7 +412,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	 * do ASPM for now.
 	 */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		if (child->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE) {
+		if (pci_pcie_type(child) == PCI_EXP_TYPE_PCI_BRIDGE) {
 			link->aspm_disable = ASPM_STATE_ALL;
 			break;
 		}
@@ -425,8 +425,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		struct aspm_latency *acceptable =
 			&link->acceptable[PCI_FUNC(child->devfn)];
 
-		if (child->pcie_type != PCI_EXP_TYPE_ENDPOINT &&
-		    child->pcie_type != PCI_EXP_TYPE_LEG_END)
+		if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT &&
+		    pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END)
 			continue;
 
 		pos = pci_pcie_cap(child);
@@ -552,7 +552,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
 	INIT_LIST_HEAD(&link->children);
 	INIT_LIST_HEAD(&link->link);
 	link->pdev = pdev;
-	if (pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM) {
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM) {
 		struct pcie_link_state *parent;
 		parent = pdev->bus->parent->self->link_state;
 		if (!parent) {
@@ -585,12 +585,12 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 
 	if (!pci_is_pcie(pdev) || pdev->link_state)
 		return;
-	if (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT &&
-	    pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)
+	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
+	    pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM)
 		return;
 
 	/* VIA has a strange chipset, root port is under a bridge */
-	if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT &&
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
 	    pdev->bus->self)
 		return;
 
@@ -647,8 +647,8 @@ static void pcie_update_aspm_capable(struct pcie_link_state *root)
 		if (link->root != root)
 			continue;
 		list_for_each_entry(child, &linkbus->devices, bus_list) {
-			if ((child->pcie_type != PCI_EXP_TYPE_ENDPOINT) &&
-			    (child->pcie_type != PCI_EXP_TYPE_LEG_END))
+			if ((pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT) &&
+			    (pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END))
 				continue;
 			pcie_aspm_check_latency(child);
 		}
@@ -663,8 +663,8 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	if (!pci_is_pcie(pdev) || !parent || !parent->link_state)
 		return;
-	if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
-	    (parent->pcie_type != PCI_EXP_TYPE_DOWNSTREAM))
+	if ((pci_pcie_type(parent) != PCI_EXP_TYPE_ROOT_PORT) &&
+	    (pci_pcie_type(parent) != PCI_EXP_TYPE_DOWNSTREAM))
 		return;
 
 	down_read(&pci_bus_sem);
@@ -704,8 +704,8 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev)
 
 	if (aspm_disabled || !pci_is_pcie(pdev) || !link)
 		return;
-	if ((pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
-	    (pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM))
+	if ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) &&
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
 		return;
 	/*
 	 * Devices changed PM state, we should recheck if latency
@@ -729,8 +729,8 @@ void pcie_aspm_powersave_config_link(struct pci_dev *pdev)
 	if (aspm_policy != POLICY_POWERSAVE)
 		return;
 
-	if ((pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
-	    (pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM))
+	if ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) &&
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
 		return;
 
 	down_read(&pci_bus_sem);
@@ -757,8 +757,8 @@ static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem,
 	if (!pci_is_pcie(pdev))
 		return;
 
-	if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT ||
-	    pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM)
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+	    pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM)
 		parent = pdev;
 	if (!parent || !parent->link_state)
 		return;
@@ -933,8 +933,8 @@ void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev)
 	struct pcie_link_state *link_state = pdev->link_state;
 
 	if (!pci_is_pcie(pdev) ||
-	    (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT &&
-	     pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM) || !link_state)
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
+	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM) || !link_state)
 		return;
 
 	if (link_state->aspm_support)
@@ -950,8 +950,8 @@ void pcie_aspm_remove_sysfs_dev_files(struct pci_dev *pdev)
 	struct pcie_link_state *link_state = pdev->link_state;
 
 	if (!pci_is_pcie(pdev) ||
-	    (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT &&
-	     pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM) || !link_state)
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
+	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM) || !link_state)
 		return;
 
 	if (link_state->aspm_support)
diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 001f1b7..30897bf 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -120,7 +120,7 @@ static bool pcie_pme_from_pci_bridge(struct pci_bus *bus, u8 devfn)
 	if (!dev)
 		return false;
 
-	if (pci_is_pcie(dev) && dev->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE) {
+	if (pci_is_pcie(dev) && pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE) {
 		down_read(&pci_bus_sem);
 		if (pcie_pme_walk_bus(bus))
 			found = true;
@@ -335,13 +335,13 @@ static void pcie_pme_mark_devices(struct pci_dev *port)
 		struct pci_dev *dev;
 
 		/* Check if this is a root port event collector. */
-		if (port->pcie_type != PCI_EXP_TYPE_RC_EC || !bus)
+		if (pci_pcie_type(port) != PCI_EXP_TYPE_RC_EC || !bus)
 			return;
 
 		down_read(&pci_bus_sem);
 		list_for_each_entry(dev, &bus->devices, bus_list)
 			if (pci_is_pcie(dev)
-			    && dev->pcie_type == PCI_EXP_TYPE_RC_END)
+			    && pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END)
 				pcie_pme_set_native(dev, NULL);
 		up_read(&pci_bus_sem);
 	}
diff --git a/drivers/pci/pcie/portdrv_bus.c b/drivers/pci/pcie/portdrv_bus.c
index 18bf90f..67be55a 100644
--- a/drivers/pci/pcie/portdrv_bus.c
+++ b/drivers/pci/pcie/portdrv_bus.c
@@ -38,7 +38,7 @@ static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
 		return 0;
 
 	if ((driver->port_type != PCIE_ANY_PORT) &&
-	    (driver->port_type != pciedev->port->pcie_type))
+	    (driver->port_type != pci_pcie_type(pciedev->port)))
 		return 0;
 
 	return 1;
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 75915b3..bf320a9 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -298,7 +298,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 		services |= PCIE_PORT_SERVICE_VC;
 	/* Root ports are capable of generating PME too */
 	if ((cap_mask & PCIE_PORT_SERVICE_PME)
-	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
+	    && pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
 		services |= PCIE_PORT_SERVICE_PME;
 		/*
 		 * Disable PME interrupt on this port in case it's been enabled
@@ -336,7 +336,7 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
 	device->release = release_pcie_device;	/* callback to free pcie dev */
 	dev_set_name(device, "%s:pcie%02x",
 		     pci_name(pdev),
-		     get_descriptor_id(pdev->pcie_type, service));
+		     get_descriptor_id(pci_pcie_type(pdev), service));
 	device->parent = &pdev->dev;
 	device_enable_async_suspend(device);
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 3a7eefc..24d1463 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -95,7 +95,7 @@ static int pcie_port_resume_noirq(struct device *dev)
 	 * which breaks ACPI-based runtime wakeup on PCI Express, so clear those
 	 * bits now just in case (shouldn't hurt).
 	 */
-	if(pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
 		pcie_clear_root_pme_status(pdev);
 	return 0;
 }
@@ -186,9 +186,9 @@ static int __devinit pcie_portdrv_probe(struct pci_dev *dev,
 	int status;
 
 	if (!pci_is_pcie(dev) ||
-	    ((dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
-	     (dev->pcie_type != PCI_EXP_TYPE_UPSTREAM) &&
-	     (dev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)))
+	    ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
+	     (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
+	     (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)))
 		return -ENODEV;
 
 	if (!dev->irq && dev->pin) {
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6fd58df..00c7bab 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1384,9 +1384,9 @@ static int only_one_child(struct pci_bus *bus)
 
 	if (!parent || !pci_is_pcie(parent))
 		return 0;
-	if (parent->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
+	if (pci_pcie_type(parent) == PCI_EXP_TYPE_ROOT_PORT)
 		return 1;
-	if (parent->pcie_type == PCI_EXP_TYPE_DOWNSTREAM &&
+	if (pci_pcie_type(parent) == PCI_EXP_TYPE_DOWNSTREAM &&
 	    !pci_has_flag(PCI_SCAN_ALL_PCIE_DEVS))
 		return 1;
 	return 0;
@@ -1463,7 +1463,7 @@ static int pcie_find_smpss(struct pci_dev *dev, void *data)
 	 */
 	if (dev->is_hotplug_bridge && (!list_is_singular(&dev->bus->devices) ||
 	     (dev->bus->self &&
-	      dev->bus->self->pcie_type != PCI_EXP_TYPE_ROOT_PORT)))
+	      pci_pcie_type(dev->bus->self) != PCI_EXP_TYPE_ROOT_PORT)))
 		*smpss = 0;
 
 	if (*smpss > dev->pcie_mpss)
@@ -1479,7 +1479,8 @@ static void pcie_write_mps(struct pci_dev *dev, int mps)
 	if (pcie_bus_config == PCIE_BUS_PERFORMANCE) {
 		mps = 128 << dev->pcie_mpss;
 
-		if (dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT && dev->bus->self)
+		if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT &&
+		    dev->bus->self)
 			/* For "Performance", the assumption is made that
 			 * downstream communication will never be larger than
 			 * the MRRS.  So, the MPS only needs to be configured
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 993d4a0..621b162 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -41,7 +41,7 @@ pci_find_upstream_pcie_bridge(struct pci_dev *pdev)
 			continue;
 		}
 		/* PCI device should connect to a PCIe bridge */
-		if (pdev->pcie_type != PCI_EXP_TYPE_PCI_BRIDGE) {
+		if (pci_pcie_type(pdev) != PCI_EXP_TYPE_PCI_BRIDGE) {
 			/* Busted hardware? */
 			WARN_ON_ONCE(1);
 			return NULL;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 03/32] PCI: remove unused field pcie_type from struct pci_dev
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 02/32] PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs Jiang Liu
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Yijing Wang, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, linux-kernel, linux-pci, Jiang Liu

From: Yijing Wang <wangyijing@huawei.com>

From: Yijing Wang <wangyijing@huawei.com>

With introduction of pci_pcie_type(), pci_dev->pcie_type field becomes
redundant, so remove it.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/probe.c |    1 -
 include/linux/pci.h |    1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 00c7bab..8bcc985 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -930,7 +930,6 @@ void set_pcie_port_type(struct pci_dev *pdev)
 	pdev->pcie_cap = pos;
 	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
 	pdev->pcie_flags_reg = reg16;
-	pdev->pcie_type = (reg16 & PCI_EXP_FLAGS_TYPE) >> 4;
 	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
 	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 95662b2..9807da5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -254,7 +254,6 @@ struct pci_dev {
 	u8		revision;	/* PCI revision, low byte of class word */
 	u8		hdr_type;	/* PCI header type (`multi' flag masked out) */
 	u8		pcie_cap;	/* PCI-E capability offset */
-	u8		pcie_type:4;	/* PCI-E device/port type */
 	u8		pcie_mpss:3;	/* PCI-E Max Payload Size Supported */
 	u8		rom_base_reg;	/* which config register controls the ROM */
 	u8		pin;  		/* which interrupt pin this device uses */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (2 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 03/32] PCI: remove unused field pcie_type from struct pci_dev Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 21:12   ` Don Dutile
  2012-07-24 16:31 ` [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation Jiang Liu
                   ` (19 subsequent siblings)
  23 siblings, 1 reply; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

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;
+	}
+
+	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;
+	}
+
+	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) */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (3 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-25 21:12   ` Don Dutile
  2012-07-24 16:31 ` [RFC PATCH v2 06/32] PCI/hotplug: " Jiang Liu
                   ` (18 subsequent siblings)
  23 siblings, 1 reply; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCI core implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c    |  260 +++++++++++---------------------------------------
 drivers/pci/probe.c  |   18 +---
 drivers/pci/quirks.c |    9 +-
 3 files changed, 66 insertions(+), 221 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 28eb55b..fd83647 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -254,38 +254,6 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
 }
 
 /**
- * pci_pcie_cap2 - query for devices' PCI_CAP_ID_EXP v2 capability structure
- * @dev: PCI device to check
- *
- * Like pci_pcie_cap() but also checks that the PCIe capability version is
- * >= 2.  Note that v1 capability structures could be sparse in that not
- * all register fields were required.  v2 requires the entire structure to
- * be present size wise, while still allowing for non-implemented registers
- * to exist but they must be hardwired to 0.
- *
- * Due to the differences in the versions of capability structures, one
- * must be careful not to try and access non-existant registers that may
- * exist in early versions - v1 - of Express devices.
- *
- * Returns the offset of the PCIe capability structure as long as the
- * capability version is >= 2; otherwise 0 is returned.
- */
-static int pci_pcie_cap2(struct pci_dev *dev)
-{
-	u16 flags;
-	int pos;
-
-	pos = pci_pcie_cap(dev);
-	if (pos) {
-		pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags);
-		if ((flags & PCI_EXP_FLAGS_VERS) < 2)
-			pos = 0;
-	}
-
-	return pos;
-}
-
-/**
  * pci_find_ext_capability - Find an extended capability
  * @dev: PCI device to query
  * @cap: capability code
@@ -854,21 +822,6 @@ EXPORT_SYMBOL(pci_choose_state);
 
 #define PCI_EXP_SAVE_REGS	7
 
-#define pcie_cap_has_devctl(type, flags)	1
-#define pcie_cap_has_lnkctl(type, flags)		\
-		((flags & PCI_EXP_FLAGS_VERS) > 1 ||	\
-		 (type == PCI_EXP_TYPE_ROOT_PORT ||	\
-		  type == PCI_EXP_TYPE_ENDPOINT ||	\
-		  type == PCI_EXP_TYPE_LEG_END))
-#define pcie_cap_has_sltctl(type, flags)		\
-		((flags & PCI_EXP_FLAGS_VERS) > 1 ||	\
-		 ((type == PCI_EXP_TYPE_ROOT_PORT) ||	\
-		  (type == PCI_EXP_TYPE_DOWNSTREAM &&	\
-		   (flags & PCI_EXP_FLAGS_SLOT))))
-#define pcie_cap_has_rtctl(type, flags)			\
-		((flags & PCI_EXP_FLAGS_VERS) > 1 ||	\
-		 (type == PCI_EXP_TYPE_ROOT_PORT ||	\
-		  type == PCI_EXP_TYPE_RC_EC))
 
 static struct pci_cap_saved_state *pci_find_saved_cap(
 	struct pci_dev *pci_dev, char cap)
@@ -885,13 +838,11 @@ static struct pci_cap_saved_state *pci_find_saved_cap(
 
 static int pci_save_pcie_state(struct pci_dev *dev)
 {
-	int type, pos, i = 0;
+	int i = 0;
 	struct pci_cap_saved_state *save_state;
 	u16 *cap;
-	u16 flags;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return 0;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
@@ -901,60 +852,35 @@ static int pci_save_pcie_state(struct pci_dev *dev)
 	}
 	cap = (u16 *)&save_state->cap.data[0];
 
-	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags);
-
-	type = pci_pcie_type(dev);
-	if (pcie_cap_has_devctl(type, flags))
-		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &cap[i++]);
-	if (pcie_cap_has_lnkctl(type, flags))
-		pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &cap[i++]);
-	if (pcie_cap_has_sltctl(type, flags))
-		pci_read_config_word(dev, pos + PCI_EXP_SLTCTL, &cap[i++]);
-	if (pcie_cap_has_rtctl(type, flags))
-		pci_read_config_word(dev, pos + PCI_EXP_RTCTL, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_RTCTL,  &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &cap[i++]);
+	pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL2, &cap[i++]);
 
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return 0;
-
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &cap[i++]);
-	pci_read_config_word(dev, pos + PCI_EXP_LNKCTL2, &cap[i++]);
-	pci_read_config_word(dev, pos + PCI_EXP_SLTCTL2, &cap[i++]);
 	return 0;
 }
 
 static void pci_restore_pcie_state(struct pci_dev *dev)
 {
-	int i = 0, pos, type;
+	int i = 0;
 	struct pci_cap_saved_state *save_state;
 	u16 *cap;
-	u16 flags;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
-	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (!save_state || pos <= 0)
+	if (!save_state || !pci_is_pcie(dev))
 		return;
 	cap = (u16 *)&save_state->cap.data[0];
 
-	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &flags);
-
-	type = pci_pcie_type(dev);
-	if (pcie_cap_has_devctl(type, flags))
-		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, cap[i++]);
-	if (pcie_cap_has_lnkctl(type, flags))
-		pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]);
-	if (pcie_cap_has_sltctl(type, flags))
-		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
-	if (pcie_cap_has_rtctl(type, flags))
-		pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
-
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return;
-
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, cap[i++]);
-	pci_write_config_word(dev, pos + PCI_EXP_LNKCTL2, cap[i++]);
-	pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_RTCTL, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, cap[i++]);
+	pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL2, cap[i++]);
 }
 
 
@@ -2068,7 +1994,6 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
  */
 void pci_enable_ari(struct pci_dev *dev)
 {
-	int pos;
 	u32 cap;
 	u16 ctrl;
 	struct pci_dev *bridge;
@@ -2076,26 +2001,20 @@ void pci_enable_ari(struct pci_dev *dev)
 	if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
 		return;
 
-	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
-	if (!pos)
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI))
 		return;
 
 	bridge = dev->bus->self;
 	if (!bridge)
 		return;
 
-	/* ARI is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(bridge);
-	if (!pos)
-		return;
-
-	pci_read_config_dword(bridge, pos + PCI_EXP_DEVCAP2, &cap);
+	pci_pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
 	if (!(cap & PCI_EXP_DEVCAP2_ARI))
 		return;
 
-	pci_read_config_word(bridge, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(bridge, PCI_EXP_DEVCTL2, &ctrl);
 	ctrl |= PCI_EXP_DEVCTL2_ARI;
-	pci_write_config_word(bridge, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(bridge, PCI_EXP_DEVCTL2, ctrl);
 
 	bridge->ari_enabled = 1;
 }
@@ -2111,20 +2030,14 @@ void pci_enable_ari(struct pci_dev *dev)
  */
 void pci_enable_ido(struct pci_dev *dev, unsigned long type)
 {
-	int pos;
 	u16 ctrl;
 
-	/* ID-based Ordering is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return;
-
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	if (type & PCI_EXP_IDO_REQUEST)
 		ctrl |= PCI_EXP_IDO_REQ_EN;
 	if (type & PCI_EXP_IDO_COMPLETION)
 		ctrl |= PCI_EXP_IDO_CMP_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 }
 EXPORT_SYMBOL(pci_enable_ido);
 
@@ -2135,20 +2048,14 @@ EXPORT_SYMBOL(pci_enable_ido);
  */
 void pci_disable_ido(struct pci_dev *dev, unsigned long type)
 {
-	int pos;
 	u16 ctrl;
 
-	/* ID-based Ordering is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return;
-
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	if (type & PCI_EXP_IDO_REQUEST)
 		ctrl &= ~PCI_EXP_IDO_REQ_EN;
 	if (type & PCI_EXP_IDO_COMPLETION)
 		ctrl &= ~PCI_EXP_IDO_CMP_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 }
 EXPORT_SYMBOL(pci_disable_ido);
 
@@ -2173,17 +2080,11 @@ EXPORT_SYMBOL(pci_disable_ido);
  */
 int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
 {
-	int pos;
 	u32 cap;
 	u16 ctrl;
 	int ret;
 
-	/* OBFF is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return -ENOTSUPP;
-
-	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2, &cap);
+	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
 	if (!(cap & PCI_EXP_OBFF_MASK))
 		return -ENOTSUPP; /* no OBFF support at all */
 
@@ -2194,7 +2095,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
 			return ret;
 	}
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	if (cap & PCI_EXP_OBFF_WAKE)
 		ctrl |= PCI_EXP_OBFF_WAKE_EN;
 	else {
@@ -2212,7 +2113,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
 			return -ENOTSUPP;
 		}
 	}
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 
 	return 0;
 }
@@ -2226,17 +2127,11 @@ EXPORT_SYMBOL(pci_enable_obff);
  */
 void pci_disable_obff(struct pci_dev *dev)
 {
-	int pos;
 	u16 ctrl;
 
-	/* OBFF is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return;
-
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	ctrl &= ~PCI_EXP_OBFF_WAKE_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 }
 EXPORT_SYMBOL(pci_disable_obff);
 
@@ -2249,15 +2144,9 @@ EXPORT_SYMBOL(pci_disable_obff);
  */
 static bool pci_ltr_supported(struct pci_dev *dev)
 {
-	int pos;
 	u32 cap;
 
-	/* LTR is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return false;
-
-	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2, &cap);
+	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
 
 	return cap & PCI_EXP_DEVCAP2_LTR;
 }
@@ -2274,22 +2163,16 @@ static bool pci_ltr_supported(struct pci_dev *dev)
  */
 int pci_enable_ltr(struct pci_dev *dev)
 {
-	int pos;
 	u16 ctrl;
 	int ret;
 
-	if (!pci_ltr_supported(dev))
-		return -ENOTSUPP;
-
-	/* LTR is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return -ENOTSUPP;
-
 	/* Only primary function can enable/disable LTR */
 	if (PCI_FUNC(dev->devfn) != 0)
 		return -EINVAL;
 
+	if (!pci_ltr_supported(dev))
+		return -ENOTSUPP;
+
 	/* Enable upstream ports first */
 	if (dev->bus->self) {
 		ret = pci_enable_ltr(dev->bus->self);
@@ -2297,9 +2180,9 @@ int pci_enable_ltr(struct pci_dev *dev)
 			return ret;
 	}
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	ctrl |= PCI_EXP_LTR_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 
 	return 0;
 }
@@ -2311,24 +2194,18 @@ EXPORT_SYMBOL(pci_enable_ltr);
  */
 void pci_disable_ltr(struct pci_dev *dev)
 {
-	int pos;
 	u16 ctrl;
 
-	if (!pci_ltr_supported(dev))
-		return;
-
-	/* LTR is a PCIe cap v2 feature */
-	pos = pci_pcie_cap2(dev);
-	if (!pos)
-		return;
-
 	/* Only primary function can enable/disable LTR */
 	if (PCI_FUNC(dev->devfn) != 0)
 		return;
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2, &ctrl);
+	if (!pci_ltr_supported(dev))
+		return;
+
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &ctrl);
 	ctrl &= ~PCI_EXP_LTR_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
 }
 EXPORT_SYMBOL(pci_disable_ltr);
 
@@ -3178,15 +3055,10 @@ EXPORT_SYMBOL(pci_set_dma_seg_boundary);
 static int pcie_flr(struct pci_dev *dev, int probe)
 {
 	int i;
-	int pos;
 	u32 cap;
 	u16 status, control;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
-		return -ENOTTY;
-
-	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP, &cap);
+	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
 	if (!(cap & PCI_EXP_DEVCAP_FLR))
 		return -ENOTTY;
 
@@ -3198,7 +3070,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
 		if (i)
 			msleep((1 << (i - 1)) * 100);
 
-		pci_read_config_word(dev, pos + PCI_EXP_DEVSTA, &status);
+		pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
 		if (!(status & PCI_EXP_DEVSTA_TRPND))
 			goto clear;
 	}
@@ -3207,9 +3079,9 @@ static int pcie_flr(struct pci_dev *dev, int probe)
 			"proceeding with reset anyway\n");
 
 clear:
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &control);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &control);
 	control |= PCI_EXP_DEVCTL_BCR_FLR;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, control);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, control);
 
 	msleep(100);
 
@@ -3577,18 +3449,11 @@ EXPORT_SYMBOL(pcix_set_mmrbc);
  */
 int pcie_get_readrq(struct pci_dev *dev)
 {
-	int ret, cap;
 	u16 ctl;
 
-	cap = pci_pcie_cap(dev);
-	if (!cap)
-		return -EINVAL;
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
 
-	ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
-	if (!ret)
-		ret = 128 << ((ctl & PCI_EXP_DEVCTL_READRQ) >> 12);
-
-	return ret;
+	return 128 << ((ctl & PCI_EXP_DEVCTL_READRQ) >> 12);
 }
 EXPORT_SYMBOL(pcie_get_readrq);
 
@@ -3602,17 +3467,16 @@ EXPORT_SYMBOL(pcie_get_readrq);
  */
 int pcie_set_readrq(struct pci_dev *dev, int rq)
 {
-	int cap, err = -EINVAL;
+	int err = -EINVAL;
 	u16 ctl, v;
 
 	if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
 		goto out;
 
-	cap = pci_pcie_cap(dev);
-	if (!cap)
+	if (!pci_is_pcie(dev))
 		goto out;
 
-	err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
+	err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
 	if (err)
 		goto out;
 	/*
@@ -3635,7 +3499,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
 	if ((ctl & PCI_EXP_DEVCTL_READRQ) != v) {
 		ctl &= ~PCI_EXP_DEVCTL_READRQ;
 		ctl |= v;
-		err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
+		err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
 	}
 
 out:
@@ -3652,18 +3516,11 @@ EXPORT_SYMBOL(pcie_set_readrq);
  */
 int pcie_get_mps(struct pci_dev *dev)
 {
-	int ret, cap;
 	u16 ctl;
 
-	cap = pci_pcie_cap(dev);
-	if (!cap)
-		return -EINVAL;
-
-	ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
-	if (!ret)
-		ret = 128 << ((ctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
 
-	return ret;
+	return 128 << ((ctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5);
 }
 
 /**
@@ -3676,7 +3533,7 @@ int pcie_get_mps(struct pci_dev *dev)
  */
 int pcie_set_mps(struct pci_dev *dev, int mps)
 {
-	int cap, err = -EINVAL;
+	int err = -EINVAL;
 	u16 ctl, v;
 
 	if (mps < 128 || mps > 4096 || !is_power_of_2(mps))
@@ -3687,18 +3544,17 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
 		goto out;
 	v <<= 5;
 
-	cap = pci_pcie_cap(dev);
-	if (!cap)
+	if (!pci_is_pcie(dev))
 		goto out;
 
-	err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
+	err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
 	if (err)
 		goto out;
 
 	if ((ctl & PCI_EXP_DEVCTL_PAYLOAD) != v) {
 		ctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
 		ctl |= v;
-		err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
+		err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
 	}
 out:
 	return err;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8bcc985..6a2eac8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -603,10 +603,11 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 		u32 linkcap;
 		u16 linksta;
 
-		pci_read_config_dword(bridge, pos + PCI_EXP_LNKCAP, &linkcap);
+		pci_pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP,
+					       &linkcap);
 		bus->max_bus_speed = pcie_link_speed[linkcap & 0xf];
 
-		pci_read_config_word(bridge, pos + PCI_EXP_LNKSTA, &linksta);
+		pci_pcie_capability_read_word(bridge, PCI_EXP_LNKSTA, &linksta);
 		pcie_update_link_speed(bus, linksta);
 	}
 }
@@ -936,17 +937,9 @@ void set_pcie_port_type(struct pci_dev *pdev)
 
 void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 {
-	int pos;
-	u16 reg16;
 	u32 reg32;
 
-	pos = pci_pcie_cap(pdev);
-	if (!pos)
-		return;
-	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
-	if (!(reg16 & PCI_EXP_FLAGS_SLOT))
-		return;
-	pci_read_config_dword(pdev, pos + PCI_EXP_SLTCAP, &reg32);
+	pci_pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &reg32);
 	if (reg32 & PCI_EXP_SLTCAP_HPC)
 		pdev->is_hotplug_bridge = 1;
 }
@@ -1160,8 +1153,7 @@ int pci_cfg_space_size(struct pci_dev *dev)
 	if (class == PCI_CLASS_BRIDGE_HOST)
 		return pci_cfg_space_size_ext(dev);
 
-	pos = pci_pcie_cap(dev);
-	if (!pos) {
+	if (!pci_is_pcie(dev)) {
 		pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
 		if (!pos)
 			goto fail;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 003f356..484284e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3081,17 +3081,14 @@ static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
 
 static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
 {
-	int pos;
-
-	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return -ENOTTY;
 
 	if (probe)
 		return 0;
 
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
-				PCI_EXP_DEVCTL_BCR_FLR);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL,
+				       PCI_EXP_DEVCTL_BCR_FLR);
 	msleep(100);
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 06/32] PCI/hotplug: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (4 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 07/32] PCI/portdrv: " Jiang Liu
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify cpihp_slot.c.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/hotplug/pcihp_slot.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c
index 8c05a18..b7bd558 100644
--- a/drivers/pci/hotplug/pcihp_slot.c
+++ b/drivers/pci/hotplug/pcihp_slot.c
@@ -102,9 +102,7 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	if (!hpp)
 		return;
 
-	/* Find PCI Express capability */
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return;
 
 	if (hpp->revision > 1) {
@@ -114,16 +112,16 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	}
 
 	/* Initialize Device Control Register */
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &reg16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &reg16);
 	reg16 = (reg16 & hpp->pci_exp_devctl_and) | hpp->pci_exp_devctl_or;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, reg16);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, reg16);
 
 	/* Initialize Link Control Register */
 	if (dev->subordinate) {
-		pci_read_config_word(dev, pos + PCI_EXP_LNKCTL, &reg16);
+		pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &reg16);
 		reg16 = (reg16 & hpp->pci_exp_lnkctl_and)
 			| hpp->pci_exp_lnkctl_or;
-		pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, reg16);
+		pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL, reg16);
 	}
 
 	/* Find Advanced Error Reporting Enhanced Capability */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 07/32] PCI/portdrv: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (5 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 06/32] PCI/hotplug: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-25  5:51   ` Kaneshige, Kenji
  2012-07-24 16:31 ` [RFC PATCH v2 08/32] PCI/pciehp: " Jiang Liu
                   ` (16 subsequent siblings)
  23 siblings, 1 reply; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe portdrv implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pcie/portdrv_core.c |   20 ++++++++------------
 drivers/pci/pcie/portdrv_pci.c  |    7 ++-----
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index bf320a9..37bff83 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -76,7 +76,6 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
 	struct msix_entry *msix_entries;
 	int idx[PCIE_PORT_DEVICE_MAXSERVICES];
 	int nr_entries, status, pos, i, nvec;
-	u16 reg16;
 	u32 reg32;
 
 	nr_entries = pci_msix_table_size(dev);
@@ -120,9 +119,7 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
 		 * the value in this field indicates which MSI-X Table entry is
 		 * used to generate the interrupt message."
 		 */
-		pos = pci_pcie_cap(dev);
-		pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
-		entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9;
+		entry = (dev->pcie_flags_reg & PCI_EXP_FLAGS_IRQ) >> 9;
 		if (entry >= nr_entries)
 			goto Error;
 
@@ -246,7 +243,7 @@ static void cleanup_service_irqs(struct pci_dev *dev)
  */
 static int get_port_device_capability(struct pci_dev *dev)
 {
-	int services = 0, pos;
+	int services = 0;
 	u16 reg16;
 	u32 reg32;
 	int cap_mask = 0;
@@ -265,11 +262,9 @@ static int get_port_device_capability(struct pci_dev *dev)
 			return 0;
 	}
 
-	pos = pci_pcie_cap(dev);
-	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
 	/* Hot-Plug Capable */
-	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 & PCI_EXP_FLAGS_SLOT)) {
-		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP, &reg32);
+	if ((cap_mask & PCIE_PORT_SERVICE_HP)) {
+		pci_pcie_capability_read_dword(dev, PCI_EXP_SLTCAP, &reg32);
 		if (reg32 & PCI_EXP_SLTCAP_HPC) {
 			services |= PCIE_PORT_SERVICE_HP;
 			/*
@@ -277,10 +272,11 @@ static int get_port_device_capability(struct pci_dev *dev)
 			 * enabled by the BIOS and the hot-plug service driver
 			 * is not loaded.
 			 */
-			pos += PCI_EXP_SLTCTL;
-			pci_read_config_word(dev, pos, &reg16);
+			pci_pcie_capability_read_word(dev,
+						      PCI_EXP_SLTCTL, &reg16);
 			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
-			pci_write_config_word(dev, pos, reg16);
+			pci_pcie_capability_write_word(dev,
+						       PCI_EXP_SLTCTL, reg16);
 		}
 	}
 	/* AER capable */
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 24d1463..1b2b378 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -64,14 +64,11 @@ __setup("pcie_ports=", pcie_port_setup);
  */
 void pcie_clear_root_pme_status(struct pci_dev *dev)
 {
-	int rtsta_pos;
 	u32 rtsta;
 
-	rtsta_pos = pci_pcie_cap(dev) + PCI_EXP_RTSTA;
-
-	pci_read_config_dword(dev, rtsta_pos, &rtsta);
+	pci_pcie_capability_read_dword(dev, PCI_EXP_RTSTA, &rtsta);
 	rtsta |= PCI_EXP_RTSTA_PME;
-	pci_write_config_dword(dev, rtsta_pos, rtsta);
+	pci_pcie_capability_write_dword(dev, PCI_EXP_RTSTA, rtsta);
 }
 
 static int pcie_portdrv_restore_config(struct pci_dev *dev)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 08/32] PCI/pciehp: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (6 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 07/32] PCI/portdrv: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 09/32] PCI/PME: " Jiang Liu
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify pciehp implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/pciehp_acpi.c |    6 +-----
 drivers/pci/hotplug/pciehp_hpc.c  |   10 +++++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
index 376d70d..751b41c 100644
--- a/drivers/pci/hotplug/pciehp_acpi.c
+++ b/drivers/pci/hotplug/pciehp_acpi.c
@@ -81,16 +81,12 @@ static struct list_head __initdata dummy_slots = LIST_HEAD_INIT(dummy_slots);
 /* Dummy driver for dumplicate name detection */
 static int __init dummy_probe(struct pcie_device *dev)
 {
-	int pos;
 	u32 slot_cap;
 	acpi_handle handle;
 	struct dummy_slot *slot, *tmp;
 	struct pci_dev *pdev = dev->port;
 
-	pos = pci_pcie_cap(pdev);
-	if (!pos)
-		return -ENODEV;
-	pci_read_config_dword(pdev, pos + PCI_EXP_SLTCAP, &slot_cap);
+	pci_pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap);
 	slot = kzalloc(sizeof(*slot), GFP_KERNEL);
 	if (!slot)
 		return -ENOMEM;
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 302451e..c81f27c 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -44,25 +44,25 @@
 static inline int pciehp_readw(struct controller *ctrl, int reg, u16 *value)
 {
 	struct pci_dev *dev = ctrl->pcie->port;
-	return pci_read_config_word(dev, pci_pcie_cap(dev) + reg, value);
+	return pci_pcie_capability_read_word(dev, reg, value);
 }
 
 static inline int pciehp_readl(struct controller *ctrl, int reg, u32 *value)
 {
 	struct pci_dev *dev = ctrl->pcie->port;
-	return pci_read_config_dword(dev, pci_pcie_cap(dev) + reg, value);
+	return pci_pcie_capability_read_dword(dev, reg, value);
 }
 
 static inline int pciehp_writew(struct controller *ctrl, int reg, u16 value)
 {
 	struct pci_dev *dev = ctrl->pcie->port;
-	return pci_write_config_word(dev, pci_pcie_cap(dev) + reg, value);
+	return pci_pcie_capability_write_word(dev, reg, value);
 }
 
 static inline int pciehp_writel(struct controller *ctrl, int reg, u32 value)
 {
 	struct pci_dev *dev = ctrl->pcie->port;
-	return pci_write_config_dword(dev, pci_pcie_cap(dev) + reg, value);
+	return pci_pcie_capability_write_dword(dev, reg, value);
 }
 
 /* Power Control Command */
@@ -855,7 +855,7 @@ struct controller *pcie_init(struct pcie_device *dev)
 		goto abort;
 	}
 	ctrl->pcie = dev;
-	if (!pci_pcie_cap(pdev)) {
+	if (!pci_is_pcie(pdev)) {
 		ctrl_err(ctrl, "Cannot find PCI Express capability\n");
 		goto abort_ctrl;
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 09/32] PCI/PME: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (7 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 08/32] PCI/pciehp: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 10/32] PCI/AER: " Jiang Liu
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe PME implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pcie/pme.c |   17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 30897bf..6cd6ab7 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -57,17 +57,14 @@ struct pcie_pme_service_data {
  */
 void pcie_pme_interrupt_enable(struct pci_dev *dev, bool enable)
 {
-	int rtctl_pos;
 	u16 rtctl;
 
-	rtctl_pos = pci_pcie_cap(dev) + PCI_EXP_RTCTL;
-
-	pci_read_config_word(dev, rtctl_pos, &rtctl);
+	pci_pcie_capability_read_word(dev, PCI_EXP_RTCTL, &rtctl);
 	if (enable)
 		rtctl |= PCI_EXP_RTCTL_PMEIE;
 	else
 		rtctl &= ~PCI_EXP_RTCTL_PMEIE;
-	pci_write_config_word(dev, rtctl_pos, rtctl);
+	pci_pcie_capability_write_word(dev, PCI_EXP_RTCTL, rtctl);
 }
 
 /**
@@ -226,18 +223,15 @@ static void pcie_pme_work_fn(struct work_struct *work)
 	struct pcie_pme_service_data *data =
 			container_of(work, struct pcie_pme_service_data, work);
 	struct pci_dev *port = data->srv->port;
-	int rtsta_pos;
 	u32 rtsta;
 
-	rtsta_pos = pci_pcie_cap(port) + PCI_EXP_RTSTA;
-
 	spin_lock_irq(&data->lock);
 
 	for (;;) {
 		if (data->noirq)
 			break;
 
-		pci_read_config_dword(port, rtsta_pos, &rtsta);
+		pci_pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
 		if (rtsta & PCI_EXP_RTSTA_PME) {
 			/*
 			 * Clear PME status of the port.  If there are other
@@ -276,17 +270,14 @@ static irqreturn_t pcie_pme_irq(int irq, void *context)
 {
 	struct pci_dev *port;
 	struct pcie_pme_service_data *data;
-	int rtsta_pos;
 	u32 rtsta;
 	unsigned long flags;
 
 	port = ((struct pcie_device *)context)->port;
 	data = get_service_data((struct pcie_device *)context);
 
-	rtsta_pos = pci_pcie_cap(port) + PCI_EXP_RTSTA;
-
 	spin_lock_irqsave(&data->lock, flags);
-	pci_read_config_dword(port, rtsta_pos, &rtsta);
+	pci_pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta);
 
 	if (!(rtsta & PCI_EXP_RTSTA_PME)) {
 		spin_unlock_irqrestore(&data->lock, flags);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 10/32] PCI/AER: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (8 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 09/32] PCI/PME: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 11/32] PCI/ASPM: " Jiang Liu
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe AER implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pcie/aer/aerdrv.c      |   16 +++++++---------
 drivers/pci/pcie/aer/aerdrv_core.c |   25 ++++++++++---------------
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index f7c6245..cadee93 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -122,19 +122,18 @@ static void set_downstream_devices_error_reporting(struct pci_dev *dev,
 static void aer_enable_rootport(struct aer_rpc *rpc)
 {
 	struct pci_dev *pdev = rpc->rpd->port;
-	int pos, aer_pos;
+	int aer_pos;
 	u16 reg16;
 	u32 reg32;
 
-	pos = pci_pcie_cap(pdev);
 	/* Clear PCIe Capability's Device Status */
-	pci_read_config_word(pdev, pos+PCI_EXP_DEVSTA, &reg16);
-	pci_write_config_word(pdev, pos+PCI_EXP_DEVSTA, reg16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_DEVSTA, &reg16);
+	pci_pcie_capability_write_word(pdev, PCI_EXP_DEVSTA, reg16);
 
 	/* Disable system error generation in response to error messages */
-	pci_read_config_word(pdev, pos + PCI_EXP_RTCTL, &reg16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_RTCTL, &reg16);
 	reg16 &= ~(SYSTEM_ERROR_INTR_ON_MESG_MASK);
-	pci_write_config_word(pdev, pos + PCI_EXP_RTCTL, reg16);
+	pci_pcie_capability_write_word(pdev, PCI_EXP_RTCTL, reg16);
 
 	aer_pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
 	/* Clear error status */
@@ -396,9 +395,8 @@ static void aer_error_resume(struct pci_dev *dev)
 	u16 reg16;
 
 	/* Clean up Root device status */
-	pos = pci_pcie_cap(dev);
-	pci_read_config_word(dev, pos + PCI_EXP_DEVSTA, &reg16);
-	pci_write_config_word(dev, pos + PCI_EXP_DEVSTA, reg16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &reg16);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVSTA, reg16);
 
 	/* Clean AER Root Error Status */
 	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index f551534..a37e277 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -35,25 +35,22 @@ module_param(nosourceid, bool, 0);
 int pci_enable_pcie_error_reporting(struct pci_dev *dev)
 {
 	u16 reg16 = 0;
-	int pos;
 
 	if (pcie_aer_get_firmware_first(dev))
 		return -EIO;
 
-	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
-	if (!pos)
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
 		return -EIO;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return -EIO;
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &reg16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &reg16);
 	reg16 |= (PCI_EXP_DEVCTL_CERE |
 		PCI_EXP_DEVCTL_NFERE |
 		PCI_EXP_DEVCTL_FERE |
 		PCI_EXP_DEVCTL_URRE);
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, reg16);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, reg16);
 
 	return 0;
 }
@@ -62,21 +59,19 @@ EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
 int pci_disable_pcie_error_reporting(struct pci_dev *dev)
 {
 	u16 reg16 = 0;
-	int pos;
 
 	if (pcie_aer_get_firmware_first(dev))
 		return -EIO;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return -EIO;
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &reg16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &reg16);
 	reg16 &= ~(PCI_EXP_DEVCTL_CERE |
 		PCI_EXP_DEVCTL_NFERE |
 		PCI_EXP_DEVCTL_FERE |
 		PCI_EXP_DEVCTL_URRE);
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, reg16);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, reg16);
 
 	return 0;
 }
@@ -151,18 +146,18 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
 	 */
 	if (atomic_read(&dev->enable_cnt) == 0)
 		return false;
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return false;
 
 	/* Check if AER is enabled */
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &reg16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &reg16);
 	if (!(reg16 & (
 		PCI_EXP_DEVCTL_CERE |
 		PCI_EXP_DEVCTL_NFERE |
 		PCI_EXP_DEVCTL_FERE |
 		PCI_EXP_DEVCTL_URRE)))
 		return false;
+
 	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
 	if (!pos)
 		return false;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 11/32] PCI/ASPM: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (9 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 10/32] PCI/AER: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 12/32] PCI/ARM: " Jiang Liu
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe ASPM implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pcie/aspm.c |   66 +++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 42 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 2591603..5c00f8c 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -125,21 +125,17 @@ static int policy_to_clkpm_state(struct pcie_link_state *link)
 
 static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable)
 {
-	int pos;
 	u16 reg16;
 	struct pci_dev *child;
 	struct pci_bus *linkbus = link->pdev->subordinate;
 
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		pos = pci_pcie_cap(child);
-		if (!pos)
-			return;
-		pci_read_config_word(child, pos + PCI_EXP_LNKCTL, &reg16);
+		pci_pcie_capability_read_word(child, PCI_EXP_LNKCTL, &reg16);
 		if (enable)
 			reg16 |= PCI_EXP_LNKCTL_CLKREQ_EN;
 		else
 			reg16 &= ~PCI_EXP_LNKCTL_CLKREQ_EN;
-		pci_write_config_word(child, pos + PCI_EXP_LNKCTL, reg16);
+		pci_pcie_capability_write_word(child, PCI_EXP_LNKCTL, reg16);
 	}
 	link->clkpm_enabled = !!enable;
 }
@@ -157,7 +153,7 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
-	int pos, capable = 1, enabled = 1;
+	int capable = 1, enabled = 1;
 	u32 reg32;
 	u16 reg16;
 	struct pci_dev *child;
@@ -165,16 +161,13 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 
 	/* All functions should have the same cap and state, take the worst */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		pos = pci_pcie_cap(child);
-		if (!pos)
-			return;
-		pci_read_config_dword(child, pos + PCI_EXP_LNKCAP, &reg32);
+		pci_pcie_capability_read_dword(child, PCI_EXP_LNKCAP, &reg32);
 		if (!(reg32 & PCI_EXP_LNKCAP_CLKPM)) {
 			capable = 0;
 			enabled = 0;
 			break;
 		}
-		pci_read_config_word(child, pos + PCI_EXP_LNKCTL, &reg16);
+		pci_pcie_capability_read_word(child, PCI_EXP_LNKCTL, &reg16);
 		if (!(reg16 & PCI_EXP_LNKCTL_CLKREQ_EN))
 			enabled = 0;
 	}
@@ -190,7 +183,7 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
  */
 static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
 {
-	int ppos, cpos, same_clock = 1;
+	int same_clock = 1;
 	u16 reg16, parent_reg, child_reg[8];
 	unsigned long start_jiffies;
 	struct pci_dev *child, *parent = link->pdev;
@@ -203,46 +196,43 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
 	BUG_ON(!pci_is_pcie(child));
 
 	/* Check downstream component if bit Slot Clock Configuration is 1 */
-	cpos = pci_pcie_cap(child);
-	pci_read_config_word(child, cpos + PCI_EXP_LNKSTA, &reg16);
+	pci_pcie_capability_read_word(child, PCI_EXP_LNKSTA, &reg16);
 	if (!(reg16 & PCI_EXP_LNKSTA_SLC))
 		same_clock = 0;
 
 	/* Check upstream component if bit Slot Clock Configuration is 1 */
-	ppos = pci_pcie_cap(parent);
-	pci_read_config_word(parent, ppos + PCI_EXP_LNKSTA, &reg16);
+	pci_pcie_capability_read_word(parent, PCI_EXP_LNKSTA, &reg16);
 	if (!(reg16 & PCI_EXP_LNKSTA_SLC))
 		same_clock = 0;
 
 	/* Configure downstream component, all functions */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		cpos = pci_pcie_cap(child);
-		pci_read_config_word(child, cpos + PCI_EXP_LNKCTL, &reg16);
+		pci_pcie_capability_read_word(child, PCI_EXP_LNKCTL, &reg16);
 		child_reg[PCI_FUNC(child->devfn)] = reg16;
 		if (same_clock)
 			reg16 |= PCI_EXP_LNKCTL_CCC;
 		else
 			reg16 &= ~PCI_EXP_LNKCTL_CCC;
-		pci_write_config_word(child, cpos + PCI_EXP_LNKCTL, reg16);
+		pci_pcie_capability_write_word(child, PCI_EXP_LNKCTL, reg16);
 	}
 
 	/* Configure upstream component */
-	pci_read_config_word(parent, ppos + PCI_EXP_LNKCTL, &reg16);
+	pci_pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &reg16);
 	parent_reg = reg16;
 	if (same_clock)
 		reg16 |= PCI_EXP_LNKCTL_CCC;
 	else
 		reg16 &= ~PCI_EXP_LNKCTL_CCC;
-	pci_write_config_word(parent, ppos + PCI_EXP_LNKCTL, reg16);
+	pci_pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
 
 	/* Retrain link */
 	reg16 |= PCI_EXP_LNKCTL_RL;
-	pci_write_config_word(parent, ppos + PCI_EXP_LNKCTL, reg16);
+	pci_pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
 
 	/* Wait for link training end. Break out after waiting for timeout */
 	start_jiffies = jiffies;
 	for (;;) {
-		pci_read_config_word(parent, ppos + PCI_EXP_LNKSTA, &reg16);
+		pci_pcie_capability_read_word(parent, PCI_EXP_LNKSTA, &reg16);
 		if (!(reg16 & PCI_EXP_LNKSTA_LT))
 			break;
 		if (time_after(jiffies, start_jiffies + LINK_RETRAIN_TIMEOUT))
@@ -256,11 +246,10 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
 	dev_printk(KERN_ERR, &parent->dev,
 		   "ASPM: Could not configure common clock\n");
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		cpos = pci_pcie_cap(child);
-		pci_write_config_word(child, cpos + PCI_EXP_LNKCTL,
-				      child_reg[PCI_FUNC(child->devfn)]);
+		pci_pcie_capability_write_word(child, PCI_EXP_LNKCTL,
+					child_reg[PCI_FUNC(child->devfn)]);
 	}
-	pci_write_config_word(parent, ppos + PCI_EXP_LNKCTL, parent_reg);
+	pci_pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_reg);
 }
 
 /* Convert L0s latency encoding to ns */
@@ -305,16 +294,14 @@ struct aspm_register_info {
 static void pcie_get_aspm_reg(struct pci_dev *pdev,
 			      struct aspm_register_info *info)
 {
-	int pos;
 	u16 reg16;
 	u32 reg32;
 
-	pos = pci_pcie_cap(pdev);
-	pci_read_config_dword(pdev, pos + PCI_EXP_LNKCAP, &reg32);
+	pci_pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &reg32);
 	info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10;
 	info->latency_encoding_l0s = (reg32 & PCI_EXP_LNKCAP_L0SEL) >> 12;
 	info->latency_encoding_l1  = (reg32 & PCI_EXP_LNKCAP_L1EL) >> 15;
-	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &reg16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &reg16);
 	info->enabled = reg16 & PCI_EXP_LNKCTL_ASPMC;
 }
 
@@ -420,7 +407,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 
 	/* Get and check endpoint acceptable latencies */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		int pos;
 		u32 reg32, encoding;
 		struct aspm_latency *acceptable =
 			&link->acceptable[PCI_FUNC(child->devfn)];
@@ -429,8 +415,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		    pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END)
 			continue;
 
-		pos = pci_pcie_cap(child);
-		pci_read_config_dword(child, pos + PCI_EXP_DEVCAP, &reg32);
+		pci_pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
 		/* Calculate endpoint L0s acceptable latency */
 		encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6;
 		acceptable->l0s = calc_l0s_acceptable(encoding);
@@ -445,12 +430,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
 {
 	u16 reg16;
-	int pos = pci_pcie_cap(pdev);
 
-	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &reg16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &reg16);
 	reg16 &= ~0x3;
 	reg16 |= val;
-	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
+	pci_pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, reg16);
 }
 
 static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
@@ -505,7 +489,6 @@ static void free_link_state(struct pcie_link_state *link)
 static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 {
 	struct pci_dev *child;
-	int pos;
 	u32 reg32;
 
 	/*
@@ -513,8 +496,7 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 	 * very strange. Disable ASPM for the whole slot
 	 */
 	list_for_each_entry(child, &pdev->subordinate->devices, bus_list) {
-		pos = pci_pcie_cap(child);
-		if (!pos)
+		if (!pci_is_pcie(child))
 			return -EINVAL;
 
 		/*
@@ -530,7 +512,7 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 		 * Disable ASPM for pre-1.1 PCIe device, we follow MS to use
 		 * RBER bit to determine if a function is 1.1 version device
 		 */
-		pci_read_config_dword(child, pos + PCI_EXP_DEVCAP, &reg32);
+		pci_pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
 		if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
 			dev_printk(KERN_INFO, &child->dev, "disabling ASPM"
 				" on pre-1.1 PCIe device.  You can enable it"
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 12/32] PCI/ARM: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (10 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 11/32] PCI/ASPM: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 13/32] PCI/MIPS: " Jiang Liu
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe ARM implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/arm/mach-tegra/pcie.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c
index 0e09137..e41e64d 100644
--- a/arch/arm/mach-tegra/pcie.c
+++ b/arch/arm/mach-tegra/pcie.c
@@ -368,16 +368,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
 static void __devinit tegra_pcie_relax_enable(struct pci_dev *dev)
 {
 	u16 val16;
-	int pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
 
-	if (pos <= 0) {
+	if (!pci_is_pcie(dev)) {
 		dev_err(&dev->dev, "skipping relaxed ordering fixup\n");
 		return;
 	}
 
-	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &val16);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &val16);
 	val16 |= PCI_EXP_DEVCTL_RELAX_EN;
-	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, val16);
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, val16);
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 13/32] PCI/MIPS: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (11 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 12/32] PCI/ARM: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 14/32] PCI/tile: " Jiang Liu
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe MIPS implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/mips/pci/pci-octeon.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/mips/pci/pci-octeon.c b/arch/mips/pci/pci-octeon.c
index 52a1ba7..fac6308 100644
--- a/arch/mips/pci/pci-octeon.c
+++ b/arch/mips/pci/pci-octeon.c
@@ -117,15 +117,14 @@ int pcibios_plat_dev_init(struct pci_dev *dev)
 	}
 
 	/* Enable the PCIe normal error reporting */
-	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (pos) {
+	if (pci_is_pcie(dev)) {
 		/* Update Device Control */
-		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &config);
+		pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &config);
 		config |= PCI_EXP_DEVCTL_CERE; /* Correctable Error Reporting */
 		config |= PCI_EXP_DEVCTL_NFERE; /* Non-Fatal Error Reporting */
 		config |= PCI_EXP_DEVCTL_FERE;  /* Fatal Error Reporting */
 		config |= PCI_EXP_DEVCTL_URRE;  /* Unsupported Request */
-		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, config);
+		pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, config);
 	}
 
 	/* Find the Advanced Error Reporting capability */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 14/32] PCI/tile: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (12 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 13/32] PCI/MIPS: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 15/32] PCI/r8169: " Jiang Liu
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify PCIe tile implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/tile/kernel/pci.c |   17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 0fdd99d..bf296cf 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -246,16 +246,13 @@ static void __devinit fixup_read_and_payload_sizes(void)
 
 	/* Scan for the smallest maximum payload size. */
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-		int pcie_caps_offset;
 		u32 devcap;
 		int max_payload;
 
-		pcie_caps_offset = pci_find_capability(dev, PCI_CAP_ID_EXP);
-		if (pcie_caps_offset == 0)
+		if (!pci_is_pcie(dev))
 			continue;
 
-		pci_read_config_dword(dev, pcie_caps_offset + PCI_EXP_DEVCAP,
-				      &devcap);
+		pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &devcap);
 		max_payload = devcap & PCI_EXP_DEVCAP_PAYLOAD;
 		if (max_payload < smallest_max_payload)
 			smallest_max_payload = max_payload;
@@ -264,19 +261,15 @@ static void __devinit fixup_read_and_payload_sizes(void)
 	/* Now, set the max_payload_size for all devices to that value. */
 	new_values = (max_read_size << 12) | (smallest_max_payload << 5);
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-		int pcie_caps_offset;
 		u16 devctl;
 
-		pcie_caps_offset = pci_find_capability(dev, PCI_CAP_ID_EXP);
-		if (pcie_caps_offset == 0)
+		if (!pci_is_pcie(dev))
 			continue;
 
-		pci_read_config_word(dev, pcie_caps_offset + PCI_EXP_DEVCTL,
-				     &devctl);
+		pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &devctl);
 		devctl &= ~(PCI_EXP_DEVCTL_PAYLOAD | PCI_EXP_DEVCTL_READRQ);
 		devctl |= new_values;
-		pci_write_config_word(dev, pcie_caps_offset + PCI_EXP_DEVCTL,
-				      devctl);
+		pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, devctl);
 	}
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 15/32] PCI/r8169: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (13 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 14/32] PCI/tile: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 16/32] PCI/broadcom: " Jiang Liu
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify r8169 driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/net/ethernet/realtek/r8169.c |   35 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 9757ce3..2f6ff78 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -807,14 +807,12 @@ static void rtl_unlock_work(struct rtl8169_private *tp)
 
 static void rtl_tx_performance_tweak(struct pci_dev *pdev, u16 force)
 {
-	int cap = pci_pcie_cap(pdev);
-
-	if (cap) {
+	if (pci_is_pcie(pdev)) {
 		u16 ctl;
 
-		pci_read_config_word(pdev, cap + PCI_EXP_DEVCTL, &ctl);
+		pci_pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &ctl);
 		ctl = (ctl & ~PCI_EXP_DEVCTL_READRQ) | force;
-		pci_write_config_word(pdev, cap + PCI_EXP_DEVCTL, ctl);
+		pci_pcie_capability_write_word(pdev, PCI_EXP_DEVCTL, ctl);
 	}
 }
 
@@ -4504,27 +4502,23 @@ static void rtl_ephy_init(void __iomem *ioaddr, const struct ephy_info *e, int l
 
 static void rtl_disable_clock_request(struct pci_dev *pdev)
 {
-	int cap = pci_pcie_cap(pdev);
-
-	if (cap) {
+	if (pci_is_pcie(pdev)) {
 		u16 ctl;
 
-		pci_read_config_word(pdev, cap + PCI_EXP_LNKCTL, &ctl);
+		pci_pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
 		ctl &= ~PCI_EXP_LNKCTL_CLKREQ_EN;
-		pci_write_config_word(pdev, cap + PCI_EXP_LNKCTL, ctl);
+		pci_pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, ctl);
 	}
 }
 
 static void rtl_enable_clock_request(struct pci_dev *pdev)
 {
-	int cap = pci_pcie_cap(pdev);
-
-	if (cap) {
+	if (pci_is_pcie(pdev)) {
 		u16 ctl;
 
-		pci_read_config_word(pdev, cap + PCI_EXP_LNKCTL, &ctl);
+		pci_pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl);
 		ctl |= PCI_EXP_LNKCTL_CLKREQ_EN;
-		pci_write_config_word(pdev, cap + PCI_EXP_LNKCTL, ctl);
+		pci_pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, ctl);
 	}
 }
 
@@ -5132,14 +5126,9 @@ static void rtl_hw_start_8101(struct net_device *dev)
 		tp->event_slow &= ~RxFIFOOver;
 
 	if (tp->mac_version == RTL_GIGA_MAC_VER_13 ||
-	    tp->mac_version == RTL_GIGA_MAC_VER_16) {
-		int cap = pci_pcie_cap(pdev);
-
-		if (cap) {
-			pci_write_config_word(pdev, cap + PCI_EXP_DEVCTL,
-					      PCI_EXP_DEVCTL_NOSNOOP_EN);
-		}
-	}
+	    tp->mac_version == RTL_GIGA_MAC_VER_16)
+		pci_pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
+					       PCI_EXP_DEVCTL_NOSNOOP_EN);
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 16/32] PCI/broadcom: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (14 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 15/32] PCI/r8169: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 17/32] PCI/igb: " Jiang Liu
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify broadcom ethernet drivers'
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |   24 +++--------
 drivers/net/ethernet/broadcom/tg3.c              |   46 +++++++++-------------
 2 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index f755a66..be87aff 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -1142,14 +1142,9 @@ static int bnx2x_send_final_clnup(struct bnx2x *bp, u8 clnup_func,
 
 static u8 bnx2x_is_pcie_pending(struct pci_dev *dev)
 {
-	int pos;
 	u16 status;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
-		return false;
-
-	pci_read_config_word(dev, pos + PCI_EXP_DEVSTA, &status);
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
 	return status & PCI_EXP_DEVSTA_TRPND;
 }
 
@@ -6114,8 +6109,7 @@ static void bnx2x_init_pxp(struct bnx2x *bp)
 	u16 devctl;
 	int r_order, w_order;
 
-	pci_read_config_word(bp->pdev,
-			     pci_pcie_cap(bp->pdev) + PCI_EXP_DEVCTL, &devctl);
+	pci_pcie_capability_read_word(bp->pdev, PCI_EXP_DEVCTL, &devctl);
 	DP(NETIF_MSG_HW, "read 0x%x from devctl\n", devctl);
 	w_order = ((devctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5);
 	if (bp->mrrs == -1)
@@ -9330,15 +9324,10 @@ static int __devinit bnx2x_prev_mark_path(struct bnx2x *bp)
 
 static bool __devinit bnx2x_can_flr(struct bnx2x *bp)
 {
-	int pos;
 	u32 cap;
 	struct pci_dev *dev = bp->pdev;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
-		return false;
-
-	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP, &cap);
+	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
 	if (!(cap & PCI_EXP_DEVCAP_FLR))
 		return false;
 
@@ -9347,7 +9336,7 @@ static bool __devinit bnx2x_can_flr(struct bnx2x *bp)
 
 static int __devinit bnx2x_do_flr(struct bnx2x *bp)
 {
-	int i, pos;
+	int i;
 	u16 status;
 	struct pci_dev *dev = bp->pdev;
 
@@ -9355,8 +9344,7 @@ static int __devinit bnx2x_do_flr(struct bnx2x *bp)
 	if (bnx2x_can_flr(bp))
 		return -ENOTTY;
 
-	pos = pci_pcie_cap(dev);
-	if (!pos)
+	if (!pci_is_pcie(dev))
 		return -ENOTTY;
 
 	/* Wait for Transaction Pending bit clean */
@@ -9364,7 +9352,7 @@ static int __devinit bnx2x_do_flr(struct bnx2x *bp)
 		if (i)
 			msleep((1 << (i - 1)) * 100);
 
-		pci_read_config_word(dev, pos + PCI_EXP_DEVSTA, &status);
+		pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &status);
 		if (!(status & PCI_EXP_DEVSTA_TRPND))
 			goto clear;
 	}
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 09fa3c6..1f613ec 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -3544,13 +3544,11 @@ static int tg3_power_down_prepare(struct tg3 *tp)
 	if (tg3_flag(tp, CLKREQ_BUG)) {
 		u16 lnkctl;
 
-		pci_read_config_word(tp->pdev,
-				     pci_pcie_cap(tp->pdev) + PCI_EXP_LNKCTL,
-				     &lnkctl);
+		pci_pcie_capability_read_word(tp->pdev,
+					      PCI_EXP_LNKCTL, &lnkctl);
 		lnkctl |= PCI_EXP_LNKCTL_CLKREQ_EN;
-		pci_write_config_word(tp->pdev,
-				      pci_pcie_cap(tp->pdev) + PCI_EXP_LNKCTL,
-				      lnkctl);
+		pci_pcie_capability_write_word(tp->pdev,
+					       PCI_EXP_LNKCTL, lnkctl);
 	}
 
 	misc_host_ctrl = tr32(TG3PCI_MISC_HOST_CTRL);
@@ -4324,18 +4322,16 @@ relink:
 	if (tg3_flag(tp, CLKREQ_BUG)) {
 		u16 oldlnkctl, newlnkctl;
 
-		pci_read_config_word(tp->pdev,
-				     pci_pcie_cap(tp->pdev) + PCI_EXP_LNKCTL,
-				     &oldlnkctl);
+		pci_pcie_capability_read_word(tp->pdev, PCI_EXP_LNKCTL,
+					      &oldlnkctl);
 		if (tp->link_config.active_speed == SPEED_100 ||
 		    tp->link_config.active_speed == SPEED_10)
 			newlnkctl = oldlnkctl & ~PCI_EXP_LNKCTL_CLKREQ_EN;
 		else
 			newlnkctl = oldlnkctl | PCI_EXP_LNKCTL_CLKREQ_EN;
 		if (newlnkctl != oldlnkctl)
-			pci_write_config_word(tp->pdev,
-					      pci_pcie_cap(tp->pdev) +
-					      PCI_EXP_LNKCTL, newlnkctl);
+			pci_pcie_capability_write_word(tp->pdev, PCI_EXP_LNKCTL,
+						       newlnkctl);
 	}
 
 	if (current_link_up != netif_carrier_ok(tp->dev)) {
@@ -7942,7 +7938,7 @@ static int tg3_chip_reset(struct tg3 *tp)
 
 	udelay(120);
 
-	if (tg3_flag(tp, PCI_EXPRESS) && pci_pcie_cap(tp->pdev)) {
+	if (tg3_flag(tp, PCI_EXPRESS) && pci_is_pcie(tp->pdev)) {
 		u16 val16;
 
 		if (tp->pci_chip_rev_id == CHIPREV_ID_5750_A0) {
@@ -7959,9 +7955,7 @@ static int tg3_chip_reset(struct tg3 *tp)
 		}
 
 		/* Clear the "no snoop" and "relaxed ordering" bits. */
-		pci_read_config_word(tp->pdev,
-				     pci_pcie_cap(tp->pdev) + PCI_EXP_DEVCTL,
-				     &val16);
+		pci_pcie_capability_read_word(tp->pdev, PCI_EXP_DEVCTL, &val16);
 		val16 &= ~(PCI_EXP_DEVCTL_RELAX_EN |
 			   PCI_EXP_DEVCTL_NOSNOOP_EN);
 		/*
@@ -7970,17 +7964,14 @@ static int tg3_chip_reset(struct tg3 *tp)
 		 */
 		if (!tg3_flag(tp, CPMU_PRESENT))
 			val16 &= ~PCI_EXP_DEVCTL_PAYLOAD;
-		pci_write_config_word(tp->pdev,
-				      pci_pcie_cap(tp->pdev) + PCI_EXP_DEVCTL,
-				      val16);
+		pci_pcie_capability_write_word(tp->pdev, PCI_EXP_DEVCTL, val16);
 
 		/* Clear error status */
-		pci_write_config_word(tp->pdev,
-				      pci_pcie_cap(tp->pdev) + PCI_EXP_DEVSTA,
-				      PCI_EXP_DEVSTA_CED |
-				      PCI_EXP_DEVSTA_NFED |
-				      PCI_EXP_DEVSTA_FED |
-				      PCI_EXP_DEVSTA_URD);
+		pci_pcie_capability_write_word(tp->pdev, PCI_EXP_DEVSTA,
+					       PCI_EXP_DEVSTA_CED |
+					       PCI_EXP_DEVSTA_NFED |
+					       PCI_EXP_DEVSTA_FED |
+					       PCI_EXP_DEVSTA_URD);
 	}
 
 	tg3_restore_pci_state(tp);
@@ -14303,9 +14294,8 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 
 		tg3_flag_set(tp, PCI_EXPRESS);
 
-		pci_read_config_word(tp->pdev,
-				     pci_pcie_cap(tp->pdev) + PCI_EXP_LNKCTL,
-				     &lnkctl);
+		pci_pcie_capability_read_word(tp->pdev,
+					      PCI_EXP_LNKCTL, &lnkctl);
 		if (lnkctl & PCI_EXP_LNKCTL_CLKREQ_EN) {
 			if (GET_ASIC_REV(tp->pci_chip_rev_id) ==
 			    ASIC_REV_5906) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 17/32] PCI/igb: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (15 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 16/32] PCI/broadcom: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 18/32] PCI/vxge: " Jiang Liu
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify Intel ethernet drivers'
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   20 ++++++++------------
 drivers/net/ethernet/intel/igb/igb_main.c  |   12 ++++--------
 2 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index a4b0435..180a795 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5582,16 +5582,15 @@ static void e1000_complete_shutdown(struct pci_dev *pdev, bool sleep,
 	 */
 	if (adapter->flags & FLAG_IS_QUAD_PORT) {
 		struct pci_dev *us_dev = pdev->bus->self;
-		int pos = pci_pcie_cap(us_dev);
 		u16 devctl;
 
-		pci_read_config_word(us_dev, pos + PCI_EXP_DEVCTL, &devctl);
-		pci_write_config_word(us_dev, pos + PCI_EXP_DEVCTL,
-		                      (devctl & ~PCI_EXP_DEVCTL_CERE));
+		pci_pcie_capability_read_word(us_dev, PCI_EXP_DEVCTL, &devctl);
+		pci_pcie_capability_write_word(us_dev, PCI_EXP_DEVCTL,
+					       (devctl & ~PCI_EXP_DEVCTL_CERE));
 
 		e1000_power_off(pdev, sleep, wake);
 
-		pci_write_config_word(us_dev, pos + PCI_EXP_DEVCTL, devctl);
+		pci_pcie_capability_write_word(us_dev, PCI_EXP_DEVCTL, devctl);
 	} else {
 		e1000_power_off(pdev, sleep, wake);
 	}
@@ -5605,25 +5604,22 @@ static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
 #else
 static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
 {
-	int pos;
 	u16 reg16;
 
 	/*
 	 * Both device and parent should have the same ASPM setting.
 	 * Disable ASPM in downstream component first and then upstream.
 	 */
-	pos = pci_pcie_cap(pdev);
-	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &reg16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &reg16);
 	reg16 &= ~state;
-	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
+	pci_pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, reg16);
 
 	if (!pdev->bus->self)
 		return;
 
-	pos = pci_pcie_cap(pdev->bus->self);
-	pci_read_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, &reg16);
+	pci_pcie_capability_read_word(pdev->bus->self, PCI_EXP_LNKCTL, &reg16);
 	reg16 &= ~state;
-	pci_write_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, reg16);
+	pci_pcie_capability_write_word(pdev->bus->self, PCI_EXP_LNKCTL, reg16);
 }
 #endif
 static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index dd3bfe8..ef28e32 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6446,13 +6446,11 @@ static int igb_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 s32 igb_read_pcie_cap_reg(struct e1000_hw *hw, u32 reg, u16 *value)
 {
 	struct igb_adapter *adapter = hw->back;
-	u16 cap_offset;
 
-	cap_offset = adapter->pdev->pcie_cap;
-	if (!cap_offset)
+	if (!pci_is_pcie(adapter->pdev))
 		return -E1000_ERR_CONFIG;
 
-	pci_read_config_word(adapter->pdev, cap_offset + reg, value);
+	pci_pcie_capability_read_word(adapter->pdev, reg, value);
 
 	return 0;
 }
@@ -6460,13 +6458,11 @@ s32 igb_read_pcie_cap_reg(struct e1000_hw *hw, u32 reg, u16 *value)
 s32 igb_write_pcie_cap_reg(struct e1000_hw *hw, u32 reg, u16 *value)
 {
 	struct igb_adapter *adapter = hw->back;
-	u16 cap_offset;
 
-	cap_offset = adapter->pdev->pcie_cap;
-	if (!cap_offset)
+	if (!pci_is_pcie(adapter->pdev))
 		return -E1000_ERR_CONFIG;
 
-	pci_write_config_word(adapter->pdev, cap_offset + reg, *value);
+	pci_pcie_capability_write_word(adapter->pdev, reg, *value);
 
 	return 0;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 18/32] PCI/vxge: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (16 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 17/32] PCI/igb: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 19/32] PCI/mlx4: " Jiang Liu
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify vxge driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/neterion/vxge/vxge-config.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/neterion/vxge/vxge-config.c b/drivers/net/ethernet/neterion/vxge/vxge-config.c
index 98e2c10..f83f0a4 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-config.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-config.c
@@ -757,7 +757,7 @@ __vxge_hw_verify_pci_e_info(struct __vxge_hw_device *hldev)
 	u16 lnk;
 
 	/* Get the negotiated link width and speed from PCI config space */
-	pci_read_config_word(dev, dev->pcie_cap + PCI_EXP_LNKSTA, &lnk);
+	pci_pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnk);
 
 	if ((lnk & PCI_EXP_LNKSTA_CLS) != 1)
 		return VXGE_HW_ERR_INVALID_PCI_INFO;
@@ -1982,7 +1982,7 @@ u16 vxge_hw_device_link_width_get(struct __vxge_hw_device *hldev)
 	struct pci_dev *dev = hldev->pdev;
 	u16 lnk;
 
-	pci_read_config_word(dev, dev->pcie_cap + PCI_EXP_LNKSTA, &lnk);
+	pci_pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnk);
 	return (lnk & VXGE_HW_PCI_EXP_LNKCAP_LNK_WIDTH) >> 4;
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 19/32] PCI/mlx4: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (17 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 18/32] PCI/vxge: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 20/32] PCI/niu: " Jiang Liu
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify mlx4 driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/reset.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/reset.c b/drivers/net/ethernet/mellanox/mlx4/reset.c
index 11e7c1c..6895214 100644
--- a/drivers/net/ethernet/mellanox/mlx4/reset.c
+++ b/drivers/net/ethernet/mellanox/mlx4/reset.c
@@ -141,16 +141,16 @@ int mlx4_reset(struct mlx4_dev *dev)
 	/* Now restore the PCI headers */
 	if (pcie_cap) {
 		devctl = hca_header[(pcie_cap + PCI_EXP_DEVCTL) / 4];
-		if (pci_write_config_word(dev->pdev, pcie_cap + PCI_EXP_DEVCTL,
-					   devctl)) {
+		if (pci_pcie_capability_write_word(dev->pdev, PCI_EXP_DEVCTL,
+						   devctl)) {
 			err = -ENODEV;
 			mlx4_err(dev, "Couldn't restore HCA PCI Express "
 				 "Device Control register, aborting.\n");
 			goto out;
 		}
 		linkctl = hca_header[(pcie_cap + PCI_EXP_LNKCTL) / 4];
-		if (pci_write_config_word(dev->pdev, pcie_cap + PCI_EXP_LNKCTL,
-					   linkctl)) {
+		if (pci_pcie_capability_write_word(dev->pdev, PCI_EXP_LNKCTL,
+						   linkctl)) {
 			err = -ENODEV;
 			mlx4_err(dev, "Couldn't restore HCA PCI Express "
 				 "Link control register, aborting.\n");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 20/32] PCI/niu: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (18 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 19/32] PCI/mlx4: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 21/32] PCI/myri10ge: " Jiang Liu
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify niu driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/sun/niu.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 703c8cc..3f914c1 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -9768,7 +9768,7 @@ static int __devinit niu_pci_init_one(struct pci_dev *pdev,
 	union niu_parent_id parent_id;
 	struct net_device *dev;
 	struct niu *np;
-	int err, pos;
+	int err;
 	u64 dma_mask;
 	u16 val16;
 
@@ -9793,8 +9793,7 @@ static int __devinit niu_pci_init_one(struct pci_dev *pdev,
 		goto err_out_disable_pdev;
 	}
 
-	pos = pci_pcie_cap(pdev);
-	if (pos <= 0) {
+	if (!pci_is_pcie(pdev)) {
 		dev_err(&pdev->dev, "Cannot find PCI Express capability, aborting\n");
 		goto err_out_free_res;
 	}
@@ -9819,14 +9818,14 @@ static int __devinit niu_pci_init_one(struct pci_dev *pdev,
 		goto err_out_free_dev;
 	}
 
-	pci_read_config_word(pdev, pos + PCI_EXP_DEVCTL, &val16);
+	pci_pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
 	val16 &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
 	val16 |= (PCI_EXP_DEVCTL_CERE |
 		  PCI_EXP_DEVCTL_NFERE |
 		  PCI_EXP_DEVCTL_FERE |
 		  PCI_EXP_DEVCTL_URRE |
 		  PCI_EXP_DEVCTL_RELAX_EN);
-	pci_write_config_word(pdev, pos + PCI_EXP_DEVCTL, val16);
+	pci_pcie_capability_write_word(pdev, PCI_EXP_DEVCTL, val16);
 
 	dma_mask = DMA_BIT_MASK(44);
 	err = pci_set_dma_mask(pdev, dma_mask);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 21/32] PCI/myri10ge: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (19 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 20/32] PCI/niu: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 22/32] PCI/chelsio: " Jiang Liu
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify myri10ge driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 90153fc..3566236 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1078,14 +1078,13 @@ static int myri10ge_reset(struct myri10ge_priv *mgp)
 #ifdef CONFIG_MYRI10GE_DCA
 static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
 {
-	int ret, cap, err;
+	int ret, err;
 	u16 ctl;
 
-	cap = pci_pcie_cap(pdev);
-	if (!cap)
+	if (!pci_is_pcie(pdev))
 		return 0;
 
-	err = pci_read_config_word(pdev, cap + PCI_EXP_DEVCTL, &ctl);
+	err = pci_pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &ctl);
 	if (err)
 		return 0;
 
@@ -1093,7 +1092,7 @@ static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
 	if (ret != on) {
 		ctl &= ~PCI_EXP_DEVCTL_RELAX_EN;
 		ctl |= (on << 4);
-		pci_write_config_word(pdev, cap + PCI_EXP_DEVCTL, ctl);
+		pci_pcie_capability_write_word(pdev, PCI_EXP_DEVCTL, ctl);
 	}
 	return ret;
 }
@@ -3200,8 +3199,7 @@ static void myri10ge_enable_ecrc(struct myri10ge_priv *mgp)
 		return;
 
 	/* check that the bridge is a root port */
-	cap = pci_pcie_cap(bridge);
-	pci_read_config_word(bridge, cap + PCI_CAP_FLAGS, &val);
+	val = bridge->pcie_flags_reg;
 	ext_type = (val & PCI_EXP_FLAGS_TYPE) >> 4;
 	if (ext_type != PCI_EXP_TYPE_ROOT_PORT) {
 		if (myri10ge_ecrc_enable > 1) {
@@ -3218,9 +3216,7 @@ static void myri10ge_enable_ecrc(struct myri10ge_priv *mgp)
 						" to force ECRC\n");
 					return;
 				}
-				cap = pci_pcie_cap(bridge);
-				pci_read_config_word(bridge,
-						     cap + PCI_CAP_FLAGS, &val);
+				val = bridge->pcie_flags_reg;
 				ext_type = (val & PCI_EXP_FLAGS_TYPE) >> 4;
 			} while (ext_type != PCI_EXP_TYPE_ROOT_PORT);
 
@@ -3335,11 +3331,10 @@ static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
 	int overridden = 0;
 
 	if (myri10ge_force_firmware == 0) {
-		int link_width, exp_cap;
+		int link_width;
 		u16 lnk;
 
-		exp_cap = pci_pcie_cap(mgp->pdev);
-		pci_read_config_word(mgp->pdev, exp_cap + PCI_EXP_LNKSTA, &lnk);
+		pci_pcie_capability_read_word(mgp->pdev, PCI_EXP_LNKSTA, &lnk);
 		link_width = (lnk >> 4) & 0x3f;
 
 		/* Check to see if Link is less than 8 or if the
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 22/32] PCI/chelsio: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (20 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 21/32] PCI/myri10ge: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 16:31 ` [RFC PATCH v2 23/32] PCI/atl1c: " Jiang Liu
  2012-07-24 21:09 ` [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Don Dutile
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify chelsio ethernet drivers'
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb3/t3_hw.c      |   19 +++++++------------
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   10 +++-------
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c      |    7 +++----
 3 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
index 44ac2f4..8fadbb3 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
@@ -3289,22 +3289,18 @@ static void config_pcie(struct adapter *adap)
 	unsigned int log2_width, pldsize;
 	unsigned int fst_trn_rx, fst_trn_tx, acklat, rpllmt;
 
-	pci_read_config_word(adap->pdev,
-			     adap->pdev->pcie_cap + PCI_EXP_DEVCTL,
-			     &val);
+	pci_pcie_capability_read_word(adap->pdev, PCI_EXP_DEVCTL, &val);
 	pldsize = (val & PCI_EXP_DEVCTL_PAYLOAD) >> 5;
 
 	pci_read_config_word(adap->pdev, 0x2, &devid);
 	if (devid == 0x37) {
-		pci_write_config_word(adap->pdev,
-				      adap->pdev->pcie_cap + PCI_EXP_DEVCTL,
+		pci_pcie_capability_write_word(adap->pdev, PCI_EXP_DEVCTL,
 				      val & ~PCI_EXP_DEVCTL_READRQ &
 				      ~PCI_EXP_DEVCTL_PAYLOAD);
 		pldsize = 0;
 	}
 
-	pci_read_config_word(adap->pdev, adap->pdev->pcie_cap + PCI_EXP_LNKCTL,
-			     &val);
+	pci_pcie_capability_read_word(adap->pdev, PCI_EXP_LNKCTL, &val);
 
 	fst_trn_tx = G_NUMFSTTRNSEQ(t3_read_reg(adap, A_PCIE_PEX_CTRL0));
 	fst_trn_rx = adap->params.rev == 0 ? fst_trn_tx :
@@ -3425,15 +3421,14 @@ out_err:
 static void get_pci_mode(struct adapter *adapter, struct pci_params *p)
 {
 	static unsigned short speed_map[] = { 33, 66, 100, 133 };
-	u32 pci_mode, pcie_cap;
+	u32 pci_mode;
 
-	pcie_cap = pci_pcie_cap(adapter->pdev);
-	if (pcie_cap) {
+	if (pci_is_pcie(adapter->pdev)) {
 		u16 val;
 
 		p->variant = PCI_VARIANT_PCIE;
-		pci_read_config_word(adapter->pdev, pcie_cap + PCI_EXP_LNKSTA,
-					&val);
+		pci_pcie_capability_read_word(adapter->pdev, PCI_EXP_LNKSTA,
+					      &val);
 		p->width = (val >> 4) & 0x3f;
 		return;
 	}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index e1f96fb..074d3ea 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -3695,14 +3695,10 @@ static void __devinit print_port_info(const struct net_device *dev)
 static void __devinit enable_pcie_relaxed_ordering(struct pci_dev *dev)
 {
 	u16 v;
-	int pos;
 
-	pos = pci_pcie_cap(dev);
-	if (pos > 0) {
-		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &v);
-		v |= PCI_EXP_DEVCTL_RELAX_EN;
-		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, v);
-	}
+	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
+	v |= PCI_EXP_DEVCTL_RELAX_EN;
+	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, v);
 }
 
 /*
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 32e1dd5..e5184e7 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -2741,11 +2741,10 @@ static void __devinit get_pci_mode(struct adapter *adapter,
 				   struct pci_params *p)
 {
 	u16 val;
-	u32 pcie_cap = pci_pcie_cap(adapter->pdev);
 
-	if (pcie_cap) {
-		pci_read_config_word(adapter->pdev, pcie_cap + PCI_EXP_LNKSTA,
-				     &val);
+	if (pci_is_pcie(adapter->pdev)) {
+		pci_pcie_capability_read_word(adapter->pdev, PCI_EXP_LNKSTA,
+					      &val);
 		p->speed = val & PCI_EXP_LNKSTA_CLS;
 		p->width = (val & PCI_EXP_LNKSTA_NLW) >> 4;
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RFC PATCH v2 23/32] PCI/atl1c: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (21 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 22/32] PCI/chelsio: " Jiang Liu
@ 2012-07-24 16:31 ` Jiang Liu
  2012-07-24 21:09 ` [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Don Dutile
  23 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-24 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci, Jiang Liu

From: Jiang Liu <jiang.liu@huawei.com>

Use PCIe capabilities access functions to simplify atl1c driver's
implementation.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index 9cc1570..1b7e1ff 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -149,7 +149,7 @@ static void atl1c_reset_pcie(struct atl1c_hw *hw, u32 flag)
 	data &= ~(PCI_ERR_UNC_DLP | PCI_ERR_UNC_FCP);
 	pci_write_config_dword(pdev, pos + PCI_ERR_UNCOR_SEVER, data);
 	/* clear error status */
-	pci_write_config_word(pdev, pci_pcie_cap(pdev) + PCI_EXP_DEVSTA,
+	pci_pcie_capability_write_word(pdev, PCI_EXP_DEVSTA,
 			PCI_EXP_DEVSTA_NFED |
 			PCI_EXP_DEVSTA_FED |
 			PCI_EXP_DEVSTA_CED |
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers
  2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
                   ` (22 preceding siblings ...)
  2012-07-24 16:31 ` [RFC PATCH v2 23/32] PCI/atl1c: " Jiang Liu
@ 2012-07-24 21:09 ` Don Dutile
  2012-07-29  2:26   ` Jiang Liu
  23 siblings, 1 reply; 34+ messages in thread
From: Don Dutile @ 2012-07-24 21:09 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci

On 07/24/2012 12:31 PM, Jiang Liu wrote:
> From: Jiang Liu<liuj97@gmail.com>
>
> As suggested by Bjorn Helgaas and Don Dutile in threads
> http://www.spinics.net/lists/linux-pci/msg15663.html, we could improve access
> to PCIe capabilities register in to way:
> 1) cache content of PCIe Capabilities Register into struct pce_dev to avoid
>     repeatedly reading this register because it's read only.
> 2) provide access functions for PCIe Capabilities registers to hide differences
>     among PCIe base specifications, so the caller don't need to handle those
>     differences.
>
> This patch set applies to
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci-next
>
> And you could pull the change set from
> https://github.com/jiangliu/linux.git topic/pcie-cap
>
> These patch set is still RFC. It provides the new interfaces and has made the
> major changes to adopt those new interfaces. But there are still several device
> drivers left untouched. Any comments about the new interfaces are welcomed,
> especially about function names:). Once we reach an agreement, I will send out
> a formal version with all needed work done.
>
> v2: 1) change return value to 0 when the register is not implemented by
>         V1 PCIe devices.
>      2) Change all driver in the source tree to use the new interfaces.
>
> Jiang Liu (29):
>    PCI: add PCIe capabilities access functions to hide differences among
>      PCIe specs
>    PCI/core: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/hotplug: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/portdrv: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/pciehp: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/PME: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/AER: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/ASPM: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/ARM: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/MIPS: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/tile: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/r8169: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/broadcom: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/igb: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/vxge: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/mlx4: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/niu: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/myri10ge: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/chelsio: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/atl1c: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/ath9k: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/iwl: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/mthca: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/qib: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/qla: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/radeon: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/tsi721: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/et131x: use PCIe capabilities access functions to simplify
>      implementation
>    PCI/rtl8192e: use PCIe capabilities access functions to simplify
>      implementation
>
> Yijing Wang (3):
>    PCI: add pcie_flags_reg into struct pci_dev to cache PCIe
>      capabilities register
>    PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type
>    PCI: remove unused field pcie_type from struct pci_dev
>
>   arch/arm/mach-tegra/pcie.c                         |    7 +-
>   arch/mips/pci/pci-octeon.c                         |    7 +-
>   arch/powerpc/platforms/powernv/pci-ioda.c          |    2 +-
>   arch/tile/kernel/pci.c                             |   17 +-
>   drivers/gpu/drm/radeon/evergreen.c                 |    9 +-
>   drivers/infiniband/hw/mthca/mthca_reset.c          |    8 +-
>   drivers/infiniband/hw/qib/qib_pcie.c               |   40 ++-
>   drivers/iommu/intel-iommu.c                        |    6 +-
>   drivers/net/ethernet/atheros/atl1c/atl1c_main.c    |    2 +-
>   drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   |   24 +-
>   drivers/net/ethernet/broadcom/tg3.c                |   46 ++--
>   drivers/net/ethernet/chelsio/cxgb3/t3_hw.c         |   19 +-
>   drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    |   10 +-
>   drivers/net/ethernet/chelsio/cxgb4/t4_hw.c         |    7 +-
>   drivers/net/ethernet/intel/e1000e/netdev.c         |   20 +-
>   drivers/net/ethernet/intel/igb/igb_main.c          |   12 +-
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |    2 +-
>   drivers/net/ethernet/mellanox/mlx4/reset.c         |    8 +-
>   drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |   21 +-
>   drivers/net/ethernet/neterion/vxge/vxge-config.c   |    4 +-
>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |    2 +-
>   drivers/net/ethernet/realtek/r8169.c               |   35 +--
>   drivers/net/ethernet/sun/niu.c                     |    9 +-
>   drivers/net/wireless/ath/ath9k/pci.c               |   18 +-
>   drivers/net/wireless/iwlegacy/common.h             |    5 +-
>   drivers/net/wireless/iwlwifi/iwl-trans-pcie.c      |    4 +-
>   drivers/net/wireless/rtlwifi/pci.c                 |    8 +-
>   drivers/pci/access.c                               |  157 ++++++++++++
>   drivers/pci/hotplug/pciehp_acpi.c                  |    6 +-
>   drivers/pci/hotplug/pciehp_hpc.c                   |   10 +-
>   drivers/pci/hotplug/pcihp_slot.c                   |   12 +-
>   drivers/pci/iov.c                                  |    6 +-
>   drivers/pci/pci.c                                  |  262 +++++---------------
>   drivers/pci/pcie/aer/aer_inject.c                  |    2 +-
>   drivers/pci/pcie/aer/aerdrv.c                      |   23 +-
>   drivers/pci/pcie/aer/aerdrv_acpi.c                 |    2 +-
>   drivers/pci/pcie/aer/aerdrv_core.c                 |   27 +-
>   drivers/pci/pcie/aspm.c                            |  108 ++++----
>   drivers/pci/pcie/pme.c                             |   23 +-
>   drivers/pci/pcie/portdrv_bus.c                     |    2 +-
>   drivers/pci/pcie/portdrv_core.c                    |   24 +-
>   drivers/pci/pcie/portdrv_pci.c                     |   15 +-
>   drivers/pci/probe.c                                |   29 +--
>   drivers/pci/quirks.c                               |    9 +-
>   drivers/pci/search.c                               |    2 +-
>   drivers/rapidio/devices/tsi721.c                   |   13 +-
>   drivers/scsi/qla2xxx/qla_init.c                    |    4 +-
>   drivers/scsi/qla2xxx/qla_nx.c                      |    8 +-
>   drivers/scsi/qla4xxx/ql4_nx.c                      |    4 +-
>   drivers/staging/et131x/et131x.c                    |    9 +-
>   drivers/staging/rtl8192e/rtl8192e/rtl_pci.c        |    8 +-
>   include/linux/pci.h                                |   17 +-
>   include/linux/pci_regs.h                           |    2 +
>   53 files changed, 510 insertions(+), 626 deletions(-)
>

Summary:
I commented on 01, 04 & 05/32;
Overall, excellent cleanup, esp. in all the various drivers,
since driver replication is common, and that'll help correct future
drivers (wishful thinking, I know...).

Note that the comment of eliminating pci_is_pcie() and incorporating
it into the pci_pcie_capability_[read,write]_[byte,word,dword]() functions
could be applied to other patches as well (06, 10 ... and that I stopped looking
due to time constraints & repetitiveness... since this is RFC, not final patch post).



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs
  2012-07-24 16:31 ` [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs Jiang Liu
@ 2012-07-24 21:12   ` Don Dutile
  2012-07-29 16:22     ` Jiang Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Don Dutile @ 2012-07-24 21:12 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Jiang Liu, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, Yijing Wang, linux-kernel,
	linux-pci

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) */


^ permalink raw reply	[flat|nested] 34+ messages in thread

* RE: [RFC PATCH v2 07/32] PCI/portdrv: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 ` [RFC PATCH v2 07/32] PCI/portdrv: " Jiang Liu
@ 2012-07-25  5:51   ` Kaneshige, Kenji
  2012-07-25  9:44     ` Jiang Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Kaneshige, Kenji @ 2012-07-25  5:51 UTC (permalink / raw)
  To: Jiang Liu, Bjorn Helgaas, Don Dutile
  Cc: Jiang Liu, Yinghai Lu, Izumi, Taku, Rafael J . Wysocki,
	Yijing Wang, linux-kernel, linux-pci

> -----Original Message-----
> From: Jiang Liu [mailto:liuj97@gmail.com]
> Sent: Wednesday, July 25, 2012 1:31 AM
> To: Bjorn Helgaas; Don Dutile
> Cc: Jiang Liu; Yinghai Lu; Izumi, Taku/泉 拓; Rafael J . Wysocki; Kaneshige,
> Kenji/金重 憲治; Yijing Wang; linux-kernel@vger.kernel.org;
> linux-pci@vger.kernel.org; Jiang Liu
> Subject: [RFC PATCH v2 07/32] PCI/portdrv: use PCIe capabilities access
> functions to simplify implementation
> 
> From: Jiang Liu <jiang.liu@huawei.com>
> 
> Use PCIe capabilities access functions to simplify PCIe portdrv
> implementation.
> 
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/pci/pcie/portdrv_core.c |   20 ++++++++------------
>  drivers/pci/pcie/portdrv_pci.c  |    7 ++-----
>  2 files changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/pcie/portdrv_core.c
> b/drivers/pci/pcie/portdrv_core.c
> index bf320a9..37bff83 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -76,7 +76,6 @@ static int pcie_port_enable_msix(struct pci_dev *dev,
> int *vectors, int mask)
>  	struct msix_entry *msix_entries;
>  	int idx[PCIE_PORT_DEVICE_MAXSERVICES];
>  	int nr_entries, status, pos, i, nvec;
> -	u16 reg16;
>  	u32 reg32;
> 
>  	nr_entries = pci_msix_table_size(dev);
> @@ -120,9 +119,7 @@ static int pcie_port_enable_msix(struct pci_dev *dev,
> int *vectors, int mask)
>  		 * the value in this field indicates which MSI-X Table entry
> is
>  		 * used to generate the interrupt message."
>  		 */
> -		pos = pci_pcie_cap(dev);
> -		pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
> -		entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9;
> +		entry = (dev->pcie_flags_reg & PCI_EXP_FLAGS_IRQ) >> 9;
>  		if (entry >= nr_entries)
>  			goto Error;

I think we need to use pci_read_config_word() for MSI setup.

"Interrupt Message Number" in the PCIe capability register can vary depending
on whether MSI or MSI-x is enabled. Please see PCIe spec for details.

Could you double-check that?

Regards,
Kenji Kaneshige



> 
> @@ -246,7 +243,7 @@ static void cleanup_service_irqs(struct pci_dev *dev)
>   */
>  static int get_port_device_capability(struct pci_dev *dev)
>  {
> -	int services = 0, pos;
> +	int services = 0;
>  	u16 reg16;
>  	u32 reg32;
>  	int cap_mask = 0;
> @@ -265,11 +262,9 @@ static int get_port_device_capability(struct pci_dev
> *dev)
>  			return 0;
>  	}
> 
> -	pos = pci_pcie_cap(dev);
> -	pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
>  	/* Hot-Plug Capable */
> -	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 &
> PCI_EXP_FLAGS_SLOT)) {
> -		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP,
> &reg32);
> +	if ((cap_mask & PCIE_PORT_SERVICE_HP)) {
> +		pci_pcie_capability_read_dword(dev, PCI_EXP_SLTCAP,
> &reg32);
>  		if (reg32 & PCI_EXP_SLTCAP_HPC) {
>  			services |= PCIE_PORT_SERVICE_HP;
>  			/*
> @@ -277,10 +272,11 @@ static int get_port_device_capability(struct pci_dev
> *dev)
>  			 * enabled by the BIOS and the hot-plug service
> driver
>  			 * is not loaded.
>  			 */
> -			pos += PCI_EXP_SLTCTL;
> -			pci_read_config_word(dev, pos, &reg16);
> +			pci_pcie_capability_read_word(dev,
> +						      PCI_EXP_SLTCTL,
> &reg16);
>  			reg16 &= ~(PCI_EXP_SLTCTL_CCIE |
> PCI_EXP_SLTCTL_HPIE);
> -			pci_write_config_word(dev, pos, reg16);
> +			pci_pcie_capability_write_word(dev,
> +						       PCI_EXP_SLTCTL,
> reg16);
>  		}
>  	}
>  	/* AER capable */
> diff --git a/drivers/pci/pcie/portdrv_pci.c
> b/drivers/pci/pcie/portdrv_pci.c
> index 24d1463..1b2b378 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -64,14 +64,11 @@ __setup("pcie_ports=", pcie_port_setup);
>   */
>  void pcie_clear_root_pme_status(struct pci_dev *dev)
>  {
> -	int rtsta_pos;
>  	u32 rtsta;
> 
> -	rtsta_pos = pci_pcie_cap(dev) + PCI_EXP_RTSTA;
> -
> -	pci_read_config_dword(dev, rtsta_pos, &rtsta);
> +	pci_pcie_capability_read_dword(dev, PCI_EXP_RTSTA, &rtsta);
>  	rtsta |= PCI_EXP_RTSTA_PME;
> -	pci_write_config_dword(dev, rtsta_pos, rtsta);
> +	pci_pcie_capability_write_dword(dev, PCI_EXP_RTSTA, rtsta);
>  }
> 
>  static int pcie_portdrv_restore_config(struct pci_dev *dev)
> --
> 1.7.9.5


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 07/32] PCI/portdrv: use PCIe capabilities access functions to simplify implementation
  2012-07-25  5:51   ` Kaneshige, Kenji
@ 2012-07-25  9:44     ` Jiang Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-25  9:44 UTC (permalink / raw)
  To: Kaneshige, Kenji
  Cc: Jiang Liu, Bjorn Helgaas, Don Dutile, Yinghai Lu, Izumi, Taku,
	Rafael J . Wysocki, Yijing Wang, linux-kernel, linux-pci

>> diff --git a/drivers/pci/pcie/portdrv_core.c
>> b/drivers/pci/pcie/portdrv_core.c
>> index bf320a9..37bff83 100644
>> --- a/drivers/pci/pcie/portdrv_core.c
>> +++ b/drivers/pci/pcie/portdrv_core.c
>> @@ -76,7 +76,6 @@ static int pcie_port_enable_msix(struct pci_dev *dev,
>> int *vectors, int mask)
>>  	struct msix_entry *msix_entries;
>>  	int idx[PCIE_PORT_DEVICE_MAXSERVICES];
>>  	int nr_entries, status, pos, i, nvec;
>> -	u16 reg16;
>>  	u32 reg32;
>>
>>  	nr_entries = pci_msix_table_size(dev);
>> @@ -120,9 +119,7 @@ static int pcie_port_enable_msix(struct pci_dev *dev,
>> int *vectors, int mask)
>>  		 * the value in this field indicates which MSI-X Table entry
>> is
>>  		 * used to generate the interrupt message."
>>  		 */
>> -		pos = pci_pcie_cap(dev);
>> -		pci_read_config_word(dev, pos + PCI_EXP_FLAGS, &reg16);
>> -		entry = (reg16 & PCI_EXP_FLAGS_IRQ) >> 9;
>> +		entry = (dev->pcie_flags_reg & PCI_EXP_FLAGS_IRQ) >> 9;
>>  		if (entry >= nr_entries)
>>  			goto Error;
> 
> I think we need to use pci_read_config_word() for MSI setup.
> 
> "Interrupt Message Number" in the PCIe capability register can vary depending
> on whether MSI or MSI-x is enabled. Please see PCIe spec for details.
> 
> Could you double-check that?
> 
> Regards,
> Kenji Kaneshige
Good catch, will revert this change.
Thanks!
Gerry


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register
  2012-07-24 16:31 ` [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register Jiang Liu
@ 2012-07-25 15:12   ` Don Dutile
  2012-07-26 13:47     ` Yijing Wang
  0 siblings, 1 reply; 34+ messages in thread
From: Don Dutile @ 2012-07-25 15:12 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Yijing Wang, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, linux-kernel, linux-pci

resending since i did a reply vs reply-all last time...

On 07/24/2012 12:31 PM, Jiang Liu wrote:
> From: Yijing Wang<wangyijing@huawei.com>
>
> From: Yijing Wang<wangyijing@huawei.com>
>
> Since PCI Express Capabilities Register is read only, cache its value
> into struct pci_dev to avoid repeatedly calling pci_read_config_*().
>
> Signed-off-by: Yijing Wang<wangyijing@huawei.com>
> Signed-off-by: Jiang Liu<liuj97@gmail.com>
> ---
>   drivers/pci/probe.c |    1 +
>   include/linux/pci.h |   10 ++++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6c143b4..6fd58df 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -929,6 +929,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
>   	pdev->is_pcie = 1;
>   	pdev->pcie_cap = pos;
>   	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS,&reg16);
> +	pdev->pcie_flags_reg = reg16;
>   	pdev->pcie_type = (reg16&  PCI_EXP_FLAGS_TYPE)>>  4;
So, given the patch below, shouldn't the above line be ?
     pdev->pcie_type = pci_pcie_type(pdev);

Missed part of patch ?

>   	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP,&reg16);
>   	pdev->pcie_mpss = reg16&  PCI_EXP_DEVCAP_PAYLOAD;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 5faa831..95662b2 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -258,6 +258,7 @@ struct pci_dev {
>   	u8		pcie_mpss:3;	/* PCI-E Max Payload Size Supported */
>   	u8		rom_base_reg;	/* which config register controls the ROM */
>   	u8		pin;  		/* which interrupt pin this device uses */
> +	u16		pcie_flags_reg;	/* cached PCI-E Capabilities Register */
>
>   	struct pci_driver *driver;	/* which driver has allocated this device */
>   	u64		dma_mask;	/* Mask of the bits of bus address this
> @@ -1650,6 +1651,15 @@ static inline bool pci_is_pcie(struct pci_dev *dev)
>   	return !!pci_pcie_cap(dev);
>   }
>
> +/**
> + * pci_pcie_type - get the PCIe device/port type
> + * @dev: PCI device
> + */
> +static inline int pci_pcie_type(const struct pci_dev *dev)
> +{
> +	return (dev->pcie_flags_reg&  PCI_EXP_FLAGS_TYPE)>>  4;
> +}
> +
>   void pci_request_acs(void);
>   bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
>   bool pci_acs_path_enabled(struct pci_dev *start,


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation
  2012-07-24 16:31 ` [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation Jiang Liu
@ 2012-07-25 21:12   ` Don Dutile
  2012-07-29  2:12     ` Jiang Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Don Dutile @ 2012-07-25 21:12 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Bjorn Helgaas, Jiang Liu, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, Yijing Wang, linux-kernel,
	linux-pci

sorry for delay... i thought i had sent it out yesterday,
but found it today when cleaning up/out the multiple of
windows/xterms on my desktop.


On 07/24/2012 12:31 PM, Jiang Liu wrote:
> From: Jiang Liu<jiang.liu@huawei.com>
>
> Use PCIe capabilities access functions to simplify PCI core implementation.
>
> Signed-off-by: Jiang Liu<liuj97@gmail.com>
> Signed-off-by: Yijing Wang<wangyijing@huawei.com>
> ---
>   drivers/pci/pci.c    |  260 +++++++++++---------------------------------------
>   drivers/pci/probe.c  |   18 +---
>   drivers/pci/quirks.c |    9 +-
>   3 files changed, 66 insertions(+), 221 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 28eb55b..fd83647 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -254,38 +254,6 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
>   }
>
>   /**
> - * pci_pcie_cap2 - query for devices' PCI_CAP_ID_EXP v2 capability structure
> - * @dev: PCI device to check
> - *
> - * Like pci_pcie_cap() but also checks that the PCIe capability version is
> - *>= 2.  Note that v1 capability structures could be sparse in that not
> - * all register fields were required.  v2 requires the entire structure to
> - * be present size wise, while still allowing for non-implemented registers
> - * to exist but they must be hardwired to 0.
> - *
> - * Due to the differences in the versions of capability structures, one
> - * must be careful not to try and access non-existant registers that may
> - * exist in early versions - v1 - of Express devices.
> - *
> - * Returns the offset of the PCIe capability structure as long as the
> - * capability version is>= 2; otherwise 0 is returned.
> - */
> -static int pci_pcie_cap2(struct pci_dev *dev)
> -{
> -	u16 flags;
> -	int pos;
> -
> -	pos = pci_pcie_cap(dev);
> -	if (pos) {
> -		pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
> -		if ((flags&  PCI_EXP_FLAGS_VERS)<  2)
> -			pos = 0;
> -	}
> -
> -	return pos;
> -}
> -
> -/**
>    * pci_find_ext_capability - Find an extended capability
>    * @dev: PCI device to query
>    * @cap: capability code
> @@ -854,21 +822,6 @@ EXPORT_SYMBOL(pci_choose_state);
>
>   #define PCI_EXP_SAVE_REGS	7
>
> -#define pcie_cap_has_devctl(type, flags)	1
> -#define pcie_cap_has_lnkctl(type, flags)		\
> -		((flags&  PCI_EXP_FLAGS_VERS)>  1 ||	\
> -		 (type == PCI_EXP_TYPE_ROOT_PORT ||	\
> -		  type == PCI_EXP_TYPE_ENDPOINT ||	\
> -		  type == PCI_EXP_TYPE_LEG_END))
> -#define pcie_cap_has_sltctl(type, flags)		\
> -		((flags&  PCI_EXP_FLAGS_VERS)>  1 ||	\
> -		 ((type == PCI_EXP_TYPE_ROOT_PORT) ||	\
> -		  (type == PCI_EXP_TYPE_DOWNSTREAM&&	\
> -		   (flags&  PCI_EXP_FLAGS_SLOT))))
> -#define pcie_cap_has_rtctl(type, flags)			\
> -		((flags&  PCI_EXP_FLAGS_VERS)>  1 ||	\
> -		 (type == PCI_EXP_TYPE_ROOT_PORT ||	\
> -		  type == PCI_EXP_TYPE_RC_EC))
>
>   static struct pci_cap_saved_state *pci_find_saved_cap(
>   	struct pci_dev *pci_dev, char cap)
> @@ -885,13 +838,11 @@ static struct pci_cap_saved_state *pci_find_saved_cap(
>
>   static int pci_save_pcie_state(struct pci_dev *dev)
>   {
> -	int type, pos, i = 0;
> +	int i = 0;
>   	struct pci_cap_saved_state *save_state;
>   	u16 *cap;
> -	u16 flags;
>
> -	pos = pci_pcie_cap(dev);
> -	if (!pos)
> +	if (!pci_is_pcie(dev))
>   		return 0;
>
Why not put the above check in all the pci_pcie_capability_read_*() functions ?
Myron did some good cleanup to get rid of these pci_is_pcie() throughout
these functions, and put them in base functions, so it'd be good if that
effort could remain intact.

>   	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> @@ -901,60 +852,35 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>   	}
>   	cap = (u16 *)&save_state->cap.data[0];
>
> -	pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
> -
> -	type = pci_pcie_type(dev);
> -	if (pcie_cap_has_devctl(type, flags))
> -		pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&cap[i++]);
> -	if (pcie_cap_has_lnkctl(type, flags))
> -		pci_read_config_word(dev, pos + PCI_EXP_LNKCTL,&cap[i++]);
> -	if (pcie_cap_has_sltctl(type, flags))
> -		pci_read_config_word(dev, pos + PCI_EXP_SLTCTL,&cap[i++]);
> -	if (pcie_cap_has_rtctl(type, flags))
> -		pci_read_config_word(dev, pos + PCI_EXP_RTCTL,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_RTCTL,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL2,&cap[i++]);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL2,&cap[i++]);
>
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return 0;
> -
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&cap[i++]);
> -	pci_read_config_word(dev, pos + PCI_EXP_LNKCTL2,&cap[i++]);
> -	pci_read_config_word(dev, pos + PCI_EXP_SLTCTL2,&cap[i++]);
>   	return 0;
>   }
>
>   static void pci_restore_pcie_state(struct pci_dev *dev)
>   {
> -	int i = 0, pos, type;
> +	int i = 0;
>   	struct pci_cap_saved_state *save_state;
>   	u16 *cap;
> -	u16 flags;
>
>   	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> -	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> -	if (!save_state || pos<= 0)
> +	if (!save_state || !pci_is_pcie(dev))
won't need pci_is_pcie() check if put in pci_pcie_capability_write_*() below...
>   		return;
>   	cap = (u16 *)&save_state->cap.data[0];
>
> -	pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
> -
> -	type = pci_pcie_type(dev);
> -	if (pcie_cap_has_devctl(type, flags))
> -		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, cap[i++]);
> -	if (pcie_cap_has_lnkctl(type, flags))
> -		pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]);
> -	if (pcie_cap_has_sltctl(type, flags))
> -		pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
> -	if (pcie_cap_has_rtctl(type, flags))
> -		pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
> -
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return;
> -
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, cap[i++]);
> -	pci_write_config_word(dev, pos + PCI_EXP_LNKCTL2, cap[i++]);
> -	pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_RTCTL, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, cap[i++]);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL2, cap[i++]);
>   }
>
>
> @@ -2068,7 +1994,6 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
>    */
>   void pci_enable_ari(struct pci_dev *dev)
>   {
> -	int pos;
>   	u32 cap;
>   	u16 ctrl;
>   	struct pci_dev *bridge;
> @@ -2076,26 +2001,20 @@ void pci_enable_ari(struct pci_dev *dev)
>   	if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
>   		return;
>
> -	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
> -	if (!pos)
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI))
>   		return;
>
>   	bridge = dev->bus->self;
>   	if (!bridge)
>   		return;
>
> -	/* ARI is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(bridge);
> -	if (!pos)
> -		return;
> -
> -	pci_read_config_dword(bridge, pos + PCI_EXP_DEVCAP2,&cap);
> +	pci_pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2,&cap);
>   	if (!(cap&  PCI_EXP_DEVCAP2_ARI))
>   		return;
>
> -	pci_read_config_word(bridge, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(bridge, PCI_EXP_DEVCTL2,&ctrl);
>   	ctrl |= PCI_EXP_DEVCTL2_ARI;
> -	pci_write_config_word(bridge, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(bridge, PCI_EXP_DEVCTL2, ctrl);
>
>   	bridge->ari_enabled = 1;
>   }
> @@ -2111,20 +2030,14 @@ void pci_enable_ari(struct pci_dev *dev)
>    */
>   void pci_enable_ido(struct pci_dev *dev, unsigned long type)
>   {
> -	int pos;
>   	u16 ctrl;
>
> -	/* ID-based Ordering is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return;
> -
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	if (type&  PCI_EXP_IDO_REQUEST)
>   		ctrl |= PCI_EXP_IDO_REQ_EN;
>   	if (type&  PCI_EXP_IDO_COMPLETION)
>   		ctrl |= PCI_EXP_IDO_CMP_EN;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>   }
>   EXPORT_SYMBOL(pci_enable_ido);
>
> @@ -2135,20 +2048,14 @@ EXPORT_SYMBOL(pci_enable_ido);
>    */
>   void pci_disable_ido(struct pci_dev *dev, unsigned long type)
>   {
> -	int pos;
>   	u16 ctrl;
>
> -	/* ID-based Ordering is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return;
> -
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	if (type&  PCI_EXP_IDO_REQUEST)
>   		ctrl&= ~PCI_EXP_IDO_REQ_EN;
>   	if (type&  PCI_EXP_IDO_COMPLETION)
>   		ctrl&= ~PCI_EXP_IDO_CMP_EN;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>   }
>   EXPORT_SYMBOL(pci_disable_ido);
>
> @@ -2173,17 +2080,11 @@ EXPORT_SYMBOL(pci_disable_ido);
>    */
>   int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>   {
> -	int pos;
>   	u32 cap;
>   	u16 ctrl;
>   	int ret;
>
> -	/* OBFF is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return -ENOTSUPP;
> -
> -	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2,&cap);
> +	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2,&cap);
>   	if (!(cap&  PCI_EXP_OBFF_MASK))
>   		return -ENOTSUPP; /* no OBFF support at all */
>
> @@ -2194,7 +2095,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>   			return ret;
>   	}
>
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	if (cap&  PCI_EXP_OBFF_WAKE)
>   		ctrl |= PCI_EXP_OBFF_WAKE_EN;
>   	else {
> @@ -2212,7 +2113,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>   			return -ENOTSUPP;
>   		}
>   	}
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>
>   	return 0;
>   }
> @@ -2226,17 +2127,11 @@ EXPORT_SYMBOL(pci_enable_obff);
>    */
>   void pci_disable_obff(struct pci_dev *dev)
>   {
> -	int pos;
>   	u16 ctrl;
>
> -	/* OBFF is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return;
> -
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	ctrl&= ~PCI_EXP_OBFF_WAKE_EN;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>   }
>   EXPORT_SYMBOL(pci_disable_obff);
>
> @@ -2249,15 +2144,9 @@ EXPORT_SYMBOL(pci_disable_obff);
>    */
>   static bool pci_ltr_supported(struct pci_dev *dev)
>   {
> -	int pos;
>   	u32 cap;
>
> -	/* LTR is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return false;
> -
> -	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2,&cap);
> +	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2,&cap);
>
>   	return cap&  PCI_EXP_DEVCAP2_LTR;
>   }
> @@ -2274,22 +2163,16 @@ static bool pci_ltr_supported(struct pci_dev *dev)
>    */
>   int pci_enable_ltr(struct pci_dev *dev)
>   {
> -	int pos;
>   	u16 ctrl;
>   	int ret;
>
> -	if (!pci_ltr_supported(dev))
> -		return -ENOTSUPP;
> -
> -	/* LTR is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return -ENOTSUPP;
> -
>   	/* Only primary function can enable/disable LTR */
>   	if (PCI_FUNC(dev->devfn) != 0)
>   		return -EINVAL;
>
> +	if (!pci_ltr_supported(dev))
> +		return -ENOTSUPP;
> +
>   	/* Enable upstream ports first */
>   	if (dev->bus->self) {
>   		ret = pci_enable_ltr(dev->bus->self);
> @@ -2297,9 +2180,9 @@ int pci_enable_ltr(struct pci_dev *dev)
>   			return ret;
>   	}
>
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	ctrl |= PCI_EXP_LTR_EN;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>
>   	return 0;
>   }
> @@ -2311,24 +2194,18 @@ EXPORT_SYMBOL(pci_enable_ltr);
>    */
>   void pci_disable_ltr(struct pci_dev *dev)
>   {
> -	int pos;
>   	u16 ctrl;
>
> -	if (!pci_ltr_supported(dev))
> -		return;
> -
> -	/* LTR is a PCIe cap v2 feature */
> -	pos = pci_pcie_cap2(dev);
> -	if (!pos)
> -		return;
> -
>   	/* Only primary function can enable/disable LTR */
>   	if (PCI_FUNC(dev->devfn) != 0)
>   		return;
>
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
> +	if (!pci_ltr_supported(dev))
> +		return;
> +
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>   	ctrl&= ~PCI_EXP_LTR_EN;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>   }
>   EXPORT_SYMBOL(pci_disable_ltr);
>
> @@ -3178,15 +3055,10 @@ EXPORT_SYMBOL(pci_set_dma_seg_boundary);
>   static int pcie_flr(struct pci_dev *dev, int probe)
>   {
>   	int i;
> -	int pos;
>   	u32 cap;
>   	u16 status, control;
>
> -	pos = pci_pcie_cap(dev);
> -	if (!pos)
> -		return -ENOTTY;
> -
> -	pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP,&cap);
> +	pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP,&cap);
>   	if (!(cap&  PCI_EXP_DEVCAP_FLR))
>   		return -ENOTTY;
>
> @@ -3198,7 +3070,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>   		if (i)
>   			msleep((1<<  (i - 1)) * 100);
>
> -		pci_read_config_word(dev, pos + PCI_EXP_DEVSTA,&status);
> +		pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA,&status);
>   		if (!(status&  PCI_EXP_DEVSTA_TRPND))
>   			goto clear;
>   	}
> @@ -3207,9 +3079,9 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>   			"proceeding with reset anyway\n");
>
>   clear:
> -	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&control);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&control);
>   	control |= PCI_EXP_DEVCTL_BCR_FLR;
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, control);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, control);
>
>   	msleep(100);
>
> @@ -3577,18 +3449,11 @@ EXPORT_SYMBOL(pcix_set_mmrbc);
>    */
>   int pcie_get_readrq(struct pci_dev *dev)
>   {
> -	int ret, cap;
>   	u16 ctl;
>
> -	cap = pci_pcie_cap(dev);
> -	if (!cap)
> -		return -EINVAL;
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>
> -	ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
> -	if (!ret)
> -		ret = 128<<  ((ctl&  PCI_EXP_DEVCTL_READRQ)>>  12);
> -
> -	return ret;
> +	return 128<<  ((ctl&  PCI_EXP_DEVCTL_READRQ)>>  12);
>   }
>   EXPORT_SYMBOL(pcie_get_readrq);
>
> @@ -3602,17 +3467,16 @@ EXPORT_SYMBOL(pcie_get_readrq);
>    */
>   int pcie_set_readrq(struct pci_dev *dev, int rq)
>   {
> -	int cap, err = -EINVAL;
> +	int err = -EINVAL;
>   	u16 ctl, v;
>
>   	if (rq<  128 || rq>  4096 || !is_power_of_2(rq))
>   		goto out;
>
> -	cap = pci_pcie_cap(dev);
> -	if (!cap)
> +	if (!pci_is_pcie(dev))
ditto ^^^

>   		goto out;
>
> -	err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
> +	err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>   	if (err)
>   		goto out;
>   	/*
> @@ -3635,7 +3499,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>   	if ((ctl&  PCI_EXP_DEVCTL_READRQ) != v) {
>   		ctl&= ~PCI_EXP_DEVCTL_READRQ;
>   		ctl |= v;
> -		err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
> +		err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
>   	}
>
>   out:
> @@ -3652,18 +3516,11 @@ EXPORT_SYMBOL(pcie_set_readrq);
>    */
>   int pcie_get_mps(struct pci_dev *dev)
>   {
> -	int ret, cap;
>   	u16 ctl;
>
> -	cap = pci_pcie_cap(dev);
> -	if (!cap)
> -		return -EINVAL;
> -
> -	ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
> -	if (!ret)
> -		ret = 128<<  ((ctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5);
> +	pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>
> -	return ret;
> +	return 128<<  ((ctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5);
>   }
>
>   /**
> @@ -3676,7 +3533,7 @@ int pcie_get_mps(struct pci_dev *dev)
>    */
>   int pcie_set_mps(struct pci_dev *dev, int mps)
>   {
> -	int cap, err = -EINVAL;
> +	int err = -EINVAL;
>   	u16 ctl, v;
>
>   	if (mps<  128 || mps>  4096 || !is_power_of_2(mps))
> @@ -3687,18 +3544,17 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
>   		goto out;
>   	v<<= 5;
>
> -	cap = pci_pcie_cap(dev);
> -	if (!cap)
> +	if (!pci_is_pcie(dev))
ditto ^^^^^^^^^^
>   		goto out;
>
> -	err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
> +	err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>   	if (err)
>   		goto out;
>
>   	if ((ctl&  PCI_EXP_DEVCTL_PAYLOAD) != v) {
>   		ctl&= ~PCI_EXP_DEVCTL_PAYLOAD;
>   		ctl |= v;
> -		err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
> +		err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
>   	}
>   out:
>   	return err;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8bcc985..6a2eac8 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -603,10 +603,11 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>   		u32 linkcap;
>   		u16 linksta;
>
> -		pci_read_config_dword(bridge, pos + PCI_EXP_LNKCAP,&linkcap);
> +		pci_pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP,
> +					&linkcap);
>   		bus->max_bus_speed = pcie_link_speed[linkcap&  0xf];
>
> -		pci_read_config_word(bridge, pos + PCI_EXP_LNKSTA,&linksta);
> +		pci_pcie_capability_read_word(bridge, PCI_EXP_LNKSTA,&linksta);
>   		pcie_update_link_speed(bus, linksta);
>   	}
>   }
> @@ -936,17 +937,9 @@ void set_pcie_port_type(struct pci_dev *pdev)
>
>   void set_pcie_hotplug_bridge(struct pci_dev *pdev)
>   {
> -	int pos;
> -	u16 reg16;
>   	u32 reg32;
>
> -	pos = pci_pcie_cap(pdev);
> -	if (!pos)
> -		return;
> -	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS,&reg16);
> -	if (!(reg16&  PCI_EXP_FLAGS_SLOT))
> -		return;
> -	pci_read_config_dword(pdev, pos + PCI_EXP_SLTCAP,&reg32);
> +	pci_pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP,&reg32);
>   	if (reg32&  PCI_EXP_SLTCAP_HPC)
>   		pdev->is_hotplug_bridge = 1;
>   }
> @@ -1160,8 +1153,7 @@ int pci_cfg_space_size(struct pci_dev *dev)
>   	if (class == PCI_CLASS_BRIDGE_HOST)
>   		return pci_cfg_space_size_ext(dev);
>
> -	pos = pci_pcie_cap(dev);
> -	if (!pos) {
> +	if (!pci_is_pcie(dev)) {
>   		pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>   		if (!pos)
>   			goto fail;
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 003f356..484284e 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3081,17 +3081,14 @@ static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
>
>   static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
>   {
> -	int pos;
> -
> -	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> -	if (!pos)
> +	if (!pci_is_pcie(dev))
ditto if check in capability_write as well

>   		return -ENOTTY;
>
>   	if (probe)
>   		return 0;
>
> -	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> -				PCI_EXP_DEVCTL_BCR_FLR);
> +	pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL,
> +				       PCI_EXP_DEVCTL_BCR_FLR);
>   	msleep(100);
>
>   	return 0;


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register
  2012-07-25 15:12   ` Don Dutile
@ 2012-07-26 13:47     ` Yijing Wang
  0 siblings, 0 replies; 34+ messages in thread
From: Yijing Wang @ 2012-07-26 13:47 UTC (permalink / raw)
  To: Don Dutile
  Cc: Jiang Liu, Bjorn Helgaas, Yijing Wang, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, linux-kernel, linux-pci

于 2012-07-25 23:12, Don Dutile 写道:
> resending since i did a reply vs reply-all last time...
> 
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> From: Yijing Wang<wangyijing@huawei.com>
>>
>> From: Yijing Wang<wangyijing@huawei.com>
>>
>> Since PCI Express Capabilities Register is read only, cache its value
>> into struct pci_dev to avoid repeatedly calling pci_read_config_*().
>>
>> Signed-off-by: Yijing Wang<wangyijing@huawei.com>
>> Signed-off-by: Jiang Liu<liuj97@gmail.com>
>> ---
>>   drivers/pci/probe.c |    1 +
>>   include/linux/pci.h |   10 ++++++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 6c143b4..6fd58df 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -929,6 +929,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
>>       pdev->is_pcie = 1;
>>       pdev->pcie_cap = pos;
>>       pci_read_config_word(pdev, pos + PCI_EXP_FLAGS,&reg16);
>> +    pdev->pcie_flags_reg = reg16;
>>       pdev->pcie_type = (reg16&  PCI_EXP_FLAGS_TYPE)>>  4;
> So, given the patch below, shouldn't the above line be ?
>     pdev->pcie_type = pci_pcie_type(pdev);
> 
Hi,Don Dutile,thanks for your comments very much!
> Missed part of patch ?
pdev->pcie_type was removed in the later patch([RFC PATCH v2 03/32] PCI: remove unused field pcie_type from struct pci_dev),
so I leave it here unmodified.
But your suggestion let this patch become more reasonable.
Thanks.
> 
>>       pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP,&reg16);
>>       pdev->pcie_mpss = reg16&  PCI_EXP_DEVCAP_PAYLOAD;
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 5faa831..95662b2 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -258,6 +258,7 @@ struct pci_dev {
>>       u8        pcie_mpss:3;    /* PCI-E Max Payload Size Supported */
>>       u8        rom_base_reg;    /* which config register controls the ROM */
>>       u8        pin;          /* which interrupt pin this device uses */
>> +    u16        pcie_flags_reg;    /* cached PCI-E Capabilities Register */
>>
>>       struct pci_driver *driver;    /* which driver has allocated this device */
>>       u64        dma_mask;    /* Mask of the bits of bus address this
>> @@ -1650,6 +1651,15 @@ static inline bool pci_is_pcie(struct pci_dev *dev)
>>       return !!pci_pcie_cap(dev);
>>   }
>>
>> +/**
>> + * pci_pcie_type - get the PCIe device/port type
>> + * @dev: PCI device
>> + */
>> +static inline int pci_pcie_type(const struct pci_dev *dev)
>> +{
>> +    return (dev->pcie_flags_reg&  PCI_EXP_FLAGS_TYPE)>>  4;
>> +}
>> +
>>   void pci_request_acs(void);
>>   bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
>>   bool pci_acs_path_enabled(struct pci_dev *start,
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation
  2012-07-25 21:12   ` Don Dutile
@ 2012-07-29  2:12     ` Jiang Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-29  2:12 UTC (permalink / raw)
  To: Don Dutile
  Cc: Bjorn Helgaas, Jiang Liu, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, Yijing Wang, linux-kernel,
	linux-pci

Hi Dan,
	Sorry for slow response, was busy with a patch for a new PCI hotplug
framework patch set last week.
	Thanks for your comments, and seems I need to be more aggressive to
reduce redundant pci_is_pcie() checks:) Will send out V3 to simplify code
further.
	Thanks!
	Gerry

On 07/26/2012 05:12 AM, Don Dutile wrote:
> sorry for delay... i thought i had sent it out yesterday,
> but found it today when cleaning up/out the multiple of
> windows/xterms on my desktop.
> 
> 
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> From: Jiang Liu<jiang.liu@huawei.com>
>>
>> Use PCIe capabilities access functions to simplify PCI core implementation.
>>
>> Signed-off-by: Jiang Liu<liuj97@gmail.com>
>> Signed-off-by: Yijing Wang<wangyijing@huawei.com>
>> ---
>>   drivers/pci/pci.c    |  260 +++++++++++---------------------------------------
>>   drivers/pci/probe.c  |   18 +---
>>   drivers/pci/quirks.c |    9 +-
>>   3 files changed, 66 insertions(+), 221 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 28eb55b..fd83647 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -254,38 +254,6 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
>>   }
>>
>>   /**
>> - * pci_pcie_cap2 - query for devices' PCI_CAP_ID_EXP v2 capability structure
>> - * @dev: PCI device to check
>> - *
>> - * Like pci_pcie_cap() but also checks that the PCIe capability version is
>> - *>= 2.  Note that v1 capability structures could be sparse in that not
>> - * all register fields were required.  v2 requires the entire structure to
>> - * be present size wise, while still allowing for non-implemented registers
>> - * to exist but they must be hardwired to 0.
>> - *
>> - * Due to the differences in the versions of capability structures, one
>> - * must be careful not to try and access non-existant registers that may
>> - * exist in early versions - v1 - of Express devices.
>> - *
>> - * Returns the offset of the PCIe capability structure as long as the
>> - * capability version is>= 2; otherwise 0 is returned.
>> - */
>> -static int pci_pcie_cap2(struct pci_dev *dev)
>> -{
>> -    u16 flags;
>> -    int pos;
>> -
>> -    pos = pci_pcie_cap(dev);
>> -    if (pos) {
>> -        pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
>> -        if ((flags&  PCI_EXP_FLAGS_VERS)<  2)
>> -            pos = 0;
>> -    }
>> -
>> -    return pos;
>> -}
>> -
>> -/**
>>    * pci_find_ext_capability - Find an extended capability
>>    * @dev: PCI device to query
>>    * @cap: capability code
>> @@ -854,21 +822,6 @@ EXPORT_SYMBOL(pci_choose_state);
>>
>>   #define PCI_EXP_SAVE_REGS    7
>>
>> -#define pcie_cap_has_devctl(type, flags)    1
>> -#define pcie_cap_has_lnkctl(type, flags)        \
>> -        ((flags&  PCI_EXP_FLAGS_VERS)>  1 ||    \
>> -         (type == PCI_EXP_TYPE_ROOT_PORT ||    \
>> -          type == PCI_EXP_TYPE_ENDPOINT ||    \
>> -          type == PCI_EXP_TYPE_LEG_END))
>> -#define pcie_cap_has_sltctl(type, flags)        \
>> -        ((flags&  PCI_EXP_FLAGS_VERS)>  1 ||    \
>> -         ((type == PCI_EXP_TYPE_ROOT_PORT) ||    \
>> -          (type == PCI_EXP_TYPE_DOWNSTREAM&&    \
>> -           (flags&  PCI_EXP_FLAGS_SLOT))))
>> -#define pcie_cap_has_rtctl(type, flags)            \
>> -        ((flags&  PCI_EXP_FLAGS_VERS)>  1 ||    \
>> -         (type == PCI_EXP_TYPE_ROOT_PORT ||    \
>> -          type == PCI_EXP_TYPE_RC_EC))
>>
>>   static struct pci_cap_saved_state *pci_find_saved_cap(
>>       struct pci_dev *pci_dev, char cap)
>> @@ -885,13 +838,11 @@ static struct pci_cap_saved_state *pci_find_saved_cap(
>>
>>   static int pci_save_pcie_state(struct pci_dev *dev)
>>   {
>> -    int type, pos, i = 0;
>> +    int i = 0;
>>       struct pci_cap_saved_state *save_state;
>>       u16 *cap;
>> -    u16 flags;
>>
>> -    pos = pci_pcie_cap(dev);
>> -    if (!pos)
>> +    if (!pci_is_pcie(dev))
>>           return 0;
>>
> Why not put the above check in all the pci_pcie_capability_read_*() functions ?
> Myron did some good cleanup to get rid of these pci_is_pcie() throughout
> these functions, and put them in base functions, so it'd be good if that
> effort could remain intact.
> 
>>       save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
>> @@ -901,60 +852,35 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>>       }
>>       cap = (u16 *)&save_state->cap.data[0];
>>
>> -    pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
>> -
>> -    type = pci_pcie_type(dev);
>> -    if (pcie_cap_has_devctl(type, flags))
>> -        pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&cap[i++]);
>> -    if (pcie_cap_has_lnkctl(type, flags))
>> -        pci_read_config_word(dev, pos + PCI_EXP_LNKCTL,&cap[i++]);
>> -    if (pcie_cap_has_sltctl(type, flags))
>> -        pci_read_config_word(dev, pos + PCI_EXP_SLTCTL,&cap[i++]);
>> -    if (pcie_cap_has_rtctl(type, flags))
>> -        pci_read_config_word(dev, pos + PCI_EXP_RTCTL,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_RTCTL,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_LNKCTL2,&cap[i++]);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_SLTCTL2,&cap[i++]);
>>
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return 0;
>> -
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&cap[i++]);
>> -    pci_read_config_word(dev, pos + PCI_EXP_LNKCTL2,&cap[i++]);
>> -    pci_read_config_word(dev, pos + PCI_EXP_SLTCTL2,&cap[i++]);
>>       return 0;
>>   }
>>
>>   static void pci_restore_pcie_state(struct pci_dev *dev)
>>   {
>> -    int i = 0, pos, type;
>> +    int i = 0;
>>       struct pci_cap_saved_state *save_state;
>>       u16 *cap;
>> -    u16 flags;
>>
>>       save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
>> -    pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
>> -    if (!save_state || pos<= 0)
>> +    if (!save_state || !pci_is_pcie(dev))
> won't need pci_is_pcie() check if put in pci_pcie_capability_write_*() below...
>>           return;
>>       cap = (u16 *)&save_state->cap.data[0];
>>
>> -    pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
>> -
>> -    type = pci_pcie_type(dev);
>> -    if (pcie_cap_has_devctl(type, flags))
>> -        pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, cap[i++]);
>> -    if (pcie_cap_has_lnkctl(type, flags))
>> -        pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]);
>> -    if (pcie_cap_has_sltctl(type, flags))
>> -        pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
>> -    if (pcie_cap_has_rtctl(type, flags))
>> -        pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
>> -
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return;
>> -
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, cap[i++]);
>> -    pci_write_config_word(dev, pos + PCI_EXP_LNKCTL2, cap[i++]);
>> -    pci_write_config_word(dev, pos + PCI_EXP_SLTCTL2, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_RTCTL, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, cap[i++]);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_SLTCTL2, cap[i++]);
>>   }
>>
>>
>> @@ -2068,7 +1994,6 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
>>    */
>>   void pci_enable_ari(struct pci_dev *dev)
>>   {
>> -    int pos;
>>       u32 cap;
>>       u16 ctrl;
>>       struct pci_dev *bridge;
>> @@ -2076,26 +2001,20 @@ void pci_enable_ari(struct pci_dev *dev)
>>       if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
>>           return;
>>
>> -    pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
>> -    if (!pos)
>> +    if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI))
>>           return;
>>
>>       bridge = dev->bus->self;
>>       if (!bridge)
>>           return;
>>
>> -    /* ARI is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(bridge);
>> -    if (!pos)
>> -        return;
>> -
>> -    pci_read_config_dword(bridge, pos + PCI_EXP_DEVCAP2,&cap);
>> +    pci_pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2,&cap);
>>       if (!(cap&  PCI_EXP_DEVCAP2_ARI))
>>           return;
>>
>> -    pci_read_config_word(bridge, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(bridge, PCI_EXP_DEVCTL2,&ctrl);
>>       ctrl |= PCI_EXP_DEVCTL2_ARI;
>> -    pci_write_config_word(bridge, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(bridge, PCI_EXP_DEVCTL2, ctrl);
>>
>>       bridge->ari_enabled = 1;
>>   }
>> @@ -2111,20 +2030,14 @@ void pci_enable_ari(struct pci_dev *dev)
>>    */
>>   void pci_enable_ido(struct pci_dev *dev, unsigned long type)
>>   {
>> -    int pos;
>>       u16 ctrl;
>>
>> -    /* ID-based Ordering is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return;
>> -
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       if (type&  PCI_EXP_IDO_REQUEST)
>>           ctrl |= PCI_EXP_IDO_REQ_EN;
>>       if (type&  PCI_EXP_IDO_COMPLETION)
>>           ctrl |= PCI_EXP_IDO_CMP_EN;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>   }
>>   EXPORT_SYMBOL(pci_enable_ido);
>>
>> @@ -2135,20 +2048,14 @@ EXPORT_SYMBOL(pci_enable_ido);
>>    */
>>   void pci_disable_ido(struct pci_dev *dev, unsigned long type)
>>   {
>> -    int pos;
>>       u16 ctrl;
>>
>> -    /* ID-based Ordering is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return;
>> -
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       if (type&  PCI_EXP_IDO_REQUEST)
>>           ctrl&= ~PCI_EXP_IDO_REQ_EN;
>>       if (type&  PCI_EXP_IDO_COMPLETION)
>>           ctrl&= ~PCI_EXP_IDO_CMP_EN;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>   }
>>   EXPORT_SYMBOL(pci_disable_ido);
>>
>> @@ -2173,17 +2080,11 @@ EXPORT_SYMBOL(pci_disable_ido);
>>    */
>>   int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>>   {
>> -    int pos;
>>       u32 cap;
>>       u16 ctrl;
>>       int ret;
>>
>> -    /* OBFF is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return -ENOTSUPP;
>> -
>> -    pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2,&cap);
>> +    pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2,&cap);
>>       if (!(cap&  PCI_EXP_OBFF_MASK))
>>           return -ENOTSUPP; /* no OBFF support at all */
>>
>> @@ -2194,7 +2095,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>>               return ret;
>>       }
>>
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       if (cap&  PCI_EXP_OBFF_WAKE)
>>           ctrl |= PCI_EXP_OBFF_WAKE_EN;
>>       else {
>> @@ -2212,7 +2113,7 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
>>               return -ENOTSUPP;
>>           }
>>       }
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>
>>       return 0;
>>   }
>> @@ -2226,17 +2127,11 @@ EXPORT_SYMBOL(pci_enable_obff);
>>    */
>>   void pci_disable_obff(struct pci_dev *dev)
>>   {
>> -    int pos;
>>       u16 ctrl;
>>
>> -    /* OBFF is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return;
>> -
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       ctrl&= ~PCI_EXP_OBFF_WAKE_EN;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>   }
>>   EXPORT_SYMBOL(pci_disable_obff);
>>
>> @@ -2249,15 +2144,9 @@ EXPORT_SYMBOL(pci_disable_obff);
>>    */
>>   static bool pci_ltr_supported(struct pci_dev *dev)
>>   {
>> -    int pos;
>>       u32 cap;
>>
>> -    /* LTR is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return false;
>> -
>> -    pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP2,&cap);
>> +    pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2,&cap);
>>
>>       return cap&  PCI_EXP_DEVCAP2_LTR;
>>   }
>> @@ -2274,22 +2163,16 @@ static bool pci_ltr_supported(struct pci_dev *dev)
>>    */
>>   int pci_enable_ltr(struct pci_dev *dev)
>>   {
>> -    int pos;
>>       u16 ctrl;
>>       int ret;
>>
>> -    if (!pci_ltr_supported(dev))
>> -        return -ENOTSUPP;
>> -
>> -    /* LTR is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return -ENOTSUPP;
>> -
>>       /* Only primary function can enable/disable LTR */
>>       if (PCI_FUNC(dev->devfn) != 0)
>>           return -EINVAL;
>>
>> +    if (!pci_ltr_supported(dev))
>> +        return -ENOTSUPP;
>> +
>>       /* Enable upstream ports first */
>>       if (dev->bus->self) {
>>           ret = pci_enable_ltr(dev->bus->self);
>> @@ -2297,9 +2180,9 @@ int pci_enable_ltr(struct pci_dev *dev)
>>               return ret;
>>       }
>>
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       ctrl |= PCI_EXP_LTR_EN;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>
>>       return 0;
>>   }
>> @@ -2311,24 +2194,18 @@ EXPORT_SYMBOL(pci_enable_ltr);
>>    */
>>   void pci_disable_ltr(struct pci_dev *dev)
>>   {
>> -    int pos;
>>       u16 ctrl;
>>
>> -    if (!pci_ltr_supported(dev))
>> -        return;
>> -
>> -    /* LTR is a PCIe cap v2 feature */
>> -    pos = pci_pcie_cap2(dev);
>> -    if (!pos)
>> -        return;
>> -
>>       /* Only primary function can enable/disable LTR */
>>       if (PCI_FUNC(dev->devfn) != 0)
>>           return;
>>
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL2,&ctrl);
>> +    if (!pci_ltr_supported(dev))
>> +        return;
>> +
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL2,&ctrl);
>>       ctrl&= ~PCI_EXP_LTR_EN;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL2, ctrl);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, ctrl);
>>   }
>>   EXPORT_SYMBOL(pci_disable_ltr);
>>
>> @@ -3178,15 +3055,10 @@ EXPORT_SYMBOL(pci_set_dma_seg_boundary);
>>   static int pcie_flr(struct pci_dev *dev, int probe)
>>   {
>>       int i;
>> -    int pos;
>>       u32 cap;
>>       u16 status, control;
>>
>> -    pos = pci_pcie_cap(dev);
>> -    if (!pos)
>> -        return -ENOTTY;
>> -
>> -    pci_read_config_dword(dev, pos + PCI_EXP_DEVCAP,&cap);
>> +    pci_pcie_capability_read_dword(dev, PCI_EXP_DEVCAP,&cap);
>>       if (!(cap&  PCI_EXP_DEVCAP_FLR))
>>           return -ENOTTY;
>>
>> @@ -3198,7 +3070,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>>           if (i)
>>               msleep((1<<  (i - 1)) * 100);
>>
>> -        pci_read_config_word(dev, pos + PCI_EXP_DEVSTA,&status);
>> +        pci_pcie_capability_read_word(dev, PCI_EXP_DEVSTA,&status);
>>           if (!(status&  PCI_EXP_DEVSTA_TRPND))
>>               goto clear;
>>       }
>> @@ -3207,9 +3079,9 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>>               "proceeding with reset anyway\n");
>>
>>   clear:
>> -    pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&control);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&control);
>>       control |= PCI_EXP_DEVCTL_BCR_FLR;
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, control);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, control);
>>
>>       msleep(100);
>>
>> @@ -3577,18 +3449,11 @@ EXPORT_SYMBOL(pcix_set_mmrbc);
>>    */
>>   int pcie_get_readrq(struct pci_dev *dev)
>>   {
>> -    int ret, cap;
>>       u16 ctl;
>>
>> -    cap = pci_pcie_cap(dev);
>> -    if (!cap)
>> -        return -EINVAL;
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>>
>> -    ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
>> -    if (!ret)
>> -        ret = 128<<  ((ctl&  PCI_EXP_DEVCTL_READRQ)>>  12);
>> -
>> -    return ret;
>> +    return 128<<  ((ctl&  PCI_EXP_DEVCTL_READRQ)>>  12);
>>   }
>>   EXPORT_SYMBOL(pcie_get_readrq);
>>
>> @@ -3602,17 +3467,16 @@ EXPORT_SYMBOL(pcie_get_readrq);
>>    */
>>   int pcie_set_readrq(struct pci_dev *dev, int rq)
>>   {
>> -    int cap, err = -EINVAL;
>> +    int err = -EINVAL;
>>       u16 ctl, v;
>>
>>       if (rq<  128 || rq>  4096 || !is_power_of_2(rq))
>>           goto out;
>>
>> -    cap = pci_pcie_cap(dev);
>> -    if (!cap)
>> +    if (!pci_is_pcie(dev))
> ditto ^^^
> 
>>           goto out;
>>
>> -    err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
>> +    err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>>       if (err)
>>           goto out;
>>       /*
>> @@ -3635,7 +3499,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>>       if ((ctl&  PCI_EXP_DEVCTL_READRQ) != v) {
>>           ctl&= ~PCI_EXP_DEVCTL_READRQ;
>>           ctl |= v;
>> -        err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
>> +        err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
>>       }
>>
>>   out:
>> @@ -3652,18 +3516,11 @@ EXPORT_SYMBOL(pcie_set_readrq);
>>    */
>>   int pcie_get_mps(struct pci_dev *dev)
>>   {
>> -    int ret, cap;
>>       u16 ctl;
>>
>> -    cap = pci_pcie_cap(dev);
>> -    if (!cap)
>> -        return -EINVAL;
>> -
>> -    ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
>> -    if (!ret)
>> -        ret = 128<<  ((ctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5);
>> +    pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>>
>> -    return ret;
>> +    return 128<<  ((ctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5);
>>   }
>>
>>   /**
>> @@ -3676,7 +3533,7 @@ int pcie_get_mps(struct pci_dev *dev)
>>    */
>>   int pcie_set_mps(struct pci_dev *dev, int mps)
>>   {
>> -    int cap, err = -EINVAL;
>> +    int err = -EINVAL;
>>       u16 ctl, v;
>>
>>       if (mps<  128 || mps>  4096 || !is_power_of_2(mps))
>> @@ -3687,18 +3544,17 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
>>           goto out;
>>       v<<= 5;
>>
>> -    cap = pci_pcie_cap(dev);
>> -    if (!cap)
>> +    if (!pci_is_pcie(dev))
> ditto ^^^^^^^^^^
>>           goto out;
>>
>> -    err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL,&ctl);
>> +    err = pci_pcie_capability_read_word(dev, PCI_EXP_DEVCTL,&ctl);
>>       if (err)
>>           goto out;
>>
>>       if ((ctl&  PCI_EXP_DEVCTL_PAYLOAD) != v) {
>>           ctl&= ~PCI_EXP_DEVCTL_PAYLOAD;
>>           ctl |= v;
>> -        err = pci_write_config_word(dev, cap + PCI_EXP_DEVCTL, ctl);
>> +        err = pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL, ctl);
>>       }
>>   out:
>>       return err;
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 8bcc985..6a2eac8 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -603,10 +603,11 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>           u32 linkcap;
>>           u16 linksta;
>>
>> -        pci_read_config_dword(bridge, pos + PCI_EXP_LNKCAP,&linkcap);
>> +        pci_pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP,
>> +                    &linkcap);
>>           bus->max_bus_speed = pcie_link_speed[linkcap&  0xf];
>>
>> -        pci_read_config_word(bridge, pos + PCI_EXP_LNKSTA,&linksta);
>> +        pci_pcie_capability_read_word(bridge, PCI_EXP_LNKSTA,&linksta);
>>           pcie_update_link_speed(bus, linksta);
>>       }
>>   }
>> @@ -936,17 +937,9 @@ void set_pcie_port_type(struct pci_dev *pdev)
>>
>>   void set_pcie_hotplug_bridge(struct pci_dev *pdev)
>>   {
>> -    int pos;
>> -    u16 reg16;
>>       u32 reg32;
>>
>> -    pos = pci_pcie_cap(pdev);
>> -    if (!pos)
>> -        return;
>> -    pci_read_config_word(pdev, pos + PCI_EXP_FLAGS,&reg16);
>> -    if (!(reg16&  PCI_EXP_FLAGS_SLOT))
>> -        return;
>> -    pci_read_config_dword(pdev, pos + PCI_EXP_SLTCAP,&reg32);
>> +    pci_pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP,&reg32);
>>       if (reg32&  PCI_EXP_SLTCAP_HPC)
>>           pdev->is_hotplug_bridge = 1;
>>   }
>> @@ -1160,8 +1153,7 @@ int pci_cfg_space_size(struct pci_dev *dev)
>>       if (class == PCI_CLASS_BRIDGE_HOST)
>>           return pci_cfg_space_size_ext(dev);
>>
>> -    pos = pci_pcie_cap(dev);
>> -    if (!pos) {
>> +    if (!pci_is_pcie(dev)) {
>>           pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>>           if (!pos)
>>               goto fail;
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 003f356..484284e 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -3081,17 +3081,14 @@ static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
>>
>>   static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
>>   {
>> -    int pos;
>> -
>> -    pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
>> -    if (!pos)
>> +    if (!pci_is_pcie(dev))
> ditto if check in capability_write as well
> 
>>           return -ENOTTY;
>>
>>       if (probe)
>>           return 0;
>>
>> -    pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
>> -                PCI_EXP_DEVCTL_BCR_FLR);
>> +    pci_pcie_capability_write_word(dev, PCI_EXP_DEVCTL,
>> +                       PCI_EXP_DEVCTL_BCR_FLR);
>>       msleep(100);
>>
>>       return 0;
> 


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers
  2012-07-24 21:09 ` [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Don Dutile
@ 2012-07-29  2:26   ` Jiang Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-29  2:26 UTC (permalink / raw)
  To: Don Dutile
  Cc: Bjorn Helgaas, Yinghai Lu, Taku Izumi, Rafael J . Wysocki,
	Kenji Kaneshige, Yijing Wang, linux-kernel, linux-pci

On 07/25/2012 05:09 AM, Don Dutile wrote:
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> From: Jiang Liu<liuj97@gmail.com>
>>
>> As suggested by Bjorn Helgaas and Don Dutile in threads
>> http://www.spinics.net/lists/linux-pci/msg15663.html, we could improve access
>> to PCIe capabilities register in to way:
>> 1) cache content of PCIe Capabilities Register into struct pce_dev to avoid
>>     repeatedly reading this register because it's read only.
>> 2) provide access functions for PCIe Capabilities registers to hide differences
>>     among PCIe base specifications, so the caller don't need to handle those
>>     differences.
>>
>> This patch set applies to
>> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci-next
>>
>> And you could pull the change set from
>> https://github.com/jiangliu/linux.git topic/pcie-cap
>>
>> These patch set is still RFC. It provides the new interfaces and has made the
>> major changes to adopt those new interfaces. But there are still several device
>> drivers left untouched. Any comments about the new interfaces are welcomed,
>> especially about function names:). Once we reach an agreement, I will send out
>> a formal version with all needed work done.
>>
>> v2: 1) change return value to 0 when the register is not implemented by
>>         V1 PCIe devices.
>>      2) Change all driver in the source tree to use the new interfaces.
>>
>> Jiang Liu (29):
>>    PCI: add PCIe capabilities access functions to hide differences among
>>      PCIe specs
>>    PCI/core: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/hotplug: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/portdrv: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/pciehp: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/PME: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/AER: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/ASPM: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/ARM: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/MIPS: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/tile: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/r8169: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/broadcom: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/igb: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/vxge: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/mlx4: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/niu: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/myri10ge: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/chelsio: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/atl1c: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/ath9k: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/iwl: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/mthca: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/qib: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/qla: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/radeon: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/tsi721: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/et131x: use PCIe capabilities access functions to simplify
>>      implementation
>>    PCI/rtl8192e: use PCIe capabilities access functions to simplify
>>      implementation
>>
>> Yijing Wang (3):
>>    PCI: add pcie_flags_reg into struct pci_dev to cache PCIe
>>      capabilities register
>>    PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type
>>    PCI: remove unused field pcie_type from struct pci_dev
>>
>>   arch/arm/mach-tegra/pcie.c                         |    7 +-
>>   arch/mips/pci/pci-octeon.c                         |    7 +-
>>   arch/powerpc/platforms/powernv/pci-ioda.c          |    2 +-
>>   arch/tile/kernel/pci.c                             |   17 +-
>>   drivers/gpu/drm/radeon/evergreen.c                 |    9 +-
>>   drivers/infiniband/hw/mthca/mthca_reset.c          |    8 +-
>>   drivers/infiniband/hw/qib/qib_pcie.c               |   40 ++-
>>   drivers/iommu/intel-iommu.c                        |    6 +-
>>   drivers/net/ethernet/atheros/atl1c/atl1c_main.c    |    2 +-
>>   drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   |   24 +-
>>   drivers/net/ethernet/broadcom/tg3.c                |   46 ++--
>>   drivers/net/ethernet/chelsio/cxgb3/t3_hw.c         |   19 +-
>>   drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    |   10 +-
>>   drivers/net/ethernet/chelsio/cxgb4/t4_hw.c         |    7 +-
>>   drivers/net/ethernet/intel/e1000e/netdev.c         |   20 +-
>>   drivers/net/ethernet/intel/igb/igb_main.c          |   12 +-
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |    2 +-
>>   drivers/net/ethernet/mellanox/mlx4/reset.c         |    8 +-
>>   drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |   21 +-
>>   drivers/net/ethernet/neterion/vxge/vxge-config.c   |    4 +-
>>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |    2 +-
>>   drivers/net/ethernet/realtek/r8169.c               |   35 +--
>>   drivers/net/ethernet/sun/niu.c                     |    9 +-
>>   drivers/net/wireless/ath/ath9k/pci.c               |   18 +-
>>   drivers/net/wireless/iwlegacy/common.h             |    5 +-
>>   drivers/net/wireless/iwlwifi/iwl-trans-pcie.c      |    4 +-
>>   drivers/net/wireless/rtlwifi/pci.c                 |    8 +-
>>   drivers/pci/access.c                               |  157 ++++++++++++
>>   drivers/pci/hotplug/pciehp_acpi.c                  |    6 +-
>>   drivers/pci/hotplug/pciehp_hpc.c                   |   10 +-
>>   drivers/pci/hotplug/pcihp_slot.c                   |   12 +-
>>   drivers/pci/iov.c                                  |    6 +-
>>   drivers/pci/pci.c                                  |  262 +++++---------------
>>   drivers/pci/pcie/aer/aer_inject.c                  |    2 +-
>>   drivers/pci/pcie/aer/aerdrv.c                      |   23 +-
>>   drivers/pci/pcie/aer/aerdrv_acpi.c                 |    2 +-
>>   drivers/pci/pcie/aer/aerdrv_core.c                 |   27 +-
>>   drivers/pci/pcie/aspm.c                            |  108 ++++----
>>   drivers/pci/pcie/pme.c                             |   23 +-
>>   drivers/pci/pcie/portdrv_bus.c                     |    2 +-
>>   drivers/pci/pcie/portdrv_core.c                    |   24 +-
>>   drivers/pci/pcie/portdrv_pci.c                     |   15 +-
>>   drivers/pci/probe.c                                |   29 +--
>>   drivers/pci/quirks.c                               |    9 +-
>>   drivers/pci/search.c                               |    2 +-
>>   drivers/rapidio/devices/tsi721.c                   |   13 +-
>>   drivers/scsi/qla2xxx/qla_init.c                    |    4 +-
>>   drivers/scsi/qla2xxx/qla_nx.c                      |    8 +-
>>   drivers/scsi/qla4xxx/ql4_nx.c                      |    4 +-
>>   drivers/staging/et131x/et131x.c                    |    9 +-
>>   drivers/staging/rtl8192e/rtl8192e/rtl_pci.c        |    8 +-
>>   include/linux/pci.h                                |   17 +-
>>   include/linux/pci_regs.h                           |    2 +
>>   53 files changed, 510 insertions(+), 626 deletions(-)
>>
> 
> Summary:
> I commented on 01, 04 & 05/32;
> Overall, excellent cleanup, esp. in all the various drivers,
> since driver replication is common, and that'll help correct future
> drivers (wishful thinking, I know...).
> 
> Note that the comment of eliminating pci_is_pcie() and incorporating
> it into the pci_pcie_capability_[read,write]_[byte,word,dword]() functions
> could be applied to other patches as well (06, 10 ... and that I stopped looking
> due to time constraints & repetitiveness... since this is RFC, not final patch post).
Hi Dan,
	Thanks for your comments, and will address them in V3.
	Regards!
	Gerry


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs
  2012-07-24 21:12   ` Don Dutile
@ 2012-07-29 16:22     ` Jiang Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Jiang Liu @ 2012-07-29 16:22 UTC (permalink / raw)
  To: Don Dutile
  Cc: Bjorn Helgaas, Jiang Liu, Yinghai Lu, Taku Izumi,
	Rafael J . Wysocki, Kenji Kaneshige, Yijing Wang, linux-kernel,
	linux-pci

On 07/25/2012 05:12 AM, Don Dutile wrote:
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> +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 ?
Hi Don,
	Yes, that's the purpose. PCIe spec v2/v3 defines that hardware should return
a value with bit PCI_EXP_SLTSTA_PDS set if PCI_EXP_SLTSTA register is not implemented.
So for PCIe v1 hardwares, we try to behave in the same way as v2/v3.
	Regards!
	Gerry

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2012-07-29 16:22 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-24 16:31 [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 01/32] PCI: add pcie_flags_reg into struct pci_dev to cache PCIe capabilities register Jiang Liu
2012-07-25 15:12   ` Don Dutile
2012-07-26 13:47     ` Yijing Wang
2012-07-24 16:31 ` [RFC PATCH v2 02/32] PCI: introduce pci_pcie_type(dev) to replace pci_dev->pcie_type Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 03/32] PCI: remove unused field pcie_type from struct pci_dev Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 04/32] PCI: add PCIe capabilities access functions to hide differences among PCIe specs Jiang Liu
2012-07-24 21:12   ` Don Dutile
2012-07-29 16:22     ` Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 05/32] PCI/core: use PCIe capabilities access functions to simplify implementation Jiang Liu
2012-07-25 21:12   ` Don Dutile
2012-07-29  2:12     ` Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 06/32] PCI/hotplug: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 07/32] PCI/portdrv: " Jiang Liu
2012-07-25  5:51   ` Kaneshige, Kenji
2012-07-25  9:44     ` Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 08/32] PCI/pciehp: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 09/32] PCI/PME: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 10/32] PCI/AER: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 11/32] PCI/ASPM: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 12/32] PCI/ARM: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 13/32] PCI/MIPS: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 14/32] PCI/tile: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 15/32] PCI/r8169: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 16/32] PCI/broadcom: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 17/32] PCI/igb: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 18/32] PCI/vxge: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 19/32] PCI/mlx4: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 20/32] PCI/niu: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 21/32] PCI/myri10ge: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 22/32] PCI/chelsio: " Jiang Liu
2012-07-24 16:31 ` [RFC PATCH v2 23/32] PCI/atl1c: " Jiang Liu
2012-07-24 21:09 ` [RFC PATCH v2 00/32] provide interfaces to access PCIe capabilities registers Don Dutile
2012-07-29  2:26   ` Jiang Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).