2020-02-29 22:22:46

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 00/10] PCI: Add and use constant PCI_STATUS_ERROR_BITS and helper pci_status_get_and_clear_errors

Several drivers have own definitions for this constant, so move it
to the PCI core. In addition in multiple places the following code
sequence is used:
1. Read PCI_STATUS
2. Mask out non-error bits
3. Action based on set error bits
4. Write back set error bits to clear them

As this is a repeated pattern, add a helper to the PCI core.

Most affected drivers are network drivers. But as it's about core
PCI functionality, I suppose the series should go through the PCI
tree.

v2:
- fix formal issue with cover letter
v3:
- fix dumb typo in patch 7
v4:
- add patches 1-3
- move new constant PCI_STATUS_ERROR_BITS to include/linux/pci.h
- small improvements in commit messages

Heiner Kallweit (10):
net: marvell: add PCI_STATUS_SIG_TARGET_ABORT to PCI status error bits
net: skfp: add PCI_STATUS_REC_TARGET_ABORT to PCI status error bits
r8169: add PCI_STATUS_PARITY to PCI status error bits
PCI: Add constant PCI_STATUS_ERROR_BITS
PCI: Add pci_status_get_and_clear_errors
r8169: use pci_status_get_and_clear_errors
net: sun: use pci_status_get_and_clear_errors
net: skfp: use new constant PCI_STATUS_ERROR_BITS
PCI: pci-bridge-emul: Use new constant PCI_STATUS_ERROR_BITS
sound: bt87x: use pci_status_get_and_clear_errors

drivers/net/ethernet/marvell/skge.h | 6 -----
drivers/net/ethernet/marvell/sky2.h | 6 -----
drivers/net/ethernet/realtek/r8169_main.c | 15 +++++-------
drivers/net/ethernet/sun/cassini.c | 28 ++++++++-------------
drivers/net/ethernet/sun/sungem.c | 30 +++++++----------------
drivers/net/fddi/skfp/drvfbi.c | 4 +--
drivers/net/fddi/skfp/h/skfbi.h | 5 ----
drivers/pci/pci-bridge-emul.c | 14 ++---------
drivers/pci/pci.c | 23 +++++++++++++++++
include/linux/pci.h | 8 ++++++
sound/pci/bt87x.c | 7 +-----
11 files changed, 61 insertions(+), 85 deletions(-)

--
2.25.1


2020-02-29 22:30:04

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 06/10] r8169: use pci_status_get_and_clear_errors

Use new helper pci_status_get_and_clear_errors() to simplify the code.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/ethernet/realtek/r8169_main.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 7c9892a16..4495a3cf9 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4357,13 +4357,15 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
struct pci_dev *pdev = tp->pci_dev;
- u16 pci_status, pci_cmd;
+ int pci_status_errs;
+ u16 pci_cmd;

pci_read_config_word(pdev, PCI_COMMAND, &pci_cmd);
- pci_read_config_word(pdev, PCI_STATUS, &pci_status);

- netif_err(tp, intr, dev, "PCI error (cmd = 0x%04x, status = 0x%04x)\n",
- pci_cmd, pci_status);
+ pci_status_errs = pci_status_get_and_clear_errors(pdev);
+
+ netif_err(tp, intr, dev, "PCI error (cmd = 0x%04x, status_errs = 0x%04x)\n",
+ pci_cmd, pci_status_errs);

/*
* The recovery sequence below admits a very elaborated explanation:
@@ -4380,11 +4382,6 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)

pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);

- pci_write_config_word(pdev, PCI_STATUS,
- pci_status & (PCI_STATUS_DETECTED_PARITY | PCI_STATUS_PARITY |
- PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT));
-
rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING);
}

--
2.25.1


2020-02-29 22:30:04

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 08/10] net: skfp: use new constant PCI_STATUS_ERROR_BITS

Use new PCI core constant PCI_STATUS_ERROR_BITS to simplify the code.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/fddi/skfp/drvfbi.c | 4 ++--
drivers/net/fddi/skfp/h/skfbi.h | 5 -----
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fddi/skfp/drvfbi.c b/drivers/net/fddi/skfp/drvfbi.c
index 9c8aa3a95..cc9ac5724 100644
--- a/drivers/net/fddi/skfp/drvfbi.c
+++ b/drivers/net/fddi/skfp/drvfbi.c
@@ -20,7 +20,7 @@
#include "h/supern_2.h"
#include "h/skfbiinc.h"
#include <linux/bitrev.h>
-#include <linux/pci_regs.h>
+#include <linux/pci.h>

#ifndef lint
static const char ID_sccs[] = "@(#)drvfbi.c 1.63 99/02/11 (C) SK " ;
@@ -112,7 +112,7 @@ static void card_start(struct s_smc *smc)
*/
outp(ADDR(B0_TST_CTRL), TST_CFG_WRITE_ON) ; /* enable for writes */
word = inpw(PCI_C(PCI_STATUS)) ;
- outpw(PCI_C(PCI_STATUS), word | PCI_ERRBITS) ;
+ outpw(PCI_C(PCI_STATUS), word | PCI_STATUS_ERROR_BITS);
outp(ADDR(B0_TST_CTRL), TST_CFG_WRITE_OFF) ; /* disable writes */

/*
diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
index 36e20a514..ccee00b71 100644
--- a/drivers/net/fddi/skfp/h/skfbi.h
+++ b/drivers/net/fddi/skfp/h/skfbi.h
@@ -33,11 +33,6 @@
*/
#define I2C_ADDR_VPD 0xA0 /* I2C address for the VPD EEPROM */

-
-#define PCI_ERRBITS (PCI_STATUS_DETECTED_PARITY | PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT | PCI_STATUS_PARITY)
-
-
-
/*
* Control Register File:
* Bank 0
--
2.25.1


2020-02-29 22:30:15

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 10/10] sound: bt87x: use pci_status_get_and_clear_errors

Use new helper pci_status_get_and_clear_errors() to simplify the code.

Signed-off-by: Heiner Kallweit <[email protected]>
---
sound/pci/bt87x.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c
index 8c48864c8..656750466 100644
--- a/sound/pci/bt87x.c
+++ b/sound/pci/bt87x.c
@@ -271,13 +271,8 @@ static void snd_bt87x_free_risc(struct snd_bt87x *chip)

static void snd_bt87x_pci_error(struct snd_bt87x *chip, unsigned int status)
{
- u16 pci_status;
+ int pci_status = pci_status_get_and_clear_errors(chip->pci);

- pci_read_config_word(chip->pci, PCI_STATUS, &pci_status);
- pci_status &= PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
- PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY;
- pci_write_config_word(chip->pci, PCI_STATUS, pci_status);
if (pci_status != PCI_STATUS_DETECTED_PARITY)
dev_err(chip->card->dev,
"Aieee - PCI error! status %#08x, PCI status %#04x\n",
--
2.25.1


2020-02-29 22:30:21

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 07/10] net: sun: use pci_status_get_and_clear_errors

Use new helper pci_status_get_and_clear_errors() to simplify the code.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/ethernet/sun/cassini.c | 28 ++++++++++------------------
drivers/net/ethernet/sun/sungem.c | 30 +++++++++---------------------
2 files changed, 19 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 6ec9163e2..e6d1aa882 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1716,34 +1716,26 @@ static int cas_pci_interrupt(struct net_device *dev, struct cas *cp,
pr_cont("\n");

if (stat & PCI_ERR_OTHER) {
- u16 cfg;
+ int pci_errs;

/* Interrogate PCI config space for the
* true cause.
*/
- pci_read_config_word(cp->pdev, PCI_STATUS, &cfg);
- netdev_err(dev, "Read PCI cfg space status [%04x]\n", cfg);
- if (cfg & PCI_STATUS_PARITY)
+ pci_errs = pci_status_get_and_clear_errors(cp->pdev);
+
+ netdev_err(dev, "PCI status errors[%04x]\n", pci_errs);
+ if (pci_errs & PCI_STATUS_PARITY)
netdev_err(dev, "PCI parity error detected\n");
- if (cfg & PCI_STATUS_SIG_TARGET_ABORT)
+ if (pci_errs & PCI_STATUS_SIG_TARGET_ABORT)
netdev_err(dev, "PCI target abort\n");
- if (cfg & PCI_STATUS_REC_TARGET_ABORT)
+ if (pci_errs & PCI_STATUS_REC_TARGET_ABORT)
netdev_err(dev, "PCI master acks target abort\n");
- if (cfg & PCI_STATUS_REC_MASTER_ABORT)
+ if (pci_errs & PCI_STATUS_REC_MASTER_ABORT)
netdev_err(dev, "PCI master abort\n");
- if (cfg & PCI_STATUS_SIG_SYSTEM_ERROR)
+ if (pci_errs & PCI_STATUS_SIG_SYSTEM_ERROR)
netdev_err(dev, "PCI system error SERR#\n");
- if (cfg & PCI_STATUS_DETECTED_PARITY)
+ if (pci_errs & PCI_STATUS_DETECTED_PARITY)
netdev_err(dev, "PCI parity error\n");
-
- /* Write the error bits back to clear them. */
- cfg &= (PCI_STATUS_PARITY |
- PCI_STATUS_SIG_TARGET_ABORT |
- PCI_STATUS_REC_TARGET_ABORT |
- PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_SIG_SYSTEM_ERROR |
- PCI_STATUS_DETECTED_PARITY);
- pci_write_config_word(cp->pdev, PCI_STATUS, cfg);
}

/* For all PCI errors, we should reset the chip. */
diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index 8358064fb..2d392a7b1 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -545,37 +545,25 @@ static int gem_pci_interrupt(struct net_device *dev, struct gem *gp, u32 gem_sta
}

if (pci_estat & GREG_PCIESTAT_OTHER) {
- u16 pci_cfg_stat;
+ int pci_errs;

/* Interrogate PCI config space for the
* true cause.
*/
- pci_read_config_word(gp->pdev, PCI_STATUS,
- &pci_cfg_stat);
- netdev_err(dev, "Read PCI cfg space status [%04x]\n",
- pci_cfg_stat);
- if (pci_cfg_stat & PCI_STATUS_PARITY)
+ pci_errs = pci_status_get_and_clear_errors(gp->pdev);
+ netdev_err(dev, "PCI status errors[%04x]\n", pci_errs);
+ if (pci_errs & PCI_STATUS_PARITY)
netdev_err(dev, "PCI parity error detected\n");
- if (pci_cfg_stat & PCI_STATUS_SIG_TARGET_ABORT)
+ if (pci_errs & PCI_STATUS_SIG_TARGET_ABORT)
netdev_err(dev, "PCI target abort\n");
- if (pci_cfg_stat & PCI_STATUS_REC_TARGET_ABORT)
+ if (pci_errs & PCI_STATUS_REC_TARGET_ABORT)
netdev_err(dev, "PCI master acks target abort\n");
- if (pci_cfg_stat & PCI_STATUS_REC_MASTER_ABORT)
+ if (pci_errs & PCI_STATUS_REC_MASTER_ABORT)
netdev_err(dev, "PCI master abort\n");
- if (pci_cfg_stat & PCI_STATUS_SIG_SYSTEM_ERROR)
+ if (pci_errs & PCI_STATUS_SIG_SYSTEM_ERROR)
netdev_err(dev, "PCI system error SERR#\n");
- if (pci_cfg_stat & PCI_STATUS_DETECTED_PARITY)
+ if (pci_errs & PCI_STATUS_DETECTED_PARITY)
netdev_err(dev, "PCI parity error\n");
-
- /* Write the error bits back to clear them. */
- pci_cfg_stat &= (PCI_STATUS_PARITY |
- PCI_STATUS_SIG_TARGET_ABORT |
- PCI_STATUS_REC_TARGET_ABORT |
- PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_SIG_SYSTEM_ERROR |
- PCI_STATUS_DETECTED_PARITY);
- pci_write_config_word(gp->pdev,
- PCI_STATUS, pci_cfg_stat);
}

/* For all PCI errors, we should reset the chip. */
--
2.25.1


2020-02-29 22:30:27

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 09/10] PCI: pci-bridge-emul: Use new constant PCI_STATUS_ERROR_BITS

Use new constant PCI_STATUS_ERROR_BITS to simplify the code.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/pci/pci-bridge-emul.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index fffa77093..4f4f54bc7 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -50,12 +50,7 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
(PCI_STATUS_CAP_LIST | PCI_STATUS_66MHZ |
PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MASK) << 16),
.rsvd = GENMASK(15, 10) | ((BIT(6) | GENMASK(3, 0)) << 16),
- .w1c = (PCI_STATUS_PARITY |
- PCI_STATUS_SIG_TARGET_ABORT |
- PCI_STATUS_REC_TARGET_ABORT |
- PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_SIG_SYSTEM_ERROR |
- PCI_STATUS_DETECTED_PARITY) << 16,
+ .w1c = PCI_STATUS_ERROR_BITS << 16,
},
[PCI_CLASS_REVISION / 4] = { .ro = ~0 },

@@ -100,12 +95,7 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
PCI_STATUS_DEVSEL_MASK) << 16) |
GENMASK(11, 8) | GENMASK(3, 0)),

- .w1c = (PCI_STATUS_PARITY |
- PCI_STATUS_SIG_TARGET_ABORT |
- PCI_STATUS_REC_TARGET_ABORT |
- PCI_STATUS_REC_MASTER_ABORT |
- PCI_STATUS_SIG_SYSTEM_ERROR |
- PCI_STATUS_DETECTED_PARITY) << 16,
+ .w1c = PCI_STATUS_ERROR_BITS << 16,

.rsvd = ((BIT(6) | GENMASK(4, 0)) << 16),
},
--
2.25.1


2020-02-29 22:30:49

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 05/10] PCI: Add pci_status_get_and_clear_errors

Several drivers use the following code sequence:
1. Read PCI_STATUS
2. Mask out non-error bits
3. Action based on error bits set
4. Write back set error bits to clear them

As this is a repeated pattern, add a helper to the PCI core.

Signed-off-by: Heiner Kallweit <[email protected]>
---
v4:
- improve commit description
---
drivers/pci/pci.c | 23 +++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 24 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d828ca835..c16b0ba2a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -173,6 +173,29 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus)
}
EXPORT_SYMBOL_GPL(pci_bus_max_busnr);

+/**
+ * pci_status_get_and_clear_errors - return and clear error bits in PCI_STATUS
+ * @pdev: the PCI device
+ *
+ * Returns error bits set in PCI_STATUS and clears them.
+ */
+int pci_status_get_and_clear_errors(struct pci_dev *pdev)
+{
+ u16 status;
+ int ret;
+
+ ret = pci_read_config_word(pdev, PCI_STATUS, &status);
+ if (ret != PCIBIOS_SUCCESSFUL)
+ return -EIO;
+
+ status &= PCI_STATUS_ERROR_BITS;
+ if (status)
+ pci_write_config_word(pdev, PCI_STATUS, status);
+
+ return status;
+}
+EXPORT_SYMBOL_GPL(pci_status_get_and_clear_errors);
+
#ifdef CONFIG_HAS_IOMEM
void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
{
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 101d71e0a..7beaf51e9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1210,6 +1210,7 @@ int pci_select_bars(struct pci_dev *dev, unsigned long flags);
bool pci_device_is_present(struct pci_dev *pdev);
void pci_ignore_hotplug(struct pci_dev *dev);
struct pci_dev *pci_real_dma_dev(struct pci_dev *dev);
+int pci_status_get_and_clear_errors(struct pci_dev *pdev);

int __printf(6, 7) pci_request_irq(struct pci_dev *dev, unsigned int nr,
irq_handler_t handler, irq_handler_t thread_fn, void *dev_id,
--
2.25.1


2020-02-29 22:31:12

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 02/10] net: skfp: add PCI_STATUS_REC_TARGET_ABORT to PCI status error bits

In preparation of factoring out PCI_STATUS error bit handling let drivers
use the same collection of error bits. To facilitate bisecting we do this
in a separate patch per affected driver. For the skfp driver we have to
add PCI_STATUS_REC_TARGET_ABORT to the error bits.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/fddi/skfp/h/skfbi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
index 480795681..36e20a514 100644
--- a/drivers/net/fddi/skfp/h/skfbi.h
+++ b/drivers/net/fddi/skfp/h/skfbi.h
@@ -34,7 +34,7 @@
#define I2C_ADDR_VPD 0xA0 /* I2C address for the VPD EEPROM */


-#define PCI_ERRBITS (PCI_STATUS_DETECTED_PARITY | PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_SIG_TARGET_ABORT | PCI_STATUS_PARITY)
+#define PCI_ERRBITS (PCI_STATUS_DETECTED_PARITY | PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT | PCI_STATUS_PARITY)



--
2.25.1


2020-02-29 22:31:21

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 04/10] PCI: Add constant PCI_STATUS_ERROR_BITS

This collection of PCI error bits is used in more than one driver,
so move it to the PCI core.

Signed-off-by: Heiner Kallweit <[email protected]>
---
v4:
- move new constant to include/linux/pci.h
- improve commit description
---
drivers/net/ethernet/marvell/skge.h | 7 -------
drivers/net/ethernet/marvell/sky2.h | 7 -------
include/linux/pci.h | 7 +++++++
3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.h b/drivers/net/ethernet/marvell/skge.h
index e76c03c87..e149bdfe1 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -15,13 +15,6 @@
#define PCI_VPD_ROM_SZ 7L<<14 /* VPD ROM size 0=256, 1=512, ... */
#define PCI_REV_DESC 1<<2 /* Reverse Descriptor bytes */

-#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
- PCI_STATUS_SIG_SYSTEM_ERROR | \
- PCI_STATUS_REC_MASTER_ABORT | \
- PCI_STATUS_REC_TARGET_ABORT | \
- PCI_STATUS_SIG_TARGET_ABORT | \
- PCI_STATUS_PARITY)
-
enum csr_regs {
B0_RAP = 0x0000,
B0_CTST = 0x0004,
diff --git a/drivers/net/ethernet/marvell/sky2.h b/drivers/net/ethernet/marvell/sky2.h
index aee87f838..851d8ed34 100644
--- a/drivers/net/ethernet/marvell/sky2.h
+++ b/drivers/net/ethernet/marvell/sky2.h
@@ -252,13 +252,6 @@ enum {
};


-#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
- PCI_STATUS_SIG_SYSTEM_ERROR | \
- PCI_STATUS_REC_MASTER_ABORT | \
- PCI_STATUS_REC_TARGET_ABORT | \
- PCI_STATUS_SIG_TARGET_ABORT | \
- PCI_STATUS_PARITY)
-
enum csr_regs {
B0_RAP = 0x0000,
B0_CTST = 0x0004,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3840a541a..101d71e0a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -42,6 +42,13 @@

#include <linux/pci_ids.h>

+#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
+ PCI_STATUS_SIG_SYSTEM_ERROR | \
+ PCI_STATUS_REC_MASTER_ABORT | \
+ PCI_STATUS_REC_TARGET_ABORT | \
+ PCI_STATUS_SIG_TARGET_ABORT | \
+ PCI_STATUS_PARITY)
+
/*
* The PCI interface treats multi-function devices as independent
* devices. The slot/function address of each device is encoded
--
2.25.1


2020-02-29 22:31:45

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 01/10] net: marvell: add PCI_STATUS_SIG_TARGET_ABORT to PCI status error bits

In preparation of factoring out PCI_STATUS error bit handling let drivers
use the same collection of error bits. To facilitate bisecting we do this
in a separate patch per affected driver. For the Marvell drivers we have
to add PCI_STATUS_SIG_TARGET_ABORT to the error bits.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/ethernet/marvell/skge.h | 1 +
drivers/net/ethernet/marvell/sky2.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/skge.h b/drivers/net/ethernet/marvell/skge.h
index 6fa7b6a34..e76c03c87 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -19,6 +19,7 @@
PCI_STATUS_SIG_SYSTEM_ERROR | \
PCI_STATUS_REC_MASTER_ABORT | \
PCI_STATUS_REC_TARGET_ABORT | \
+ PCI_STATUS_SIG_TARGET_ABORT | \
PCI_STATUS_PARITY)

enum csr_regs {
diff --git a/drivers/net/ethernet/marvell/sky2.h b/drivers/net/ethernet/marvell/sky2.h
index b02b65230..aee87f838 100644
--- a/drivers/net/ethernet/marvell/sky2.h
+++ b/drivers/net/ethernet/marvell/sky2.h
@@ -256,6 +256,7 @@ enum {
PCI_STATUS_SIG_SYSTEM_ERROR | \
PCI_STATUS_REC_MASTER_ABORT | \
PCI_STATUS_REC_TARGET_ABORT | \
+ PCI_STATUS_SIG_TARGET_ABORT | \
PCI_STATUS_PARITY)

enum csr_regs {
--
2.25.1


2020-02-29 22:31:59

by Heiner Kallweit

[permalink] [raw]
Subject: [PATCH v4 03/10] r8169: add PCI_STATUS_PARITY to PCI status error bits

In preparation of factoring out PCI_STATUS error bit handling let drivers
use the same collection of error bits. To facilitate bisecting we do this
in a separate patch per affected driver. For the r8169 driver we have to
add PCI_STATUS_PARITY to the error bits.

Signed-off-by: Heiner Kallweit <[email protected]>
---
drivers/net/ethernet/realtek/r8169_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index f081007a2..7c9892a16 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4381,7 +4381,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);

pci_write_config_word(pdev, PCI_STATUS,
- pci_status & (PCI_STATUS_DETECTED_PARITY |
+ pci_status & (PCI_STATUS_DETECTED_PARITY | PCI_STATUS_PARITY |
PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_REC_MASTER_ABORT |
PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT));

--
2.25.1


2020-03-04 03:01:11

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v4 00/10] PCI: Add and use constant PCI_STATUS_ERROR_BITS and helper pci_status_get_and_clear_errors


Bjorn, please review and let me know if it is OK to merge this via the
networking tree.

Thank you.

2020-03-04 13:42:23

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 04/10] PCI: Add constant PCI_STATUS_ERROR_BITS

On Sat, Feb 29, 2020 at 11:23:44PM +0100, Heiner Kallweit wrote:
> This collection of PCI error bits is used in more than one driver,
> so move it to the PCI core.
>
> Signed-off-by: Heiner Kallweit <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> v4:
> - move new constant to include/linux/pci.h
> - improve commit description
> ---
> drivers/net/ethernet/marvell/skge.h | 7 -------
> drivers/net/ethernet/marvell/sky2.h | 7 -------
> include/linux/pci.h | 7 +++++++
> 3 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/skge.h b/drivers/net/ethernet/marvell/skge.h
> index e76c03c87..e149bdfe1 100644
> --- a/drivers/net/ethernet/marvell/skge.h
> +++ b/drivers/net/ethernet/marvell/skge.h
> @@ -15,13 +15,6 @@
> #define PCI_VPD_ROM_SZ 7L<<14 /* VPD ROM size 0=256, 1=512, ... */
> #define PCI_REV_DESC 1<<2 /* Reverse Descriptor bytes */
>
> -#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
> - PCI_STATUS_SIG_SYSTEM_ERROR | \
> - PCI_STATUS_REC_MASTER_ABORT | \
> - PCI_STATUS_REC_TARGET_ABORT | \
> - PCI_STATUS_SIG_TARGET_ABORT | \
> - PCI_STATUS_PARITY)
> -
> enum csr_regs {
> B0_RAP = 0x0000,
> B0_CTST = 0x0004,
> diff --git a/drivers/net/ethernet/marvell/sky2.h b/drivers/net/ethernet/marvell/sky2.h
> index aee87f838..851d8ed34 100644
> --- a/drivers/net/ethernet/marvell/sky2.h
> +++ b/drivers/net/ethernet/marvell/sky2.h
> @@ -252,13 +252,6 @@ enum {
> };
>
>
> -#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
> - PCI_STATUS_SIG_SYSTEM_ERROR | \
> - PCI_STATUS_REC_MASTER_ABORT | \
> - PCI_STATUS_REC_TARGET_ABORT | \
> - PCI_STATUS_SIG_TARGET_ABORT | \
> - PCI_STATUS_PARITY)
> -
> enum csr_regs {
> B0_RAP = 0x0000,
> B0_CTST = 0x0004,
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3840a541a..101d71e0a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -42,6 +42,13 @@
>
> #include <linux/pci_ids.h>
>
> +#define PCI_STATUS_ERROR_BITS (PCI_STATUS_DETECTED_PARITY | \
> + PCI_STATUS_SIG_SYSTEM_ERROR | \
> + PCI_STATUS_REC_MASTER_ABORT | \
> + PCI_STATUS_REC_TARGET_ABORT | \
> + PCI_STATUS_SIG_TARGET_ABORT | \
> + PCI_STATUS_PARITY)
> +
> /*
> * The PCI interface treats multi-function devices as independent
> * devices. The slot/function address of each device is encoded
> --
> 2.25.1
>
>

2020-03-04 13:43:40

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 05/10] PCI: Add pci_status_get_and_clear_errors

On Sat, Feb 29, 2020 at 11:24:23PM +0100, Heiner Kallweit wrote:
> Several drivers use the following code sequence:
> 1. Read PCI_STATUS
> 2. Mask out non-error bits
> 3. Action based on error bits set
> 4. Write back set error bits to clear them
>
> As this is a repeated pattern, add a helper to the PCI core.
>
> Signed-off-by: Heiner Kallweit <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

Nit: if you have any reason to revise this, I would prefer
"pci_status_get_and_clear_errors()" (with parens) in the subject.

> ---
> v4:
> - improve commit description
> ---
> drivers/pci/pci.c | 23 +++++++++++++++++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d828ca835..c16b0ba2a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -173,6 +173,29 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus)
> }
> EXPORT_SYMBOL_GPL(pci_bus_max_busnr);
>
> +/**
> + * pci_status_get_and_clear_errors - return and clear error bits in PCI_STATUS
> + * @pdev: the PCI device
> + *
> + * Returns error bits set in PCI_STATUS and clears them.
> + */
> +int pci_status_get_and_clear_errors(struct pci_dev *pdev)
> +{
> + u16 status;
> + int ret;
> +
> + ret = pci_read_config_word(pdev, PCI_STATUS, &status);
> + if (ret != PCIBIOS_SUCCESSFUL)
> + return -EIO;
> +
> + status &= PCI_STATUS_ERROR_BITS;
> + if (status)
> + pci_write_config_word(pdev, PCI_STATUS, status);
> +
> + return status;
> +}
> +EXPORT_SYMBOL_GPL(pci_status_get_and_clear_errors);
> +
> #ifdef CONFIG_HAS_IOMEM
> void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 101d71e0a..7beaf51e9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1210,6 +1210,7 @@ int pci_select_bars(struct pci_dev *dev, unsigned long flags);
> bool pci_device_is_present(struct pci_dev *pdev);
> void pci_ignore_hotplug(struct pci_dev *dev);
> struct pci_dev *pci_real_dma_dev(struct pci_dev *dev);
> +int pci_status_get_and_clear_errors(struct pci_dev *pdev);
>
> int __printf(6, 7) pci_request_irq(struct pci_dev *dev, unsigned int nr,
> irq_handler_t handler, irq_handler_t thread_fn, void *dev_id,
> --
> 2.25.1
>
>

2020-03-04 13:45:03

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 09/10] PCI: pci-bridge-emul: Use new constant PCI_STATUS_ERROR_BITS

On Sat, Feb 29, 2020 at 11:28:18PM +0100, Heiner Kallweit wrote:
> Use new constant PCI_STATUS_ERROR_BITS to simplify the code.
>
> Signed-off-by: Heiner Kallweit <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/pci-bridge-emul.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> index fffa77093..4f4f54bc7 100644
> --- a/drivers/pci/pci-bridge-emul.c
> +++ b/drivers/pci/pci-bridge-emul.c
> @@ -50,12 +50,7 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
> (PCI_STATUS_CAP_LIST | PCI_STATUS_66MHZ |
> PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MASK) << 16),
> .rsvd = GENMASK(15, 10) | ((BIT(6) | GENMASK(3, 0)) << 16),
> - .w1c = (PCI_STATUS_PARITY |
> - PCI_STATUS_SIG_TARGET_ABORT |
> - PCI_STATUS_REC_TARGET_ABORT |
> - PCI_STATUS_REC_MASTER_ABORT |
> - PCI_STATUS_SIG_SYSTEM_ERROR |
> - PCI_STATUS_DETECTED_PARITY) << 16,
> + .w1c = PCI_STATUS_ERROR_BITS << 16,
> },
> [PCI_CLASS_REVISION / 4] = { .ro = ~0 },
>
> @@ -100,12 +95,7 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
> PCI_STATUS_DEVSEL_MASK) << 16) |
> GENMASK(11, 8) | GENMASK(3, 0)),
>
> - .w1c = (PCI_STATUS_PARITY |
> - PCI_STATUS_SIG_TARGET_ABORT |
> - PCI_STATUS_REC_TARGET_ABORT |
> - PCI_STATUS_REC_MASTER_ABORT |
> - PCI_STATUS_SIG_SYSTEM_ERROR |
> - PCI_STATUS_DETECTED_PARITY) << 16,
> + .w1c = PCI_STATUS_ERROR_BITS << 16,
>
> .rsvd = ((BIT(6) | GENMASK(4, 0)) << 16),
> },
> --
> 2.25.1
>
>

2020-03-04 16:59:29

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 00/10] PCI: Add and use constant PCI_STATUS_ERROR_BITS and helper pci_status_get_and_clear_errors

On Tue, Mar 03, 2020 at 06:59:37PM -0800, David Miller wrote:
>
> Bjorn, please review and let me know if it is OK to merge this via the
> networking tree.

I acked the relevant patches and you're welcome to merge it via
networking. Thanks!

2020-03-04 22:21:59

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v4 00/10] PCI: Add and use constant PCI_STATUS_ERROR_BITS and helper pci_status_get_and_clear_errors

From: Bjorn Helgaas <[email protected]>
Date: Wed, 4 Mar 2020 10:59:04 -0600

> On Tue, Mar 03, 2020 at 06:59:37PM -0800, David Miller wrote:
>>
>> Bjorn, please review and let me know if it is OK to merge this via the
>> networking tree.
>
> I acked the relevant patches and you're welcome to merge it via
> networking. Thanks!

Ok, thanks.

Series applied to net-next.

Thanks again everyone.

2020-03-06 06:07:56

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH v4 10/10] sound: bt87x: use pci_status_get_and_clear_errors

On Sat, 29 Feb 2020 23:29:07 +0100,
Heiner Kallweit wrote:
>
> Use new helper pci_status_get_and_clear_errors() to simplify the code.
>
> Signed-off-by: Heiner Kallweit <[email protected]>

Acked-by: Takashi Iwai <[email protected]>


thanks,

Takashi

> ---
> sound/pci/bt87x.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c
> index 8c48864c8..656750466 100644
> --- a/sound/pci/bt87x.c
> +++ b/sound/pci/bt87x.c
> @@ -271,13 +271,8 @@ static void snd_bt87x_free_risc(struct snd_bt87x *chip)
>
> static void snd_bt87x_pci_error(struct snd_bt87x *chip, unsigned int status)
> {
> - u16 pci_status;
> + int pci_status = pci_status_get_and_clear_errors(chip->pci);
>
> - pci_read_config_word(chip->pci, PCI_STATUS, &pci_status);
> - pci_status &= PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
> - PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
> - PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY;
> - pci_write_config_word(chip->pci, PCI_STATUS, pci_status);
> if (pci_status != PCI_STATUS_DETECTED_PARITY)
> dev_err(chip->card->dev,
> "Aieee - PCI error! status %#08x, PCI status %#04x\n",
> --
> 2.25.1
>
>