[PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Yijing Wang]

Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Jiang Liu <jiang.liu@huawei.com>
Cc: Anil Gurumurthy <agurumur@brocade.com>
Cc: Vijaya Mohan Guvva <vmohan@brocade.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/bfa/bfad.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 9611195..d726b81 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -767,7 +767,6 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 
 	/* Adjust PCIe Maximum Read Request Size */
 	if (pcie_max_read_reqsz > 0) {
-		int pcie_cap_reg;
 		u16 pcie_dev_ctl;
 		u16 mask = 0xffff;
 
@@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 			break;
 		}
 
-		pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
-		if (mask != 0xffff && pcie_cap_reg) {
-			pcie_cap_reg += 0x08;
-			pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
+		if (mask != 0xffff && pci_is_pcie(pdev)) {
+			pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
 			if ((pcie_dev_ctl & 0x7000) != mask) {
 				printk(KERN_WARNING "BFA[%s]: "
 				"pcie_max_read_request_size is %d, "
@@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 				pcie_max_read_reqsz);
 
 				pcie_dev_ctl &= ~0x7000;
-				pci_write_config_word(pdev, pcie_cap_reg,
+				pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
 						pcie_dev_ctl | mask);
 			}
 		}
-- 
1.7.1

[PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Yijing Wang]

Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Jiang Liu <jiang.liu@huawei.com>
Cc: Anil Gurumurthy <agurumur@brocade.com>
Cc: Vijaya Mohan Guvva <vmohan@brocade.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/bfa/bfad.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 9611195..d726b81 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -767,7 +767,6 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 
 	/* Adjust PCIe Maximum Read Request Size */
 	if (pcie_max_read_reqsz > 0) {
-		int pcie_cap_reg;
 		u16 pcie_dev_ctl;
 		u16 mask = 0xffff;
 
@@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 			break;
 		}
 
-		pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
-		if (mask != 0xffff && pcie_cap_reg) {
-			pcie_cap_reg += 0x08;
-			pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
+		if (mask != 0xffff && pci_is_pcie(pdev)) {
+			pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
 			if ((pcie_dev_ctl & 0x7000) != mask) {
 				printk(KERN_WARNING "BFA[%s]: "
 				"pcie_max_read_request_size is %d, "
@@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 				pcie_max_read_reqsz);
 
 				pcie_dev_ctl &= ~0x7000;
-				pci_write_config_word(pdev, pcie_cap_reg,
+				pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
 						pcie_dev_ctl | mask);
 			}
 		}
-- 
1.7.1

[PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code

[Yijing Wang]

Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Jiang Liu <jiang.liu@huawei.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Naresh Kumar Inna <naresh@chelsio.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jesper Juhl <jj@chaosbits.net>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/csiostor/csio_hw.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 0eb35b9..be9a6ef 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -856,15 +856,12 @@ static void
 csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
 {
 	uint16_t val;
-	int pcie_cap;
 
-	if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
-		pci_read_config_word(hw->pdev,
-				     pcie_cap + PCI_EXP_DEVCTL2, &val);
+	if (pci_is_pcie(hw->pdev)) {
+		pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
 		val &= 0xfff0;
 		val |= range ;
-		pci_write_config_word(hw->pdev,
-				      pcie_cap + PCI_EXP_DEVCTL2, val);
+		pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
 	}
 }
 
-- 
1.7.1

[PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code

[Yijing Wang]

Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Jiang Liu <jiang.liu@huawei.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Naresh Kumar Inna <naresh@chelsio.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jesper Juhl <jj@chaosbits.net>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/csiostor/csio_hw.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 0eb35b9..be9a6ef 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -856,15 +856,12 @@ static void
 csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
 {
 	uint16_t val;
-	int pcie_cap;
 
-	if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
-		pci_read_config_word(hw->pdev,
-				     pcie_cap + PCI_EXP_DEVCTL2, &val);
+	if (pci_is_pcie(hw->pdev)) {
+		pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
 		val &= 0xfff0;
 		val |= range ;
-		pci_write_config_word(hw->pdev,
-				      pcie_cap + PCI_EXP_DEVCTL2, val);
+		pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
 	}
 }
 
-- 
1.7.1

[PATCH 3/7] powerpc/pci: use pci_is_pcie() to simplify code

[Yijing Wang]

Use pci_is_pcie() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
 arch/powerpc/kernel/eeh.c     |    3 +--
 arch/powerpc/sysdev/fsl_pci.c |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 39954fe..b0bd41a 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -189,8 +189,7 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char * buf, size_t len)
 	}
 
 	/* If PCI-E capable, dump PCI-E cap 10, and the AER */
-	cap = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (cap) {
+	if (pci_is_pcie(dev)) {
 		n += scnprintf(buf+n, len-n, "pci-e cap10:\n");
 		printk(KERN_WARNING
 		       "EEH: PCI-E capabilities and status follow:\n");
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 46ac1dd..5402a1d 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -41,7 +41,7 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
 	u8 hdr_type;
 
 	/* if we aren't a PCIe don't bother */
-	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
+	if (!pci_is_pcie(dev))
 		return;
 
 	/* if we aren't in host mode don't bother */
-- 
1.7.1

[PATCH 3/7] powerpc/pci: use pci_is_pcie() to simplify code

[Yijing Wang]

Use pci_is_pcie() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
 arch/powerpc/kernel/eeh.c     |    3 +--
 arch/powerpc/sysdev/fsl_pci.c |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 39954fe..b0bd41a 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -189,8 +189,7 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char * buf, size_t len)
 	}
 
 	/* If PCI-E capable, dump PCI-E cap 10, and the AER */
-	cap = pci_find_capability(dev, PCI_CAP_ID_EXP);
-	if (cap) {
+	if (pci_is_pcie(dev)) {
 		n += scnprintf(buf+n, len-n, "pci-e cap10:\n");
 		printk(KERN_WARNING
 		       "EEH: PCI-E capabilities and status follow:\n");
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 46ac1dd..5402a1d 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -41,7 +41,7 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
 	u8 hdr_type;
 
 	/* if we aren't a PCIe don't bother */
-	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
+	if (!pci_is_pcie(dev))
 		return;
 
 	/* if we aren't in host mode don't bother */
-- 
1.7.1

[PATCH 4/7] x86/pci: use pcie_cap to simplify code

[Yijing Wang]

PCI core saves PCIe Cap offset in pcie_cap,
use pcie_cap to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 arch/x86/pci/fixup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index f5809fa..ee8330d 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -288,7 +288,7 @@ static void pcie_rootport_aspm_quirk(struct pci_dev *pdev)
 		 */
 		list_for_each_entry(dev, &pbus->devices, bus_list) {
 			/* There are 0 to 8 devices attached to this bus */
-			cap_base = pci_find_capability(dev, PCI_CAP_ID_EXP);
+			cap_base = dev->pcie_cap;
 			quirk_aspm_offset[GET_INDEX(pdev->device, dev->devfn)] = cap_base + 0x10;
 		}
 		pbus->ops = &quirk_pcie_aspm_ops;
-- 
1.7.1

[PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Yijing Wang]

use pcie_capability_read_word() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bad8f14..bfa0b06 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
 static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 					  u32 reg, u16 *value)
 {
-	int pos = 0;
 	struct pci_dev *parent_dev;
 	struct pci_bus *parent_bus;
 
@@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 	if (!parent_dev)
 		return -1;
 
-	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
-	if (!pos)
+	if (!pci_is_pcie(parent_dev))
 		return -1;
 
-	pci_read_config_word(parent_dev, pos + reg, value);
+	pcie_capability_read_word(parent_dev, reg, value);
 	return 0;
 }
 
-- 
1.7.1

[PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Yijing Wang]

use pcie_capability_read_word() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bad8f14..bfa0b06 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
 static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 					  u32 reg, u16 *value)
 {
-	int pos = 0;
 	struct pci_dev *parent_dev;
 	struct pci_bus *parent_bus;
 
@@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 	if (!parent_dev)
 		return -1;
 
-	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
-	if (!pos)
+	if (!pci_is_pcie(parent_dev))
 		return -1;
 
-	pci_read_config_word(parent_dev, pos + reg, value);
+	pcie_capability_read_word(parent_dev, reg, value);
 	return 0;
 }
 
-- 
1.7.1



------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[PATCH 6/7] PCI: use pci_is_pcie() to simplify code

[Yijing Wang]

Use pci_is_pcie() instead of pci_find_capability
to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/probe.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index eeb50bd..0fa9075 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -641,8 +641,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 		return;
 	}
 
-	pos = pci_find_capability(bridge, PCI_CAP_ID_EXP);
-	if (pos) {
+	if (pci_is_pcie(bridge)) {
 		u32 linkcap;
 		u16 linksta;
 
-- 
1.7.1

[PATCH 7/7] scsi/qla2xxx: use pcie_is_pcie() to simplify code

[Yijing Wang]

Use pci_is_pcie() instead of pci_find_capability
to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Andrew Vasquez <andrew.vasquez@qlogic.com>
Cc: linux-driver@qlogic.com
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/qla2xxx/qla_mr.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index d799379..5f74271 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -507,7 +507,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
 	pci_write_config_word(ha->pdev, PCI_COMMAND, w);
 
 	/* PCIe -- adjust Maximum Read Request Size (2048). */
-	if (pci_find_capability(ha->pdev, PCI_CAP_ID_EXP))
+	if (pci_is_pcie(ha->pdev))
 		pcie_set_readrq(ha->pdev, 2048);
 
 	ha->chip_revision = ha->pdev->revision;
@@ -660,10 +660,8 @@ char *
 qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
 {
 	struct qla_hw_data *ha = vha->hw;
-	int pcie_reg;
 
-	pcie_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
-	if (pcie_reg) {
+	if (pci_is_pcie(ha->pdev)) {
 		strcpy(str, "PCIe iSA");
 		return str;
 	}
-- 
1.7.1

[PATCH 7/7] scsi/qla2xxx: use pcie_is_pcie() to simplify code

[Yijing Wang]

Use pci_is_pcie() instead of pci_find_capability
to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: Andrew Vasquez <andrew.vasquez@qlogic.com>
Cc: linux-driver@qlogic.com
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/scsi/qla2xxx/qla_mr.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index d799379..5f74271 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -507,7 +507,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
 	pci_write_config_word(ha->pdev, PCI_COMMAND, w);
 
 	/* PCIe -- adjust Maximum Read Request Size (2048). */
-	if (pci_find_capability(ha->pdev, PCI_CAP_ID_EXP))
+	if (pci_is_pcie(ha->pdev))
 		pcie_set_readrq(ha->pdev, 2048);
 
 	ha->chip_revision = ha->pdev->revision;
@@ -660,10 +660,8 @@ char *
 qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
 {
 	struct qla_hw_data *ha = vha->hw;
-	int pcie_reg;
 
-	pcie_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
-	if (pcie_reg) {
+	if (pci_is_pcie(ha->pdev)) {
 		strcpy(str, "PCIe iSA");
 		return str;
 	}
-- 
1.7.1

Re: [PATCH 7/7] scsi/qla2xxx: use pcie_is_pcie() to simplify code

[Chad Dupuis]

Looks good.

Acked-by: Chad Dupuis <chad.dupuis@qlogic.com>

On Tue, 3 Sep 2013, Yijing Wang wrote:

> Use pci_is_pcie() instead of pci_find_capability
> to simplify code.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Andrew Vasquez <andrew.vasquez@qlogic.com>
> Cc: linux-driver@qlogic.com
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/scsi/qla2xxx/qla_mr.c |    6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
> index d799379..5f74271 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.c
> +++ b/drivers/scsi/qla2xxx/qla_mr.c
> @@ -507,7 +507,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
> 	pci_write_config_word(ha->pdev, PCI_COMMAND, w);
>
> 	/* PCIe -- adjust Maximum Read Request Size (2048). */
> -	if (pci_find_capability(ha->pdev, PCI_CAP_ID_EXP))
> +	if (pci_is_pcie(ha->pdev))
> 		pcie_set_readrq(ha->pdev, 2048);
>
> 	ha->chip_revision = ha->pdev->revision;
> @@ -660,10 +660,8 @@ char *
> qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
> {
> 	struct qla_hw_data *ha = vha->hw;
> -	int pcie_reg;
>
> -	pcie_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
> -	if (pcie_reg) {
> +	if (pci_is_pcie(ha->pdev)) {
> 		strcpy(str, "PCIe iSA");
> 		return str;
> 	}
>

Re: [PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Bjorn Helgaas]

On Tue, Sep 03, 2013 at 03:35:09PM +0800, Yijing Wang wrote:
> Pcie_capability_xxx() interfaces were introudced to
> simplify code to access PCIe Cap config space. And
> because PCI core saves the PCIe Cap offset in
> set_pcie_port_type() when device is enumerated.
> So we can use pci_is_pcie() instead.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Jiang Liu <jiang.liu@huawei.com>
> Cc: Anil Gurumurthy <agurumur@brocade.com>
> Cc: Vijaya Mohan Guvva <vmohan@brocade.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/scsi/bfa/bfad.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
> index 9611195..d726b81 100644
> --- a/drivers/scsi/bfa/bfad.c
> +++ b/drivers/scsi/bfa/bfad.c
> @@ -767,7 +767,6 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>  
>  	/* Adjust PCIe Maximum Read Request Size */
>  	if (pcie_max_read_reqsz > 0) {
> -		int pcie_cap_reg;
>  		u16 pcie_dev_ctl;
>  		u16 mask = 0xffff;
>  
> @@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>  			break;
>  		}
>  
> -		pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
> -		if (mask != 0xffff && pcie_cap_reg) {
> -			pcie_cap_reg += 0x08;
> -			pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
> +		if (mask != 0xffff && pci_is_pcie(pdev)) {

Please move the pci_is_pcie() test up to the
"if (pcie_mas_read_reqsz ..." statement.  There's no point in doing
the switch statement if this isn't a PCIe device.

> +			pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
>  			if ((pcie_dev_ctl & 0x7000) != mask) {
>  				printk(KERN_WARNING "BFA[%s]: "
>  				"pcie_max_read_request_size is %d, "
> @@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>  				pcie_max_read_reqsz);
>  
>  				pcie_dev_ctl &= ~0x7000;
> -				pci_write_config_word(pdev, pcie_cap_reg,
> +				pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
>  						pcie_dev_ctl | mask);

Please rework this to use pcie_set_readrq() instead of writing
the capability directly.  If we write the capability directly, we
risk writing a value that is incompatible with the MPS
configuration done by the PCI core.

>  			}
>  		}
> -- 
> 1.7.1
> 
> 

Re: [PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code

[Bjorn Helgaas]

On Tue, Sep 03, 2013 at 03:35:10PM +0800, Yijing Wang wrote:
> Pcie_capability_xxx() interfaces were introudced to

s/introudced/introduced/

> simplify code to access PCIe Cap config space. And
> because PCI core saves the PCIe Cap offset in
> set_pcie_port_type() when device is enumerated.
> So we can use pci_is_pcie() instead.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Jiang Liu <jiang.liu@huawei.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: Naresh Kumar Inna <naresh@chelsio.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jesper Juhl <jj@chaosbits.net>
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/scsi/csiostor/csio_hw.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
> index 0eb35b9..be9a6ef 100644
> --- a/drivers/scsi/csiostor/csio_hw.c
> +++ b/drivers/scsi/csiostor/csio_hw.c
> @@ -856,15 +856,12 @@ static void
>  csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
>  {
>  	uint16_t val;
> -	int pcie_cap;
>  
> -	if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
> -		pci_read_config_word(hw->pdev,
> -				     pcie_cap + PCI_EXP_DEVCTL2, &val);
> +	if (pci_is_pcie(hw->pdev)) {
> +		pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
>  		val &= 0xfff0;
>  		val |= range ;
> -		pci_write_config_word(hw->pdev,
> -				      pcie_cap + PCI_EXP_DEVCTL2, val);
> +		pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);

Please add a #define for the Completion Timeout Value field and use
pcie_capability_clear_and_set_word() instead of writing it out the
long way here.

>  	}
>  }
>  
> -- 
> 1.7.1
> 
> 

Re: [PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Bjorn Helgaas]

On Tue, Sep 3, 2013 at 5:34 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Sep 03, 2013 at 03:35:09PM +0800, Yijing Wang wrote:
>> Pcie_capability_xxx() interfaces were introudced to
>> simplify code to access PCIe Cap config space. And
>> because PCI core saves the PCIe Cap offset in
>> set_pcie_port_type() when device is enumerated.
>> So we can use pci_is_pcie() instead.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> Cc: Jiang Liu <jiang.liu@huawei.com>
>> Cc: Anil Gurumurthy <agurumur@brocade.com>
>> Cc: Vijaya Mohan Guvva <vmohan@brocade.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  drivers/scsi/bfa/bfad.c |    9 +++------
>>  1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
>> index 9611195..d726b81 100644
>> --- a/drivers/scsi/bfa/bfad.c
>> +++ b/drivers/scsi/bfa/bfad.c
>> @@ -767,7 +767,6 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>
>>       /* Adjust PCIe Maximum Read Request Size */
>>       if (pcie_max_read_reqsz > 0) {
>> -             int pcie_cap_reg;
>>               u16 pcie_dev_ctl;
>>               u16 mask = 0xffff;
>>
>> @@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>                       break;
>>               }
>>
>> -             pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> -             if (mask != 0xffff && pcie_cap_reg) {
>> -                     pcie_cap_reg += 0x08;
>> -                     pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
>> +             if (mask != 0xffff && pci_is_pcie(pdev)) {
>
> Please move the pci_is_pcie() test up to the
> "if (pcie_mas_read_reqsz ..." statement.  There's no point in doing
> the switch statement if this isn't a PCIe device.
>
>> +                     pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
>>                       if ((pcie_dev_ctl & 0x7000) != mask) {

I forgot to mention that this 0x7000 constant should be
PCI_EXP_DEVCTL_READRQ instead.

>>                               printk(KERN_WARNING "BFA[%s]: "
>>                               "pcie_max_read_request_size is %d, "
>> @@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>                               pcie_max_read_reqsz);
>>
>>                               pcie_dev_ctl &= ~0x7000;
>> -                             pci_write_config_word(pdev, pcie_cap_reg,
>> +                             pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
>>                                               pcie_dev_ctl | mask);
>
> Please rework this to use pcie_set_readrq() instead of writing
> the capability directly.  If we write the capability directly, we
> risk writing a value that is incompatible with the MPS
> configuration done by the PCI core.
>
>>                       }
>>               }
>> --
>> 1.7.1
>>
>>

Re: [PATCH 4/7] x86/pci: use pcie_cap to simplify code

[Bjorn Helgaas]

On Tue, Sep 03, 2013 at 03:35:12PM +0800, Yijing Wang wrote:
> PCI core saves PCIe Cap offset in pcie_cap,
> use pcie_cap to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  arch/x86/pci/fixup.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index f5809fa..ee8330d 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -288,7 +288,7 @@ static void pcie_rootport_aspm_quirk(struct pci_dev *pdev)
>  		 */
>  		list_for_each_entry(dev, &pbus->devices, bus_list) {
>  			/* There are 0 to 8 devices attached to this bus */
> -			cap_base = pci_find_capability(dev, PCI_CAP_ID_EXP);
> +			cap_base = dev->pcie_cap;
>  			quirk_aspm_offset[GET_INDEX(pdev->device, dev->devfn)] = cap_base + 0x10;

This should use PCI_EXP_LNKCTL instead of "0x10".

>  		}
>  		pbus->ops = &quirk_pcie_aspm_ops;

This quirk replaces the config accessors with ones that silently ignore
writes to ASPM control bits.  That really warrants at least a dev_info()
note here, and we should be using pci_bus_set_ops().

Even that is a little bit dubious because I don't think this is really
safe -- what happens if this quirk replaces the ops, then somebody
else replaces the ops again?  aer_inject.c at least keeps track of
the old ops and seems to fall back to them, but it seems fragile to
depend on every caller of pci_bus_set_ops() to do the right thing
there.

But this is beyond the scope of your patch, so if you just
add a dev_info() note and use pci_bus_set_ops(), that should be
enough for now.

Re: [PATCH 3/7] powerpc/pci: use pci_is_pcie() to simplify code

[Gavin Shan]

>Use pci_is_pcie() to simplify code.
>
>Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
>Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>Cc: Paul Mackerras <paulus@samba.org>
>Cc: linuxppc-dev@lists.ozlabs.org
>Cc: linux-kernel@vger.kernel.org

It looks good to me:

Reviewed-by: Gavin Shan <shangw@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/kernel/eeh.c     |    3 +--
> arch/powerpc/sysdev/fsl_pci.c |    2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>index 39954fe..b0bd41a 100644
>--- a/arch/powerpc/kernel/eeh.c
>+++ b/arch/powerpc/kernel/eeh.c
>@@ -189,8 +189,7 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char * buf, size_t len)
> 	}
>
> 	/* If PCI-E capable, dump PCI-E cap 10, and the AER */
>-	cap = pci_find_capability(dev, PCI_CAP_ID_EXP);
>-	if (cap) {
>+	if (pci_is_pcie(dev)) {
> 		n += scnprintf(buf+n, len-n, "pci-e cap10:\n");
> 		printk(KERN_WARNING
> 		       "EEH: PCI-E capabilities and status follow:\n");
>diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>index 46ac1dd..5402a1d 100644
>--- a/arch/powerpc/sysdev/fsl_pci.c
>+++ b/arch/powerpc/sysdev/fsl_pci.c
>@@ -41,7 +41,7 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
> 	u8 hdr_type;
>
> 	/* if we aren't a PCIe don't bother */
>-	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>+	if (!pci_is_pcie(dev))
> 		return;
>
> 	/* if we aren't in host mode don't bother */
>-- 
>1.7.1
>
>

Re: [E1000-devel] [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

On Tue, 2013-09-03 at 15:35 +0800, Yijing Wang wrote:
> use pcie_capability_read_word() to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)

Thanks Yijing, I will add it to my queue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 442 bytes --]

On Tue, 2013-09-03 at 15:35 +0800, Yijing Wang wrote:
> use pcie_capability_read_word() to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)

Thanks Yijing, I will add it to my queue.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 433 bytes --]

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Bjorn Helgaas]

[+cc Jacob, Jeff]

On Tue, Sep 03, 2013 at 03:35:13PM +0800, Yijing Wang wrote:
> use pcie_capability_read_word() to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index bad8f14..bfa0b06 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
>  static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
>  					  u32 reg, u16 *value)
>  {
> -	int pos = 0;
>  	struct pci_dev *parent_dev;
>  	struct pci_bus *parent_bus;
>  
> @@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
>  	if (!parent_dev)
>  		return -1;
>  
> -	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
> -	if (!pos)
> +	if (!pci_is_pcie(parent_dev))
>  		return -1;
>  
> -	pci_read_config_word(parent_dev, pos + reg, value);
> +	pcie_capability_read_word(parent_dev, reg, value);
>  	return 0;
>  }
>  

Here's the caller of ixgbe_read_pci_cfg_word_parent():

    /* Get the negotiated link width and speed from PCI config space of the
     * parent, as this device is behind a switch
     */
    err = ixgbe_read_pci_cfg_word_parent(adapter, 18, &link_status);

This should be using PCI_EXP_LNKSTA instead of "18".

But it would be even better if we could drop ixgbe_get_parent_bus_info()
completely.  It seems redundant after merging Jacob's new
pcie_get_minimum_link() stuff [1].

ixgbe_disable_pcie_master() looks like it should be using
pcie_capability_read_word() with PCI_EXP_DEVSTA instead of using
IXGBE_PCI_DEVICE_STATUS.  If fact, it looks like it could use the
new pci_wait_for_pending_transaction() interface [2].

It looks like all the #defines in the "PCI Bus Info" block
(IXGBE_PCI_DEVICE_STATUS, IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING,
IXGBE_PCI_LINK_STATUS, etc.) [3] are really for PCIe-generic things.  If
so, the IXGBE-specific ones should be dropped in favor of the generic
ones.

[1] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=e027d1aec4bb49030646d2c186a721f94372d7f2
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/pci.c?id=3775a209d38aa3a0c7ed89a7d0f529e0230f280e
[3] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h#n1833

Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Keller, Jacob E]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3392 bytes --]

On Wed, 2013-09-04 at 10:20 -0600, Bjorn Helgaas wrote:
> [+cc Jacob, Jeff]
> 
> On Tue, Sep 03, 2013 at 03:35:13PM +0800, Yijing Wang wrote:
> > use pcie_capability_read_word() to simplify code.
> > 
> > Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> > Cc: e1000-devel@lists.sourceforge.net
> > Cc: netdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index bad8f14..bfa0b06 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
> >  static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> >  					  u32 reg, u16 *value)
> >  {
> > -	int pos = 0;
> >  	struct pci_dev *parent_dev;
> >  	struct pci_bus *parent_bus;
> >  
> > @@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> >  	if (!parent_dev)
> >  		return -1;
> >  
> > -	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
> > -	if (!pos)
> > +	if (!pci_is_pcie(parent_dev))
> >  		return -1;
> >  
> > -	pci_read_config_word(parent_dev, pos + reg, value);
> > +	pcie_capability_read_word(parent_dev, reg, value);
> >  	return 0;
> >  }
> >  
> 
> Here's the caller of ixgbe_read_pci_cfg_word_parent():
> 
>     /* Get the negotiated link width and speed from PCI config space of the
>      * parent, as this device is behind a switch
>      */
>     err = ixgbe_read_pci_cfg_word_parent(adapter, 18, &link_status);
> 
> This should be using PCI_EXP_LNKSTA instead of "18".

Absolutely.. Not sure why I didn't do this originally.....

> 
> But it would be even better if we could drop ixgbe_get_parent_bus_info()
> completely.  It seems redundant after merging Jacob's new
> pcie_get_minimum_link() stuff [1].

I don't know if we can fully drop it. We need this in order to read the
parent device on some quad port Ethernet adapters which have an internal
PCIe switch to link two parts together. There are a few places we read
the parent cfg word. At least one for sure.. but maybe others.. Can't
recall. The parent bus info is still used to print out the slot width
and speed. I don't know if it would be better to just print the response
from get_minimum_link or still print the actual slot.

> 
> ixgbe_disable_pcie_master() looks like it should be using
> pcie_capability_read_word() with PCI_EXP_DEVSTA instead of using
> IXGBE_PCI_DEVICE_STATUS.  If fact, it looks like it could use the
> new pci_wait_for_pending_transaction() interface [2].
> 

I can look at doing this. I know it was done this way for historic
reasons, (and likely for code share with non Linux drivers.. but that's
not really an excuse)

> It looks like all the #defines in the "PCI Bus Info" block
> (IXGBE_PCI_DEVICE_STATUS, IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING,
> IXGBE_PCI_LINK_STATUS, etc.) [3] are really for PCIe-generic things.  If
> so, the IXGBE-specific ones should be dropped in favor of the generic
> ones.

Agreed.

Thanks,
Jake


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Keller, Jacob E]

On Wed, 2013-09-04 at 10:20 -0600, Bjorn Helgaas wrote:
> [+cc Jacob, Jeff]
> 
> On Tue, Sep 03, 2013 at 03:35:13PM +0800, Yijing Wang wrote:
> > use pcie_capability_read_word() to simplify code.
> > 
> > Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> > Cc: e1000-devel@lists.sourceforge.net
> > Cc: netdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index bad8f14..bfa0b06 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
> >  static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> >  					  u32 reg, u16 *value)
> >  {
> > -	int pos = 0;
> >  	struct pci_dev *parent_dev;
> >  	struct pci_bus *parent_bus;
> >  
> > @@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> >  	if (!parent_dev)
> >  		return -1;
> >  
> > -	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
> > -	if (!pos)
> > +	if (!pci_is_pcie(parent_dev))
> >  		return -1;
> >  
> > -	pci_read_config_word(parent_dev, pos + reg, value);
> > +	pcie_capability_read_word(parent_dev, reg, value);
> >  	return 0;
> >  }
> >  
> 
> Here's the caller of ixgbe_read_pci_cfg_word_parent():
> 
>     /* Get the negotiated link width and speed from PCI config space of the
>      * parent, as this device is behind a switch
>      */
>     err = ixgbe_read_pci_cfg_word_parent(adapter, 18, &link_status);
> 
> This should be using PCI_EXP_LNKSTA instead of "18".

Absolutely.. Not sure why I didn't do this originally.....

> 
> But it would be even better if we could drop ixgbe_get_parent_bus_info()
> completely.  It seems redundant after merging Jacob's new
> pcie_get_minimum_link() stuff [1].

I don't know if we can fully drop it. We need this in order to read the
parent device on some quad port Ethernet adapters which have an internal
PCIe switch to link two parts together. There are a few places we read
the parent cfg word. At least one for sure.. but maybe others.. Can't
recall. The parent bus info is still used to print out the slot width
and speed. I don't know if it would be better to just print the response
from get_minimum_link or still print the actual slot.

> 
> ixgbe_disable_pcie_master() looks like it should be using
> pcie_capability_read_word() with PCI_EXP_DEVSTA instead of using
> IXGBE_PCI_DEVICE_STATUS.  If fact, it looks like it could use the
> new pci_wait_for_pending_transaction() interface [2].
> 

I can look at doing this. I know it was done this way for historic
reasons, (and likely for code share with non Linux drivers.. but that's
not really an excuse)

> It looks like all the #defines in the "PCI Bus Info" block
> (IXGBE_PCI_DEVICE_STATUS, IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING,
> IXGBE_PCI_LINK_STATUS, etc.) [3] are really for PCIe-generic things.  If
> so, the IXGBE-specific ones should be dropped in favor of the generic
> ones.

Agreed.

Thanks,
Jake

Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code

[Keller, Jacob E]

T24gV2VkLCAyMDEzLTA5LTA0IGF0IDEwOjIwIC0wNjAwLCBCam9ybiBIZWxnYWFzIHdyb3RlOg0K
PiBbK2NjIEphY29iLCBKZWZmXQ0KPiANCj4gT24gVHVlLCBTZXAgMDMsIDIwMTMgYXQgMDM6MzU6
MTNQTSArMDgwMCwgWWlqaW5nIFdhbmcgd3JvdGU6DQo+ID4gdXNlIHBjaWVfY2FwYWJpbGl0eV9y
ZWFkX3dvcmQoKSB0byBzaW1wbGlmeSBjb2RlLg0KPiA+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IFlp
amluZyBXYW5nIDx3YW5neWlqaW5nQGh1YXdlaS5jb20+DQo+ID4gQ2M6IGUxMDAwLWRldmVsQGxp
c3RzLnNvdXJjZWZvcmdlLm5ldA0KPiA+IENjOiBuZXRkZXZAdmdlci5rZXJuZWwub3JnDQo+ID4g
Q2M6IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmcNCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9u
ZXQvZXRoZXJuZXQvaW50ZWwvaXhnYmUvaXhnYmVfbWFpbi5jIHwgICAgNiArKy0tLS0NCj4gPiAg
MSBmaWxlcyBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pDQo+ID4gDQo+
ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L2V0aGVybmV0L2ludGVsL2l4Z2JlL2l4Z2JlX21h
aW4uYyBiL2RyaXZlcnMvbmV0L2V0aGVybmV0L2ludGVsL2l4Z2JlL2l4Z2JlX21haW4uYw0KPiA+
IGluZGV4IGJhZDhmMTQuLmJmYTBiMDYgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9uZXQvZXRo
ZXJuZXQvaW50ZWwvaXhnYmUvaXhnYmVfbWFpbi5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvZXRo
ZXJuZXQvaW50ZWwvaXhnYmUvaXhnYmVfbWFpbi5jDQo+ID4gQEAgLTE1Miw3ICsxNTIsNiBAQCBN
T0RVTEVfVkVSU0lPTihEUlZfVkVSU0lPTik7DQo+ID4gIHN0YXRpYyBpbnQgaXhnYmVfcmVhZF9w
Y2lfY2ZnX3dvcmRfcGFyZW50KHN0cnVjdCBpeGdiZV9hZGFwdGVyICphZGFwdGVyLA0KPiA+ICAJ
CQkJCSAgdTMyIHJlZywgdTE2ICp2YWx1ZSkNCj4gPiAgew0KPiA+IC0JaW50IHBvcyA9IDA7DQo+
ID4gIAlzdHJ1Y3QgcGNpX2RldiAqcGFyZW50X2RldjsNCj4gPiAgCXN0cnVjdCBwY2lfYnVzICpw
YXJlbnRfYnVzOw0KPiA+ICANCj4gPiBAQCAtMTY0LDExICsxNjMsMTAgQEAgc3RhdGljIGludCBp
eGdiZV9yZWFkX3BjaV9jZmdfd29yZF9wYXJlbnQoc3RydWN0IGl4Z2JlX2FkYXB0ZXIgKmFkYXB0
ZXIsDQo+ID4gIAlpZiAoIXBhcmVudF9kZXYpDQo+ID4gIAkJcmV0dXJuIC0xOw0KPiA+ICANCj4g
PiAtCXBvcyA9IHBjaV9maW5kX2NhcGFiaWxpdHkocGFyZW50X2RldiwgUENJX0NBUF9JRF9FWFAp
Ow0KPiA+IC0JaWYgKCFwb3MpDQo+ID4gKwlpZiAoIXBjaV9pc19wY2llKHBhcmVudF9kZXYpKQ0K
PiA+ICAJCXJldHVybiAtMTsNCj4gPiAgDQo+ID4gLQlwY2lfcmVhZF9jb25maWdfd29yZChwYXJl
bnRfZGV2LCBwb3MgKyByZWcsIHZhbHVlKTsNCj4gPiArCXBjaWVfY2FwYWJpbGl0eV9yZWFkX3dv
cmQocGFyZW50X2RldiwgcmVnLCB2YWx1ZSk7DQo+ID4gIAlyZXR1cm4gMDsNCj4gPiAgfQ0KPiA+
ICANCj4gDQo+IEhlcmUncyB0aGUgY2FsbGVyIG9mIGl4Z2JlX3JlYWRfcGNpX2NmZ193b3JkX3Bh
cmVudCgpOg0KPiANCj4gICAgIC8qIEdldCB0aGUgbmVnb3RpYXRlZCBsaW5rIHdpZHRoIGFuZCBz
cGVlZCBmcm9tIFBDSSBjb25maWcgc3BhY2Ugb2YgdGhlDQo+ICAgICAgKiBwYXJlbnQsIGFzIHRo
aXMgZGV2aWNlIGlzIGJlaGluZCBhIHN3aXRjaA0KPiAgICAgICovDQo+ICAgICBlcnIgPSBpeGdi
ZV9yZWFkX3BjaV9jZmdfd29yZF9wYXJlbnQoYWRhcHRlciwgMTgsICZsaW5rX3N0YXR1cyk7DQo+
IA0KPiBUaGlzIHNob3VsZCBiZSB1c2luZyBQQ0lfRVhQX0xOS1NUQSBpbnN0ZWFkIG9mICIxOCIu
DQoNCkFic29sdXRlbHkuLiBOb3Qgc3VyZSB3aHkgSSBkaWRuJ3QgZG8gdGhpcyBvcmlnaW5hbGx5
Li4uLi4NCg0KPiANCj4gQnV0IGl0IHdvdWxkIGJlIGV2ZW4gYmV0dGVyIGlmIHdlIGNvdWxkIGRy
b3AgaXhnYmVfZ2V0X3BhcmVudF9idXNfaW5mbygpDQo+IGNvbXBsZXRlbHkuICBJdCBzZWVtcyBy
ZWR1bmRhbnQgYWZ0ZXIgbWVyZ2luZyBKYWNvYidzIG5ldw0KPiBwY2llX2dldF9taW5pbXVtX2xp
bmsoKSBzdHVmZiBbMV0uDQoNCkkgZG9uJ3Qga25vdyBpZiB3ZSBjYW4gZnVsbHkgZHJvcCBpdC4g
V2UgbmVlZCB0aGlzIGluIG9yZGVyIHRvIHJlYWQgdGhlDQpwYXJlbnQgZGV2aWNlIG9uIHNvbWUg
cXVhZCBwb3J0IEV0aGVybmV0IGFkYXB0ZXJzIHdoaWNoIGhhdmUgYW4gaW50ZXJuYWwNClBDSWUg
c3dpdGNoIHRvIGxpbmsgdHdvIHBhcnRzIHRvZ2V0aGVyLiBUaGVyZSBhcmUgYSBmZXcgcGxhY2Vz
IHdlIHJlYWQNCnRoZSBwYXJlbnQgY2ZnIHdvcmQuIEF0IGxlYXN0IG9uZSBmb3Igc3VyZS4uIGJ1
dCBtYXliZSBvdGhlcnMuLiBDYW4ndA0KcmVjYWxsLiBUaGUgcGFyZW50IGJ1cyBpbmZvIGlzIHN0
aWxsIHVzZWQgdG8gcHJpbnQgb3V0IHRoZSBzbG90IHdpZHRoDQphbmQgc3BlZWQuIEkgZG9uJ3Qg
a25vdyBpZiBpdCB3b3VsZCBiZSBiZXR0ZXIgdG8ganVzdCBwcmludCB0aGUgcmVzcG9uc2UNCmZy
b20gZ2V0X21pbmltdW1fbGluayBvciBzdGlsbCBwcmludCB0aGUgYWN0dWFsIHNsb3QuDQoNCj4g
DQo+IGl4Z2JlX2Rpc2FibGVfcGNpZV9tYXN0ZXIoKSBsb29rcyBsaWtlIGl0IHNob3VsZCBiZSB1
c2luZw0KPiBwY2llX2NhcGFiaWxpdHlfcmVhZF93b3JkKCkgd2l0aCBQQ0lfRVhQX0RFVlNUQSBp
bnN0ZWFkIG9mIHVzaW5nDQo+IElYR0JFX1BDSV9ERVZJQ0VfU1RBVFVTLiAgSWYgZmFjdCwgaXQg
bG9va3MgbGlrZSBpdCBjb3VsZCB1c2UgdGhlDQo+IG5ldyBwY2lfd2FpdF9mb3JfcGVuZGluZ190
cmFuc2FjdGlvbigpIGludGVyZmFjZSBbMl0uDQo+IA0KDQpJIGNhbiBsb29rIGF0IGRvaW5nIHRo
aXMuIEkga25vdyBpdCB3YXMgZG9uZSB0aGlzIHdheSBmb3IgaGlzdG9yaWMNCnJlYXNvbnMsIChh
bmQgbGlrZWx5IGZvciBjb2RlIHNoYXJlIHdpdGggbm9uIExpbnV4IGRyaXZlcnMuLiBidXQgdGhh
dCdzDQpub3QgcmVhbGx5IGFuIGV4Y3VzZSkNCg0KPiBJdCBsb29rcyBsaWtlIGFsbCB0aGUgI2Rl
ZmluZXMgaW4gdGhlICJQQ0kgQnVzIEluZm8iIGJsb2NrDQo+IChJWEdCRV9QQ0lfREVWSUNFX1NU
QVRVUywgSVhHQkVfUENJX0RFVklDRV9TVEFUVVNfVFJBTlNBQ1RJT05fUEVORElORywNCj4gSVhH
QkVfUENJX0xJTktfU1RBVFVTLCBldGMuKSBbM10gYXJlIHJlYWxseSBmb3IgUENJZS1nZW5lcmlj
IHRoaW5ncy4gIElmDQo+IHNvLCB0aGUgSVhHQkUtc3BlY2lmaWMgb25lcyBzaG91bGQgYmUgZHJv
cHBlZCBpbiBmYXZvciBvZiB0aGUgZ2VuZXJpYw0KPiBvbmVzLg0KDQpBZ3JlZWQuDQoNClRoYW5r
cywNCkpha2UNCg0KDQo=

Re: [PATCH 3/7] powerpc/pci: use pci_is_pcie() to simplify code

[Kumar Gala]

On Sep 3, 2013, at 2:35 AM, Yijing Wang wrote:

> Use pci_is_pcie() to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/powerpc/kernel/eeh.c     |    3 +--
> arch/powerpc/sysdev/fsl_pci.c |    2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)

Acked-by: Kumar Gala <galak@kernel.crashing.org>

(for the fsl_pci.c) change

- k
 

Re: [PATCH 3/7] powerpc/pci: use pci_is_pcie() to simplify code

[Kumar Gala]

On Sep 3, 2013, at 2:35 AM, Yijing Wang wrote:

> Use pci_is_pcie() to simplify code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/powerpc/kernel/eeh.c     |    3 +--
> arch/powerpc/sysdev/fsl_pci.c |    2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)

Acked-by: Kumar Gala <galak@kernel.crashing.org>

(for the fsl_pci.c) change

- k
 

Re: [PATCH 4/7] x86/pci: use pcie_cap to simplify code

[Yijing Wang]

On 2013/9/4 10:59, Bjorn Helgaas wrote:
> On Tue, Sep 03, 2013 at 03:35:12PM +0800, Yijing Wang wrote:
>> PCI core saves PCIe Cap offset in pcie_cap,
>> use pcie_cap to simplify code.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  arch/x86/pci/fixup.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
>> index f5809fa..ee8330d 100644
>> --- a/arch/x86/pci/fixup.c
>> +++ b/arch/x86/pci/fixup.c
>> @@ -288,7 +288,7 @@ static void pcie_rootport_aspm_quirk(struct pci_dev *pdev)
>>  		 */
>>  		list_for_each_entry(dev, &pbus->devices, bus_list) {
>>  			/* There are 0 to 8 devices attached to this bus */
>> -			cap_base = pci_find_capability(dev, PCI_CAP_ID_EXP);
>> +			cap_base = dev->pcie_cap;
>>  			quirk_aspm_offset[GET_INDEX(pdev->device, dev->devfn)] = cap_base + 0x10;
> 
> This should use PCI_EXP_LNKCTL instead of "0x10".
> 
>>  		}
>>  		pbus->ops = &quirk_pcie_aspm_ops;

Hi Bjorn,
   Thanks for your review and comments!

> 
> This quirk replaces the config accessors with ones that silently ignore
> writes to ASPM control bits.  That really warrants at least a dev_info()
> note here, and we should be using pci_bus_set_ops().

Good idea, I will update it, thanks!

> 
> Even that is a little bit dubious because I don't think this is really
> safe -- what happens if this quirk replaces the ops, then somebody
> else replaces the ops again?  aer_inject.c at least keeps track of
> the old ops and seems to fall back to them, but it seems fragile to
> depend on every caller of pci_bus_set_ops() to do the right thing
> there.
> 
> But this is beyond the scope of your patch, so if you just
> add a dev_info() note and use pci_bus_set_ops(), that should be
> enough for now.
> 
> 


-- 
Thanks!
Yijing

Re: [PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Yijing Wang]

>> @@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>  			break;
>>  		}
>>  
>> -		pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> -		if (mask != 0xffff && pcie_cap_reg) {
>> -			pcie_cap_reg += 0x08;
>> -			pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
>> +		if (mask != 0xffff && pci_is_pcie(pdev)) {
> 
> Please move the pci_is_pcie() test up to the
> "if (pcie_mas_read_reqsz ..." statement.  There's no point in doing
> the switch statement if this isn't a PCIe device.

Right, will update.

> 
>> +			pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
>>  			if ((pcie_dev_ctl & 0x7000) != mask) {
>>  				printk(KERN_WARNING "BFA[%s]: "
>>  				"pcie_max_read_request_size is %d, "
>> @@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>  				pcie_max_read_reqsz);
>>  
>>  				pcie_dev_ctl &= ~0x7000;
>> -				pci_write_config_word(pdev, pcie_cap_reg,
>> +				pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
>>  						pcie_dev_ctl | mask);
> 
> Please rework this to use pcie_set_readrq() instead of writing
> the capability directly.  If we write the capability directly, we
> risk writing a value that is incompatible with the MPS
> configuration done by the PCI core.

Ah, this code is Long-winded, use pcie_set_readrq()/pcie_get_readrq() can simplify
this code much.

Thanks!
Yijing.

> 
>>  			}
>>  		}
>> -- 
>> 1.7.1
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code

[Yijing Wang]

>> @@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>  			break;
>>  		}
>>  
>> -		pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> -		if (mask != 0xffff && pcie_cap_reg) {
>> -			pcie_cap_reg += 0x08;
>> -			pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
>> +		if (mask != 0xffff && pci_is_pcie(pdev)) {
> 
> Please move the pci_is_pcie() test up to the
> "if (pcie_mas_read_reqsz ..." statement.  There's no point in doing
> the switch statement if this isn't a PCIe device.

Right, will update.

> 
>> +			pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &pcie_dev_ctl);
>>  			if ((pcie_dev_ctl & 0x7000) != mask) {
>>  				printk(KERN_WARNING "BFA[%s]: "
>>  				"pcie_max_read_request_size is %d, "
>> @@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>>  				pcie_max_read_reqsz);
>>  
>>  				pcie_dev_ctl &= ~0x7000;
>> -				pci_write_config_word(pdev, pcie_cap_reg,
>> +				pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
>>  						pcie_dev_ctl | mask);
> 
> Please rework this to use pcie_set_readrq() instead of writing
> the capability directly.  If we write the capability directly, we
> risk writing a value that is incompatible with the MPS
> configuration done by the PCI core.

Ah, this code is Long-winded, use pcie_set_readrq()/pcie_get_readrq() can simplify
this code much.

Thanks!
Yijing.

> 
>>  			}
>>  		}
>> -- 
>> 1.7.1
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code

[Yijing Wang]

On 2013/9/4 7:43, Bjorn Helgaas wrote:
> On Tue, Sep 03, 2013 at 03:35:10PM +0800, Yijing Wang wrote:
>> Pcie_capability_xxx() interfaces were introudced to
> 
> s/introudced/introduced/

Will update it.

> 
>> simplify code to access PCIe Cap config space. And
>> because PCI core saves the PCIe Cap offset in
>> set_pcie_port_type() when device is enumerated.
>> So we can use pci_is_pcie() instead.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> Cc: Jiang Liu <jiang.liu@huawei.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: Naresh Kumar Inna <naresh@chelsio.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Jesper Juhl <jj@chaosbits.net>
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  drivers/scsi/csiostor/csio_hw.c |    9 +++------
>>  1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
>> index 0eb35b9..be9a6ef 100644
>> --- a/drivers/scsi/csiostor/csio_hw.c
>> +++ b/drivers/scsi/csiostor/csio_hw.c
>> @@ -856,15 +856,12 @@ static void
>>  csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
>>  {
>>  	uint16_t val;
>> -	int pcie_cap;
>>  
>> -	if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
>> -		pci_read_config_word(hw->pdev,
>> -				     pcie_cap + PCI_EXP_DEVCTL2, &val);
>> +	if (pci_is_pcie(hw->pdev)) {
>> +		pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
>>  		val &= 0xfff0;
>>  		val |= range ;
>> -		pci_write_config_word(hw->pdev,
>> -				      pcie_cap + PCI_EXP_DEVCTL2, val);
>> +		pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
> 
> Please add a #define for the Completion Timeout Value field and use
> pcie_capability_clear_and_set_word() instead of writing it out the
> long way here.

Will update it, thanks!

> 
>>  	}
>>  }
>>  
>> -- 
>> 1.7.1
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code

[Yijing Wang]

On 2013/9/4 7:43, Bjorn Helgaas wrote:
> On Tue, Sep 03, 2013 at 03:35:10PM +0800, Yijing Wang wrote:
>> Pcie_capability_xxx() interfaces were introudced to
> 
> s/introudced/introduced/

Will update it.

> 
>> simplify code to access PCIe Cap config space. And
>> because PCI core saves the PCIe Cap offset in
>> set_pcie_port_type() when device is enumerated.
>> So we can use pci_is_pcie() instead.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> Cc: Jiang Liu <jiang.liu@huawei.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: Naresh Kumar Inna <naresh@chelsio.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Jesper Juhl <jj@chaosbits.net>
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  drivers/scsi/csiostor/csio_hw.c |    9 +++------
>>  1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
>> index 0eb35b9..be9a6ef 100644
>> --- a/drivers/scsi/csiostor/csio_hw.c
>> +++ b/drivers/scsi/csiostor/csio_hw.c
>> @@ -856,15 +856,12 @@ static void
>>  csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
>>  {
>>  	uint16_t val;
>> -	int pcie_cap;
>>  
>> -	if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
>> -		pci_read_config_word(hw->pdev,
>> -				     pcie_cap + PCI_EXP_DEVCTL2, &val);
>> +	if (pci_is_pcie(hw->pdev)) {
>> +		pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
>>  		val &= 0xfff0;
>>  		val |= range ;
>> -		pci_write_config_word(hw->pdev,
>> -				      pcie_cap + PCI_EXP_DEVCTL2, val);
>> +		pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
> 
> Please add a #define for the Completion Timeout Value field and use
> pcie_capability_clear_and_set_word() instead of writing it out the
> long way here.

Will update it, thanks!

> 
>>  	}
>>  }
>>  
>> -- 
>> 1.7.1
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing