[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 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 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 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

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: [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

[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]

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 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 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