* [PATCH v4 1/3] PCI: Add pci_disable_parity()
2021-03-30 17:43 [PATCH v4 0/3] PCI: Disable parity checking Bjorn Helgaas
@ 2021-03-30 17:43 ` Bjorn Helgaas
2021-03-30 17:43 ` [PATCH v4 2/3] IB/mthca: Disable parity reporting Bjorn Helgaas
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2021-03-30 17:43 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Leon Romanovsky, Russell King - ARM Linux, David Miller,
Jakub Kicinski, nic_swsd, linux-pci, linux-arm-kernel, netdev,
Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
Add pci_disable_parity() to disable reporting of parity errors for a
device by clearing PCI_COMMAND_PARITY.
The device will still set PCI_STATUS_DETECTED_PARITY when it detects
a parity error or receives a Poisoned TLP, but it will not set
PCI_STATUS_PARITY, which means it will not assert PERR#
(conventional PCI) or report Poisoned TLPs (PCIe).
Based-on: https://lore.kernel.org/linux-arm-kernel/d375987c-ea4f-dd98-4ef8-99b2fbfe7c33@gmail.com/
Based-on-patch-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pci.c | 17 +++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 16a17215f633..b1845e5e5c8f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4453,6 +4453,23 @@ void pci_clear_mwi(struct pci_dev *dev)
}
EXPORT_SYMBOL(pci_clear_mwi);
+/**
+ * pci_disable_parity - disable parity checking for device
+ * @dev: the PCI device to operate on
+ *
+ * Disable parity checking for device @dev
+ */
+void pci_disable_parity(struct pci_dev *dev)
+{
+ u16 cmd;
+
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ if (cmd & PCI_COMMAND_PARITY) {
+ cmd &= ~PCI_COMMAND_PARITY;
+ pci_write_config_word(dev, PCI_COMMAND, cmd);
+ }
+}
+
/**
* pci_intx - enables/disables PCI INTx for device dev
* @pdev: the PCI device to operate on
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..4eaa773115da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1201,6 +1201,7 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
int __must_check pcim_set_mwi(struct pci_dev *dev);
int pci_try_set_mwi(struct pci_dev *dev);
void pci_clear_mwi(struct pci_dev *dev);
+void pci_disable_parity(struct pci_dev *dev);
void pci_intx(struct pci_dev *dev, int enable);
bool pci_check_and_mask_intx(struct pci_dev *dev);
bool pci_check_and_unmask_intx(struct pci_dev *dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 2/3] IB/mthca: Disable parity reporting
2021-03-30 17:43 [PATCH v4 0/3] PCI: Disable parity checking Bjorn Helgaas
2021-03-30 17:43 ` [PATCH v4 1/3] PCI: Add pci_disable_parity() Bjorn Helgaas
@ 2021-03-30 17:43 ` Bjorn Helgaas
2021-03-30 17:43 ` [PATCH v4 3/3] ARM: iop32x: disable N2100 PCI " Bjorn Helgaas
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2021-03-30 17:43 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Leon Romanovsky, Russell King - ARM Linux, David Miller,
Jakub Kicinski, nic_swsd, linux-pci, linux-arm-kernel, netdev
From: Heiner Kallweit <hkallweit1@gmail.com>
For Mellanox Tavor devices, we previously set dev->broken_parity_status,
which does not change the device's behavior; it merely prevents the EDAC
PCI error reporting from warning about Master Data Parity Error, Signaled
System Error, or Detected Parity Error for this device.
Instead, disable Parity Error Response so the device doesn't report
parity errors in the first place.
[bhelgaas: split out pci_disable_parity(), commit log, keep quirk static]
Link: https://lore.kernel.org/r/d375987c-ea4f-dd98-4ef8-99b2fbfe7c33@gmail.com
---
drivers/pci/quirks.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..6aa9df411604 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -206,16 +206,11 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
/*
- * The Mellanox Tavor device gives false positive parity errors. Mark this
- * device with a broken_parity_status to allow PCI scanning code to "skip"
- * this now blacklisted device.
+ * The Mellanox Tavor device gives false positive parity errors. Disable
+ * parity error reporting.
*/
-static void quirk_mellanox_tavor(struct pci_dev *dev)
-{
- dev->broken_parity_status = 1; /* This device gives false positives */
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR, quirk_mellanox_tavor);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, quirk_mellanox_tavor);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR, pci_disable_parity);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_disable_parity);
/*
* Deal with broken BIOSes that neglect to enable passive release,
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 3/3] ARM: iop32x: disable N2100 PCI parity reporting
2021-03-30 17:43 [PATCH v4 0/3] PCI: Disable parity checking Bjorn Helgaas
2021-03-30 17:43 ` [PATCH v4 1/3] PCI: Add pci_disable_parity() Bjorn Helgaas
2021-03-30 17:43 ` [PATCH v4 2/3] IB/mthca: Disable parity reporting Bjorn Helgaas
@ 2021-03-30 17:43 ` Bjorn Helgaas
2021-03-31 11:08 ` [PATCH v4 0/3] PCI: Disable parity checking Heiner Kallweit
2021-03-31 11:26 ` Krzysztof Wilczyński
4 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2021-03-30 17:43 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Leon Romanovsky, Russell King - ARM Linux, David Miller,
Jakub Kicinski, nic_swsd, linux-pci, linux-arm-kernel, netdev
From: Heiner Kallweit <hkallweit1@gmail.com>
On the N2100, instead of just marking the r8169 chips as having
broken_parity_status, disable parity error reporting for them entirely.
This was the only relevant place that set broken_parity_status, so we no
longer need to check for it in the r8169 error interrupt handler.
[bhelgaas: squash into one patch, commit log]
Link: https://lore.kernel.org/r/0c0dcbf2-5f1e-954c-ebd7-e6ccfae5c60e@gmail.com
Link: https://lore.kernel.org/r/9e312679-a684-e9c7-2656-420723706451@gmail.com
---
arch/arm/mach-iop32x/n2100.c | 8 ++++----
drivers/net/ethernet/realtek/r8169_main.c | 14 --------------
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-iop32x/n2100.c b/arch/arm/mach-iop32x/n2100.c
index 78b9a5ee41c9..bf99e718f8b8 100644
--- a/arch/arm/mach-iop32x/n2100.c
+++ b/arch/arm/mach-iop32x/n2100.c
@@ -116,16 +116,16 @@ static struct hw_pci n2100_pci __initdata = {
};
/*
- * Both r8169 chips on the n2100 exhibit PCI parity problems. Set
- * the ->broken_parity_status flag for both ports so that the r8169
- * driver knows it should ignore error interrupts.
+ * Both r8169 chips on the n2100 exhibit PCI parity problems. Turn
+ * off parity reporting for both ports so we don't get error interrupts
+ * for them.
*/
static void n2100_fixup_r8169(struct pci_dev *dev)
{
if (dev->bus->number == 0 &&
(dev->devfn == PCI_DEVFN(1, 0) ||
dev->devfn == PCI_DEVFN(2, 0)))
- dev->broken_parity_status = 1;
+ pci_disable_parity(dev);
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, PCI_ANY_ID, n2100_fixup_r8169);
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index f704da3f214c..a6aff0d993eb 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4358,20 +4358,6 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
if (net_ratelimit())
netdev_err(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:
- * - it seems to work;
- * - I did not see what else could be done;
- * - it makes iop3xx happy.
- *
- * Feel free to adjust to your needs.
- */
- if (pdev->broken_parity_status)
- pci_cmd &= ~PCI_COMMAND_PARITY;
- else
- pci_cmd |= PCI_COMMAND_SERR | PCI_COMMAND_PARITY;
-
- pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4 0/3] PCI: Disable parity checking
2021-03-30 17:43 [PATCH v4 0/3] PCI: Disable parity checking Bjorn Helgaas
` (2 preceding siblings ...)
2021-03-30 17:43 ` [PATCH v4 3/3] ARM: iop32x: disable N2100 PCI " Bjorn Helgaas
@ 2021-03-31 11:08 ` Heiner Kallweit
2021-03-31 11:26 ` Krzysztof Wilczyński
4 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2021-03-31 11:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Leon Romanovsky, Russell King - ARM Linux, David Miller,
Jakub Kicinski, nic_swsd, linux-pci, linux-arm-kernel, netdev,
Bjorn Helgaas
On 30.03.2021 19:43, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> I think this is essentially the same as Heiner's v3 posting, with these
> changes:
>
> - Added a pci_disable_parity() interface in pci.c instead of making a
> public pci_quirk_broken_parity() because quirks.c is only compiled when
> CONFIG_PCI_QUIRKS=y.
>
> - Removed the setting of dev->broken_parity_status because it's really
> only used by EDAC error reporting, and if we disable parity error
> reporting, we shouldn't get there. This change will be visible in the
> sysfs "broken_parity_status" file, but I doubt that's important.
>
> I dropped Leon's reviewed-by because I fiddled with the code. Similarly I
> haven't added your signed-off-by, Heiner, because I don't want you blamed
> for my errors. But if this looks OK to you I'll add it.
>
> v1: https://lore.kernel.org/r/a6f09e1b-4076-59d1-a4e3-05c5955bfff2@gmail.com
> v2: https://lore.kernel.org/r/bbc33d9b-af7c-8910-cdb3-fa3e3b2e3266@gmail.com
> - reduce scope of N2100 change to using the new PCI core quirk
> v3: https://lore.kernel.org/r/992c800e-2e12-16b0-4845-6311b295d932@gmail.com/
> - improve commit message of patch 2
>
> v4:
> - add pci_disable_parity() (not conditional on CONFIG_PCI_QUIRKS)
> - remove setting of dev->broken_parity_status
>
>
> Bjorn Helgaas (1):
> PCI: Add pci_disable_parity()
>
> Heiner Kallweit (2):
> IB/mthca: Disable parity reporting
> ARM: iop32x: disable N2100 PCI parity reporting
>
> arch/arm/mach-iop32x/n2100.c | 8 ++++----
> drivers/net/ethernet/realtek/r8169_main.c | 14 --------------
> drivers/pci/pci.c | 17 +++++++++++++++++
> drivers/pci/quirks.c | 13 ++++---------
> include/linux/pci.h | 1 +
> 5 files changed, 26 insertions(+), 27 deletions(-)
>
LGTM. Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4 0/3] PCI: Disable parity checking
2021-03-30 17:43 [PATCH v4 0/3] PCI: Disable parity checking Bjorn Helgaas
` (3 preceding siblings ...)
2021-03-31 11:08 ` [PATCH v4 0/3] PCI: Disable parity checking Heiner Kallweit
@ 2021-03-31 11:26 ` Krzysztof Wilczyński
4 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Wilczyński @ 2021-03-31 11:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Heiner Kallweit, Leon Romanovsky, Russell King - ARM Linux,
David Miller, Jakub Kicinski, nic_swsd, linux-pci,
linux-arm-kernel, netdev, Bjorn Helgaas
Hi Bjorn,
> I think this is essentially the same as Heiner's v3 posting, with these
> changes:
>
> - Added a pci_disable_parity() interface in pci.c instead of making a
> public pci_quirk_broken_parity() because quirks.c is only compiled when
> CONFIG_PCI_QUIRKS=y.
Very nice idea to add pci_disable_parity(), looks very clean.
> - Removed the setting of dev->broken_parity_status because it's really
> only used by EDAC error reporting, and if we disable parity error
> reporting, we shouldn't get there. This change will be visible in the
> sysfs "broken_parity_status" file, but I doubt that's important.
>
> I dropped Leon's reviewed-by because I fiddled with the code. Similarly I
> haven't added your signed-off-by, Heiner, because I don't want you blamed
> for my errors. But if this looks OK to you I'll add it.
[...]
Thank you Bjorn and Heiner!
Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread