2017-04-14 19:11:47

by Christoph Hellwig

[permalink] [raw]
Subject: export pcie_flr and remove copies of it in drivers V2

Hi all,

this exports the PCI layer pcie_flr helper, and removes various opencoded
copies of it.

Changes since V1:
- rebase on top of the pci/virtualization branch
- fixed the probe case in __pci_dev_reset
- added ACKs from Bjorn


2017-04-14 19:11:54

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/7] PCI: call pcie_flr from reset_intel_82599_sfp_virtfn

The 82599 quirk contained an outdated copy of the FLR code.

Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/quirks.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 823271b13d12..8195ca294ee5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3641,19 +3641,11 @@ static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
*
* The 82599 supports FLR on VFs, but FLR support is reported only
* in the PF DEVCAP (sec 9.3.10.4), not in the VF DEVCAP (sec 9.5).
- * Therefore, we can't use pcie_flr(), which checks the VF DEVCAP.
+ * Thus we must call pcie_flr directly without first checking if it is
+ * supported.
*/
-
- if (probe)
- return 0;
-
- if (!pci_wait_for_pending_transaction(dev))
- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
-
- pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
-
- msleep(100);
-
+ if (!probe)
+ pcie_flr(dev);
return 0;
}

--
2.11.0

2017-04-14 19:12:05

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/7] ixgbe: use pcie_flr instead of duplicating it

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a7a430a7be2c..543ddde5f8e2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7112,18 +7112,6 @@ static void ixgbe_watchdog_flush_tx(struct ixgbe_adapter *adapter)
}

#ifdef CONFIG_PCI_IOV
-static inline void ixgbe_issue_vf_flr(struct ixgbe_adapter *adapter,
- struct pci_dev *vfdev)
-{
- if (!pci_wait_for_pending_transaction(vfdev))
- e_dev_warn("Issuing VFLR with pending transactions\n");
-
- e_dev_err("Issuing VFLR for VF %s\n", pci_name(vfdev));
- pcie_capability_set_word(vfdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
-
- msleep(100);
-}
-
static void ixgbe_check_for_bad_vf(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
@@ -7156,7 +7144,7 @@ static void ixgbe_check_for_bad_vf(struct ixgbe_adapter *adapter)
pci_read_config_word(vfdev, PCI_STATUS, &status_reg);
if (status_reg != IXGBE_FAILED_READ_CFG_WORD &&
status_reg & PCI_STATUS_REC_MASTER_ABORT)
- ixgbe_issue_vf_flr(adapter, vfdev);
+ pcie_flr(vfdev);
}
}

@@ -10244,7 +10232,7 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
* VFLR. Just clean up the AER in that case.
*/
if (vfdev) {
- ixgbe_issue_vf_flr(adapter, vfdev);
+ pcie_flr(vfdev);
/* Free device reference count */
pci_dev_put(vfdev);
}
--
2.11.0

2017-04-14 19:12:09

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/infiniband/hw/hfi1/chip.c | 4 ++--
drivers/infiniband/hw/hfi1/hfi.h | 1 -
drivers/infiniband/hw/hfi1/pcie.c | 30 ------------------------------
3 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 121a4c920f1b..d037f72e4d96 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -13610,14 +13610,14 @@ static void init_chip(struct hfi1_devdata *dd)
dd_dev_info(dd, "Resetting CSRs with FLR\n");

/* do the FLR, the DC reset will remain */
- hfi1_pcie_flr(dd);
+ pcie_flr(dd->pcidev);

/* restore command and BARs */
restore_pci_variables(dd);

if (is_ax(dd)) {
dd_dev_info(dd, "Resetting CSRs with FLR\n");
- hfi1_pcie_flr(dd);
+ pcie_flr(dd->pcidev);
restore_pci_variables(dd);
}
} else {
diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h
index 0808e3c3ba39..40d7559fa723 100644
--- a/drivers/infiniband/hw/hfi1/hfi.h
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -1764,7 +1764,6 @@ int hfi1_pcie_init(struct pci_dev *, const struct pci_device_id *);
void hfi1_pcie_cleanup(struct pci_dev *);
int hfi1_pcie_ddinit(struct hfi1_devdata *, struct pci_dev *);
void hfi1_pcie_ddcleanup(struct hfi1_devdata *);
-void hfi1_pcie_flr(struct hfi1_devdata *);
int pcie_speeds(struct hfi1_devdata *);
void request_msix(struct hfi1_devdata *, u32 *, struct hfi1_msix_entry *);
void hfi1_enable_intx(struct pci_dev *);
diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 0829fce06172..c81556e84831 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -240,36 +240,6 @@ void hfi1_pcie_ddcleanup(struct hfi1_devdata *dd)
iounmap(dd->piobase);
}

-/*
- * Do a Function Level Reset (FLR) on the device.
- * Based on static function drivers/pci/pci.c:pcie_flr().
- */
-void hfi1_pcie_flr(struct hfi1_devdata *dd)
-{
- int i;
- u16 status;
-
- /* no need to check for the capability - we know the device has it */
-
- /* wait for Transaction Pending bit to clear, at most a few ms */
- for (i = 0; i < 4; i++) {
- if (i)
- msleep((1 << (i - 1)) * 100);
-
- pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVSTA, &status);
- if (!(status & PCI_EXP_DEVSTA_TRPND))
- goto clear;
- }
-
- dd_dev_err(dd, "Transaction Pending bit is not clearing, proceeding with reset anyway\n");
-
-clear:
- pcie_capability_set_word(dd->pcidev, PCI_EXP_DEVCTL,
- PCI_EXP_DEVCTL_BCR_FLR);
- /* PCIe spec requires the function to be back within 100ms */
- msleep(100);
-}
-
static void msix_setup(struct hfi1_devdata *dd, int pos, u32 *msixcnt,
struct hfi1_msix_entry *hfi1_msix_entry)
{
--
2.11.0

2017-04-14 19:12:34

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 9d5e03502c76..afdbf7fa016e 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -869,20 +869,7 @@ static void octeon_pci_flr(struct octeon_device *oct)
pci_write_config_word(oct->pci_dev, PCI_COMMAND,
PCI_COMMAND_INTX_DISABLE);

- /* Wait for Transaction Pending bit clean */
- msleep(100);
- pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA, &status);
- if (status & PCI_EXP_DEVSTA_TRPND) {
- dev_info(&oct->pci_dev->dev, "Function reset incomplete after 100ms, sleeping for 5 seconds\n");
- ssleep(5);
- pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA,
- &status);
- if (status & PCI_EXP_DEVSTA_TRPND)
- dev_info(&oct->pci_dev->dev, "Function reset still incomplete after 5s, reset anyway\n");
- }
- pcie_capability_set_word(oct->pci_dev, PCI_EXP_DEVCTL,
- PCI_EXP_DEVCTL_BCR_FLR);
- mdelay(100);
+ pcie_flr(oct->pci_dev);

pci_cfg_access_unlock(oct->pci_dev);

--
2.11.0

2017-04-14 19:13:40

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 6/7] crypto: qat: use pcie_flr instead of duplicating it

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/crypto/qat/qat_common/adf_aer.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_aer.c b/drivers/crypto/qat/qat_common/adf_aer.c
index 2839fccdd84b..d3e25c37dc33 100644
--- a/drivers/crypto/qat/qat_common/adf_aer.c
+++ b/drivers/crypto/qat/qat_common/adf_aer.c
@@ -109,20 +109,7 @@ EXPORT_SYMBOL_GPL(adf_reset_sbr);

void adf_reset_flr(struct adf_accel_dev *accel_dev)
{
- struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
- u16 control = 0;
- int pos = 0;
-
- dev_info(&GET_DEV(accel_dev), "Function level reset\n");
- pos = pci_pcie_cap(pdev);
- if (!pos) {
- dev_err(&GET_DEV(accel_dev), "Restart device failed\n");
- return;
- }
- pci_read_config_word(pdev, pos + PCI_EXP_DEVCTL, &control);
- control |= PCI_EXP_DEVCTL_BCR_FLR;
- pci_write_config_word(pdev, pos + PCI_EXP_DEVCTL, control);
- msleep(100);
+ pcie_flr(accel_to_pci_dev(accel_dev));
}
EXPORT_SYMBOL_GPL(adf_reset_flr);

--
2.11.0

2017-04-14 19:14:21

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/7] PCI: call pcie_flr from reset_chelsio_generic_dev

Instead of copy & pasting and old version of the code.

Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/quirks.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 8195ca294ee5..715ed8c08fa3 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3750,20 +3750,7 @@ static int reset_chelsio_generic_dev(struct pci_dev *dev, int probe)
PCI_MSIX_FLAGS_ENABLE |
PCI_MSIX_FLAGS_MASKALL);

- /*
- * Start of pcie_flr() code sequence. This reset code is a copy of
- * the guts of pcie_flr() because that's not an exported function.
- */
-
- if (!pci_wait_for_pending_transaction(dev))
- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
-
- pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
- msleep(100);
-
- /*
- * End of pcie_flr() code sequence.
- */
+ pcie_flr(dev);

/*
* Restore the configuration information (BAR values, etc.) including
--
2.11.0

2017-04-14 19:15:33

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/7] PCI: export pcie_flr

Currently we opencode the FLR sequence in lots of place, export a core
helper instead. We split out the probing for FLR support as all the
non-core callers already know their hardware.

Note that in the new pci_has_flr function the quirk check has been moved
before the capability check as there is no point in reading the
capability in this case.

Signed-off-by: Christoph Hellwig <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/pci.c | 39 ++++++++++++++++++++++++++++-----------
include/linux/pci.h | 1 +
2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bef14777bb30..957a11a6a840 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3773,27 +3773,41 @@ static void pci_flr_wait(struct pci_dev *dev)
(i - 1) * 100);
}

-static int pcie_flr(struct pci_dev *dev, int probe)
+/**
+ * pcie_has_flr - check if a device supports function level resets
+ * @dev: device to check
+ *
+ * Returns true if the device advertises support for PCIe function level
+ * resets.
+ */
+static bool pcie_has_flr(struct pci_dev *dev)
{
u32 cap;

- pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
- if (!(cap & PCI_EXP_DEVCAP_FLR))
- return -ENOTTY;
-
if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
- return -ENOTTY;
+ return false;

- if (probe)
- return 0;
+ pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
+ return cap & PCI_EXP_DEVCAP_FLR;
+}

+/**
+ * pcie_flr - initiate a PCIe function level reset
+ * @dev: device to reset
+ *
+ * Initiate a function level reset on @dev. The caller should ensure the
+ * device supports FLR before calling this function, e.g. by using the
+ * pcie_has_flr() helper.
+ */
+void pcie_flr(struct pci_dev *dev)
+{
if (!pci_wait_for_pending_transaction(dev))
dev_err(&dev->dev, "timed out waiting for pending transaction; performing function level reset anyway\n");

pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
pci_flr_wait(dev);
- return 0;
}
+EXPORT_SYMBOL_GPL(pcie_flr);

static int pci_af_flr(struct pci_dev *dev, int probe)
{
@@ -3977,9 +3991,12 @@ static int __pci_dev_reset(struct pci_dev *dev, int probe)
if (rc != -ENOTTY)
goto done;

- rc = pcie_flr(dev, probe);
- if (rc != -ENOTTY)
+ if (pcie_has_flr(dev)) {
+ if (!probe)
+ pcie_flr(dev);
+ rc = 0;
goto done;
+ }

rc = pci_af_flr(dev, probe);
if (rc != -ENOTTY)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 20e1865233a4..60162f51227a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1054,6 +1054,7 @@ int pcie_get_mps(struct pci_dev *dev);
int pcie_set_mps(struct pci_dev *dev, int mps);
int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
enum pcie_link_width *width);
+void pcie_flr(struct pci_dev *dev);
int __pci_reset_function(struct pci_dev *dev);
int __pci_reset_function_locked(struct pci_dev *dev);
int pci_reset_function(struct pci_dev *dev);
--
2.11.0

2017-04-18 18:36:19

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: export pcie_flr and remove copies of it in drivers V2

On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> Hi all,
>
> this exports the PCI layer pcie_flr helper, and removes various opencoded
> copies of it.
>
> Changes since V1:
> - rebase on top of the pci/virtualization branch
> - fixed the probe case in __pci_dev_reset
> - added ACKs from Bjorn

Applied the first three patches:

bc13871ef35a PCI: Export pcie_flr()
e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()

to pci/virtualization for v4.12, thanks!

2017-04-19 05:37:25

by Leon Romanovsky

[permalink] [raw]
Subject: Re: export pcie_flr and remove copies of it in drivers V2

On Tue, Apr 18, 2017 at 01:36:12PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> > Hi all,
> >
> > this exports the PCI layer pcie_flr helper, and removes various opencoded
> > copies of it.
> >
> > Changes since V1:
> > - rebase on top of the pci/virtualization branch
> > - fixed the probe case in __pci_dev_reset
> > - added ACKs from Bjorn
>
> Applied the first three patches:
>
> bc13871ef35a PCI: Export pcie_flr()
> e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
> 40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()
>

Bjorn,

How do you suggest to proceed with other patches? They should be applied
to your tree either, because they depend on "bc13871ef35a PCI: Export
pcie_flr()".

Thanks


> to pci/virtualization for v4.12, thanks!


Attachments:
(No filename) (844.00 B)
signature.asc (833.00 B)
Download all attachments

2017-04-19 16:39:53

by Leon Romanovsky

[permalink] [raw]
Subject: Re: export pcie_flr and remove copies of it in drivers V2

On Wed, Apr 19, 2017 at 08:37:37AM +0300, Leon Romanovsky wrote:
> On Tue, Apr 18, 2017 at 01:36:12PM -0500, Bjorn Helgaas wrote:
> > On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> > > Hi all,
> > >
> > > this exports the PCI layer pcie_flr helper, and removes various opencoded
> > > copies of it.
> > >
> > > Changes since V1:
> > > - rebase on top of the pci/virtualization branch
> > > - fixed the probe case in __pci_dev_reset
> > > - added ACKs from Bjorn
> >
> > Applied the first three patches:
> >
> > bc13871ef35a PCI: Export pcie_flr()
> > e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
> > 40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()
> >
>
> Bjorn,
>
> How do you suggest to proceed with other patches? They should be applied
> to your tree either, because they depend on "bc13871ef35a PCI: Export
> pcie_flr()".

I finally caught the old emails and found the answer by myself.
http://marc.info/?l=linux-rdma&m=149218545228343&w=2

Thanks

>
> Thanks
>
>
> > to pci/virtualization for v4.12, thanks!



Attachments:
(No filename) (1.07 kB)
signature.asc (833.00 B)
Download all attachments

2017-04-24 14:16:45

by Byczkowski, Jakub

[permalink] [raw]
Subject: RE: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

Tested-by: Jakub Byczkowski <[email protected]>

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Christoph Hellwig
Sent: Friday, April 14, 2017 9:11 PM
To: Bjorn Helgaas <[email protected]>; Cabiddu, Giovanni <[email protected]>; Benedetto, Salvatore <[email protected]>; Marciniszyn, Mike <[email protected]>; Dalessandro, Dennis <[email protected]>; Derek Chickles <[email protected]>; Satanand Burla <[email protected]>; Felix Manlunas <[email protected]>; Raghu Vatsavayi <[email protected]>; Kirsher, Jeffrey T <[email protected]>
Cc: [email protected]; qat-linux <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/infiniband/hw/hfi1/chip.c | 4 ++-- drivers/infiniband/hw/hfi1/hfi.h | 1 - drivers/infiniband/hw/hfi1/pcie.c | 30 ------------------------------
3 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 121a4c920f1b..d037f72e4d96 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -13610,14 +13610,14 @@ static void init_chip(struct hfi1_devdata *dd)
dd_dev_info(dd, "Resetting CSRs with FLR\n");

/* do the FLR, the DC reset will remain */
- hfi1_pcie_flr(dd);
+ pcie_flr(dd->pcidev);

/* restore command and BARs */
restore_pci_variables(dd);

if (is_ax(dd)) {
dd_dev_info(dd, "Resetting CSRs with FLR\n");
- hfi1_pcie_flr(dd);
+ pcie_flr(dd->pcidev);
restore_pci_variables(dd);
}
} else {
diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h
index 0808e3c3ba39..40d7559fa723 100644
--- a/drivers/infiniband/hw/hfi1/hfi.h
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -1764,7 +1764,6 @@ int hfi1_pcie_init(struct pci_dev *, const struct pci_device_id *); void hfi1_pcie_cleanup(struct pci_dev *); int hfi1_pcie_ddinit(struct hfi1_devdata *, struct pci_dev *); void hfi1_pcie_ddcleanup(struct hfi1_devdata *); -void hfi1_pcie_flr(struct hfi1_devdata *); int pcie_speeds(struct hfi1_devdata *); void request_msix(struct hfi1_devdata *, u32 *, struct hfi1_msix_entry *); void hfi1_enable_intx(struct pci_dev *); diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 0829fce06172..c81556e84831 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -240,36 +240,6 @@ void hfi1_pcie_ddcleanup(struct hfi1_devdata *dd)
iounmap(dd->piobase);
}

-/*
- * Do a Function Level Reset (FLR) on the device.
- * Based on static function drivers/pci/pci.c:pcie_flr().
- */
-void hfi1_pcie_flr(struct hfi1_devdata *dd) -{
- int i;
- u16 status;
-
- /* no need to check for the capability - we know the device has it */
-
- /* wait for Transaction Pending bit to clear, at most a few ms */
- for (i = 0; i < 4; i++) {
- if (i)
- msleep((1 << (i - 1)) * 100);
-
- pcie_capability_read_word(dd->pcidev, PCI_EXP_DEVSTA, &status);
- if (!(status & PCI_EXP_DEVSTA_TRPND))
- goto clear;
- }
-
- dd_dev_err(dd, "Transaction Pending bit is not clearing, proceeding with reset anyway\n");
-
-clear:
- pcie_capability_set_word(dd->pcidev, PCI_EXP_DEVCTL,
- PCI_EXP_DEVCTL_BCR_FLR);
- /* PCIe spec requires the function to be back within 100ms */
- msleep(100);
-}
-
static void msix_setup(struct hfi1_devdata *dd, int pos, u32 *msixcnt,
struct hfi1_msix_entry *hfi1_msix_entry) {
--
2.11.0

2017-04-24 14:35:16

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Mon, Apr 24, 2017 at 02:16:31PM +0000, Byczkowski, Jakub wrote:
> Tested-by: Jakub Byczkowski <[email protected]>

Are you (and Doug) ok with queueing this up in the PCI tree?

2017-04-24 20:01:11

by Dennis Dalessandro

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On 04/24/2017 10:35 AM, Christoph Hellwig wrote:
> On Mon, Apr 24, 2017 at 02:16:31PM +0000, Byczkowski, Jakub wrote:
>> Tested-by: Jakub Byczkowski <[email protected]>
>
> Are you (and Doug) ok with queueing this up in the PCI tree?

We are fine however Doug wants to handle it.

-Denny


2017-04-25 01:54:44

by Felix Manlunas

[permalink] [raw]
Subject: Re: [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it

From: Christoph Hellwig <[email protected]>
Date: Fri, 14 Apr 2017 21:11:31 +0200

> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> index 9d5e03502c76..afdbf7fa016e 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> @@ -869,20 +869,7 @@ static void octeon_pci_flr(struct octeon_device *oct)
> pci_write_config_word(oct->pci_dev, PCI_COMMAND,
> PCI_COMMAND_INTX_DISABLE);
>
> - /* Wait for Transaction Pending bit clean */
> - msleep(100);
> - pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA, &status);
> - if (status & PCI_EXP_DEVSTA_TRPND) {
> - dev_info(&oct->pci_dev->dev, "Function reset incomplete after 100ms, sleeping for 5 seconds\n");
> - ssleep(5);
> - pcie_capability_read_word(oct->pci_dev, PCI_EXP_DEVSTA,
> - &status);
> - if (status & PCI_EXP_DEVSTA_TRPND)
> - dev_info(&oct->pci_dev->dev, "Function reset still incomplete after 5s, reset anyway\n");
> - }
> - pcie_capability_set_word(oct->pci_dev, PCI_EXP_DEVCTL,
> - PCI_EXP_DEVCTL_BCR_FLR);
> - mdelay(100);
> + pcie_flr(oct->pci_dev);
>
> pci_cfg_access_unlock(oct->pci_dev);
>
> --
> 2.11.0
>

This patch works. I tested it on a LiquidIO NIC and the "next" branch of the
PCI git tree.

But the patch causes a gcc warning:

.../lio_vf_main.c: In function 'octeon_pci_flr':
.../lio_vf_main.c:862:6: warning: unused variable 'status' [-Wunused-variable]

Can you rework the patch to get rid of the warning? Thanks.

2017-04-25 17:00:23

by Doug Ledford

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Mon, 2017-04-24 at 16:35 +0200, Christoph Hellwig wrote:
> On Mon, Apr 24, 2017 at 02:16:31PM +0000, Byczkowski, Jakub wrote:
> >
> > Tested-by: Jakub Byczkowski <[email protected]>
>
> Are you (and Doug) ok with queueing this up in the PCI tree?

I'm fine with that.  Feel free to add my Acked-by to the hfi1 patch.

--
Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

2017-04-25 19:40:07

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Mon, Apr 24, 2017 at 04:35:07PM +0200, Christoph Hellwig wrote:
> On Mon, Apr 24, 2017 at 02:16:31PM +0000, Byczkowski, Jakub wrote:
> > Tested-by: Jakub Byczkowski <[email protected]>
>
> Are you (and Doug) ok with queueing this up in the PCI tree?

Applied this with Jakub's tested-by and Doug's ack to pci/virtualization
for v4.12.

This still leaves these:

[PATCH 4/7] ixgbe: use pcie_flr instead of duplicating it
[PATCH 6/7] crypto: qat: use pcie_flr instead of duplicating it
[PATCH 7/7] liquidio: use pcie_flr instead of duplicating it

I haven't seen any response to 4 and 6. Felix reported an unused
variable in 7. Let me know if you'd like me to do anything with
these.

Bjorn

2017-04-26 22:10:45

by Jeff Kirsher

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Tue, 2017-04-25 at 14:39 -0500, Bjorn Helgaas wrote:
> On Mon, Apr 24, 2017 at 04:35:07PM +0200, Christoph Hellwig wrote:
> > On Mon, Apr 24, 2017 at 02:16:31PM +0000, Byczkowski, Jakub wrote:
> > > Tested-by: Jakub Byczkowski <[email protected]>
> >
> > Are you (and Doug) ok with queueing this up in the PCI tree?
>
> Applied this with Jakub's tested-by and Doug's ack to pci/virtualization
> for v4.12.
>
> This still leaves these:
>
>   [PATCH 4/7] ixgbe: use pcie_flr instead of duplicating it
>   [PATCH 6/7] crypto: qat: use pcie_flr instead of duplicating it
>   [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it
>
> I haven't seen any response to 4 and 6.  Felix reported an unused
> variable in 7.  Let me know if you'd like me to do anything with
> these.

Just provided my ACK for ixgbe patch.


Attachments:
signature.asc (819.00 B)
This is a digitally signed message part

2017-04-27 06:48:10

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Tue, Apr 25, 2017 at 02:39:55PM -0500, Bjorn Helgaas wrote:
> This still leaves these:
>
> [PATCH 4/7] ixgbe: use pcie_flr instead of duplicating it
> [PATCH 6/7] crypto: qat: use pcie_flr instead of duplicating it
> [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it
>
> I haven't seen any response to 4 and 6. Felix reported an unused
> variable in 7. Let me know if you'd like me to do anything with
> these.

Now that Jeff ACKed 4 it might be worth to add it to the pci tree last
minute. I'll resend liquidio and qat to the respective maintainers for
the next merge window.

2017-04-27 16:50:05

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

On Thu, Apr 27, 2017 at 08:47:58AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 25, 2017 at 02:39:55PM -0500, Bjorn Helgaas wrote:
> > This still leaves these:
> >
> > [PATCH 4/7] ixgbe: use pcie_flr instead of duplicating it
> > [PATCH 6/7] crypto: qat: use pcie_flr instead of duplicating it
> > [PATCH 7/7] liquidio: use pcie_flr instead of duplicating it
> >
> > I haven't seen any response to 4 and 6. Felix reported an unused
> > variable in 7. Let me know if you'd like me to do anything with
> > these.
>
> Now that Jeff ACKed 4 it might be worth to add it to the pci tree last
> minute. I'll resend liquidio and qat to the respective maintainers for
> the next merge window.

I applied 4 with Jeff's ack to pci/virtualization for v4.12, thanks!