* [PATCH v3 0/3] PCI: Disable parity checking if broken_parity is set
@ 2021-01-06 17:49 Heiner Kallweit
2021-01-06 17:50 ` [PATCH v3 1/3] " Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Heiner Kallweit @ 2021-01-06 17:49 UTC (permalink / raw)
To: Bjorn Helgaas, Russell King - ARM Linux,
Realtek linux nic maintainers, David Miller, Jakub Kicinski
Cc: linux-pci, linux-arm-kernel, netdev
If we know that a device has broken parity checking, then disable it.
This avoids quirks like in r8169 where on the first parity error
interrupt parity checking will be disabled if broken_parity_status
is set. Make pci_quirk_broken_parity() public so that it can be used
by platform code, e.g. for Thecus N2100.
v2:
- reduce scope of N2100 change to using the new PCI core quirk
v3:
- improve commit message of patch 2
Heiner Kallweit (3):
PCI: Disable parity checking if broken_parity_status is set
ARM: iop32x: improve N2100 PCI broken parity quirk
r8169: simplify broken parity handling now that PCI core takes care
arch/arm/mach-iop32x/n2100.c | 2 +-
drivers/net/ethernet/realtek/r8169_main.c | 14 --------------
drivers/pci/quirks.c | 17 +++++++++++------
include/linux/pci.h | 2 ++
4 files changed, 14 insertions(+), 21 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-06 17:49 [PATCH v3 0/3] PCI: Disable parity checking if broken_parity is set Heiner Kallweit @ 2021-01-06 17:50 ` Heiner Kallweit 2021-01-06 19:22 ` Bjorn Helgaas 2021-01-06 17:51 ` [PATCH v3 2/3] ARM: iop32x: improve N2100 PCI broken parity quirk Heiner Kallweit 2021-01-06 17:52 ` [PATCH v3 3/3] r8169: simplify broken parity handling now that PCI core takes care Heiner Kallweit 2 siblings, 1 reply; 9+ messages in thread From: Heiner Kallweit @ 2021-01-06 17:50 UTC (permalink / raw) To: Bjorn Helgaas, Russell King - ARM Linux, Realtek linux nic maintainers, David Miller, Jakub Kicinski Cc: linux-pci, linux-arm-kernel, netdev If we know that a device has broken parity checking, then disable it. This avoids quirks like in r8169 where on the first parity error interrupt parity checking will be disabled if broken_parity_status is set. Make pci_quirk_broken_parity() public so that it can be used by platform code, e.g. for Thecus N2100. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/pci/quirks.c | 17 +++++++++++------ include/linux/pci.h | 2 ++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 653660e3b..ab54e26b8 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); +void pci_quirk_broken_parity(struct pci_dev *dev) +{ + u16 cmd; + + dev->broken_parity_status = 1; /* This device gives false positives */ + pci_read_config_word(dev, PCI_COMMAND, &cmd); + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); +} + /* * 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. */ -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_quirk_broken_parity); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); /* * Deal with broken BIOSes that neglect to enable passive release, diff --git a/include/linux/pci.h b/include/linux/pci.h index b32126d26..161dcc474 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { pci_fixup_suspend_late, /* pci_device_suspend_late() */ }; +void pci_quirk_broken_parity(struct pci_dev *dev); + #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ class_shift, hook) \ -- 2.30.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-06 17:50 ` [PATCH v3 1/3] " Heiner Kallweit @ 2021-01-06 19:22 ` Bjorn Helgaas 2021-01-06 19:34 ` Heiner Kallweit 0 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2021-01-06 19:22 UTC (permalink / raw) To: Heiner Kallweit Cc: Bjorn Helgaas, Russell King - ARM Linux, Realtek linux nic maintainers, David Miller, Jakub Kicinski, linux-pci, linux-arm-kernel, netdev On Wed, Jan 06, 2021 at 06:50:22PM +0100, Heiner Kallweit wrote: > If we know that a device has broken parity checking, then disable it. > This avoids quirks like in r8169 where on the first parity error > interrupt parity checking will be disabled if broken_parity_status > is set. Make pci_quirk_broken_parity() public so that it can be used > by platform code, e.g. for Thecus N2100. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > Reviewed-by: Leon Romanovsky <leonro@nvidia.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> This series should all go together. Let me know if you want me to do anything more (would require acks for arm and r8169, of course). > --- > drivers/pci/quirks.c | 17 +++++++++++------ > include/linux/pci.h | 2 ++ > 2 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 653660e3b..ab54e26b8 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) > DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, > PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); > > +void pci_quirk_broken_parity(struct pci_dev *dev) > +{ > + u16 cmd; > + > + dev->broken_parity_status = 1; /* This device gives false positives */ > + pci_read_config_word(dev, PCI_COMMAND, &cmd); > + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); > +} > + > /* > * 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. > */ > -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_quirk_broken_parity); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); > > /* > * Deal with broken BIOSes that neglect to enable passive release, > diff --git a/include/linux/pci.h b/include/linux/pci.h > index b32126d26..161dcc474 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { > pci_fixup_suspend_late, /* pci_device_suspend_late() */ > }; > > +void pci_quirk_broken_parity(struct pci_dev *dev); > + > #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS > #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ > class_shift, hook) \ > -- > 2.30.0 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-06 19:22 ` Bjorn Helgaas @ 2021-01-06 19:34 ` Heiner Kallweit 2021-01-13 20:52 ` Heiner Kallweit 0 siblings, 1 reply; 9+ messages in thread From: Heiner Kallweit @ 2021-01-06 19:34 UTC (permalink / raw) To: Bjorn Helgaas, Lennert Buytenhek, Russell King - ARM Linux Cc: Bjorn Helgaas, Realtek linux nic maintainers, David Miller, Jakub Kicinski, linux-pci, linux-arm-kernel, netdev On 06.01.2021 20:22, Bjorn Helgaas wrote: > On Wed, Jan 06, 2021 at 06:50:22PM +0100, Heiner Kallweit wrote: >> If we know that a device has broken parity checking, then disable it. >> This avoids quirks like in r8169 where on the first parity error >> interrupt parity checking will be disabled if broken_parity_status >> is set. Make pci_quirk_broken_parity() public so that it can be used >> by platform code, e.g. for Thecus N2100. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > This series should all go together. Let me know if you want me to do > anything more (would require acks for arm and r8169, of course). > Right. For r8169 I'm the maintainer myself and agreed with Jakub that the r8169 patch will go through the PCI tree. Regarding the arm/iop32x part: MAINTAINERS file lists Lennert as maintainer, let me add him. Strange thing is that the MAINTAINERS entry for arm/iop32x has no F entry, therefore the get_maintainers scripts will never list him as addressee. The script lists Russell as "odd fixer". @Lennert: Please provide a patch to add the missing F entry. ARM/INTEL IOP32X ARM ARCHITECTURE M: Lennert Buytenhek <kernel@wantstofly.org> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained >> --- >> drivers/pci/quirks.c | 17 +++++++++++------ >> include/linux/pci.h | 2 ++ >> 2 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 653660e3b..ab54e26b8 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) >> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, >> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); >> >> +void pci_quirk_broken_parity(struct pci_dev *dev) >> +{ >> + u16 cmd; >> + >> + dev->broken_parity_status = 1; /* This device gives false positives */ >> + pci_read_config_word(dev, PCI_COMMAND, &cmd); >> + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); >> +} >> + >> /* >> * 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. >> */ >> -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_quirk_broken_parity); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); >> >> /* >> * Deal with broken BIOSes that neglect to enable passive release, >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index b32126d26..161dcc474 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { >> pci_fixup_suspend_late, /* pci_device_suspend_late() */ >> }; >> >> +void pci_quirk_broken_parity(struct pci_dev *dev); >> + >> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS >> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ >> class_shift, hook) \ >> -- >> 2.30.0 >> >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-06 19:34 ` Heiner Kallweit @ 2021-01-13 20:52 ` Heiner Kallweit 2021-01-13 21:25 ` Bjorn Helgaas 2021-01-14 12:42 ` Lennert Buytenhek 0 siblings, 2 replies; 9+ messages in thread From: Heiner Kallweit @ 2021-01-13 20:52 UTC (permalink / raw) To: Bjorn Helgaas, Lennert Buytenhek, Russell King - ARM Linux Cc: Bjorn Helgaas, Realtek linux nic maintainers, David Miller, Jakub Kicinski, linux-pci, linux-arm-kernel, netdev On 06.01.2021 20:34, Heiner Kallweit wrote: > On 06.01.2021 20:22, Bjorn Helgaas wrote: >> On Wed, Jan 06, 2021 at 06:50:22PM +0100, Heiner Kallweit wrote: >>> If we know that a device has broken parity checking, then disable it. >>> This avoids quirks like in r8169 where on the first parity error >>> interrupt parity checking will be disabled if broken_parity_status >>> is set. Make pci_quirk_broken_parity() public so that it can be used >>> by platform code, e.g. for Thecus N2100. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> >> >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> This series should all go together. Let me know if you want me to do >> anything more (would require acks for arm and r8169, of course). >> > Right. For r8169 I'm the maintainer myself and agreed with Jakub that > the r8169 patch will go through the PCI tree. > > Regarding the arm/iop32x part: > MAINTAINERS file lists Lennert as maintainer, let me add him. > Strange thing is that the MAINTAINERS entry for arm/iop32x has no > F entry, therefore the get_maintainers scripts will never list him > as addressee. The script lists Russell as "odd fixer". > @Lennert: Please provide a patch to add the missing F entry. > > ARM/INTEL IOP32X ARM ARCHITECTURE > M: Lennert Buytenhek <kernel@wantstofly.org> > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > S: Maintained > Bjorn, I saw that you set the series to "not applicable". Is this because of the missing ack for the arm part? I checked and Lennert's last kernel contribution is from 2015. Having said that the maintainer's entry may be outdated. Not sure who else would be entitled to ack this patch. The change is simple enough, could you take it w/o an ack? Alternatively, IIRC Russell has got such a device. Russell, would it be possible that you test that there's still no false-positive parity errors with this series? > >>> --- >>> drivers/pci/quirks.c | 17 +++++++++++------ >>> include/linux/pci.h | 2 ++ >>> 2 files changed, 13 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >>> index 653660e3b..ab54e26b8 100644 >>> --- a/drivers/pci/quirks.c >>> +++ b/drivers/pci/quirks.c >>> @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) >>> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, >>> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); >>> >>> +void pci_quirk_broken_parity(struct pci_dev *dev) >>> +{ >>> + u16 cmd; >>> + >>> + dev->broken_parity_status = 1; /* This device gives false positives */ >>> + pci_read_config_word(dev, PCI_COMMAND, &cmd); >>> + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); >>> +} >>> + >>> /* >>> * 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. >>> */ >>> -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_quirk_broken_parity); >>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); >>> >>> /* >>> * Deal with broken BIOSes that neglect to enable passive release, >>> diff --git a/include/linux/pci.h b/include/linux/pci.h >>> index b32126d26..161dcc474 100644 >>> --- a/include/linux/pci.h >>> +++ b/include/linux/pci.h >>> @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { >>> pci_fixup_suspend_late, /* pci_device_suspend_late() */ >>> }; >>> >>> +void pci_quirk_broken_parity(struct pci_dev *dev); >>> + >>> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS >>> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ >>> class_shift, hook) \ >>> -- >>> 2.30.0 >>> >>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-13 20:52 ` Heiner Kallweit @ 2021-01-13 21:25 ` Bjorn Helgaas 2021-01-14 12:42 ` Lennert Buytenhek 1 sibling, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2021-01-13 21:25 UTC (permalink / raw) To: Heiner Kallweit Cc: Lennert Buytenhek, Russell King - ARM Linux, Bjorn Helgaas, Realtek linux nic maintainers, David Miller, Jakub Kicinski, linux-pci, linux-arm-kernel, netdev On Wed, Jan 13, 2021 at 09:52:23PM +0100, Heiner Kallweit wrote: > On 06.01.2021 20:34, Heiner Kallweit wrote: > > On 06.01.2021 20:22, Bjorn Helgaas wrote: > >> On Wed, Jan 06, 2021 at 06:50:22PM +0100, Heiner Kallweit wrote: > >>> If we know that a device has broken parity checking, then disable it. > >>> This avoids quirks like in r8169 where on the first parity error > >>> interrupt parity checking will be disabled if broken_parity_status > >>> is set. Make pci_quirk_broken_parity() public so that it can be used > >>> by platform code, e.g. for Thecus N2100. > >>> > >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > >> > >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> > >> This series should all go together. Let me know if you want me to do > >> anything more (would require acks for arm and r8169, of course). > >> > > Right. For r8169 I'm the maintainer myself and agreed with Jakub that > > the r8169 patch will go through the PCI tree. > > > > Regarding the arm/iop32x part: > > MAINTAINERS file lists Lennert as maintainer, let me add him. > > Strange thing is that the MAINTAINERS entry for arm/iop32x has no > > F entry, therefore the get_maintainers scripts will never list him > > as addressee. The script lists Russell as "odd fixer". > > @Lennert: Please provide a patch to add the missing F entry. > > > > ARM/INTEL IOP32X ARM ARCHITECTURE > > M: Lennert Buytenhek <kernel@wantstofly.org> > > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > > S: Maintained > > Bjorn, I saw that you set the series to "not applicable". Is this because > of the missing ack for the arm part? No, it's because I screwed up. I use "not applicable" when I expect patches to go via another tree. I just missed your note about merging via the PCI tree. I'll take a look soon. > I checked and Lennert's last kernel contribution is from 2015. Having said > that the maintainer's entry may be outdated. Not sure who else would be > entitled to ack this patch. The change is simple enough, could you take > it w/o an ack? > Alternatively, IIRC Russell has got such a device. Russell, would it > be possible that you test that there's still no false-positive parity > errors with this series? > > > > > >>> --- > >>> drivers/pci/quirks.c | 17 +++++++++++------ > >>> include/linux/pci.h | 2 ++ > >>> 2 files changed, 13 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > >>> index 653660e3b..ab54e26b8 100644 > >>> --- a/drivers/pci/quirks.c > >>> +++ b/drivers/pci/quirks.c > >>> @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) > >>> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, > >>> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); > >>> > >>> +void pci_quirk_broken_parity(struct pci_dev *dev) > >>> +{ > >>> + u16 cmd; > >>> + > >>> + dev->broken_parity_status = 1; /* This device gives false positives */ > >>> + pci_read_config_word(dev, PCI_COMMAND, &cmd); > >>> + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); > >>> +} > >>> + > >>> /* > >>> * 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. > >>> */ > >>> -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_quirk_broken_parity); > >>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); > >>> > >>> /* > >>> * Deal with broken BIOSes that neglect to enable passive release, > >>> diff --git a/include/linux/pci.h b/include/linux/pci.h > >>> index b32126d26..161dcc474 100644 > >>> --- a/include/linux/pci.h > >>> +++ b/include/linux/pci.h > >>> @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { > >>> pci_fixup_suspend_late, /* pci_device_suspend_late() */ > >>> }; > >>> > >>> +void pci_quirk_broken_parity(struct pci_dev *dev); > >>> + > >>> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS > >>> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ > >>> class_shift, hook) \ > >>> -- > >>> 2.30.0 > >>> > >>> > >>> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] PCI: Disable parity checking if broken_parity is set 2021-01-13 20:52 ` Heiner Kallweit 2021-01-13 21:25 ` Bjorn Helgaas @ 2021-01-14 12:42 ` Lennert Buytenhek 1 sibling, 0 replies; 9+ messages in thread From: Lennert Buytenhek @ 2021-01-14 12:42 UTC (permalink / raw) To: Heiner Kallweit Cc: Bjorn Helgaas, Russell King - ARM Linux, Bjorn Helgaas, Realtek linux nic maintainers, David Miller, Jakub Kicinski, linux-pci, linux-arm-kernel, netdev On Wed, Jan 13, 2021 at 09:52:23PM +0100, Heiner Kallweit wrote: > On 06.01.2021 20:34, Heiner Kallweit wrote: > > On 06.01.2021 20:22, Bjorn Helgaas wrote: > >> On Wed, Jan 06, 2021 at 06:50:22PM +0100, Heiner Kallweit wrote: > >>> If we know that a device has broken parity checking, then disable it. > >>> This avoids quirks like in r8169 where on the first parity error > >>> interrupt parity checking will be disabled if broken_parity_status > >>> is set. Make pci_quirk_broken_parity() public so that it can be used > >>> by platform code, e.g. for Thecus N2100. > >>> > >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com> > >> > >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> > >> This series should all go together. Let me know if you want me to do > >> anything more (would require acks for arm and r8169, of course). > >> > > Right. For r8169 I'm the maintainer myself and agreed with Jakub that > > the r8169 patch will go through the PCI tree. > > > > Regarding the arm/iop32x part: > > MAINTAINERS file lists Lennert as maintainer, let me add him. > > Strange thing is that the MAINTAINERS entry for arm/iop32x has no > > F entry, therefore the get_maintainers scripts will never list him > > as addressee. The script lists Russell as "odd fixer". > > @Lennert: Please provide a patch to add the missing F entry. > > > > ARM/INTEL IOP32X ARM ARCHITECTURE > > M: Lennert Buytenhek <kernel@wantstofly.org> > > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > > S: Maintained > > Bjorn, I saw that you set the series to "not applicable". Is this because > of the missing ack for the arm part? > I checked and Lennert's last kernel contribution is from 2015. Having said > that the maintainer's entry may be outdated. Not sure who else would be > entitled to ack this patch. The change is simple enough, could you take > it w/o an ack? This entry is indeed outdated, I don't have access to this hardware anymore. > Alternatively, IIRC Russell has got such a device. Russell, would it > be possible that you test that there's still no false-positive parity > errors with this series? > > > > > >>> --- > >>> drivers/pci/quirks.c | 17 +++++++++++------ > >>> include/linux/pci.h | 2 ++ > >>> 2 files changed, 13 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > >>> index 653660e3b..ab54e26b8 100644 > >>> --- a/drivers/pci/quirks.c > >>> +++ b/drivers/pci/quirks.c > >>> @@ -205,17 +205,22 @@ static void quirk_mmio_always_on(struct pci_dev *dev) > >>> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID, > >>> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on); > >>> > >>> +void pci_quirk_broken_parity(struct pci_dev *dev) > >>> +{ > >>> + u16 cmd; > >>> + > >>> + dev->broken_parity_status = 1; /* This device gives false positives */ > >>> + pci_read_config_word(dev, PCI_COMMAND, &cmd); > >>> + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_PARITY); > >>> +} > >>> + > >>> /* > >>> * 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. > >>> */ > >>> -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_quirk_broken_parity); > >>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE, pci_quirk_broken_parity); > >>> > >>> /* > >>> * Deal with broken BIOSes that neglect to enable passive release, > >>> diff --git a/include/linux/pci.h b/include/linux/pci.h > >>> index b32126d26..161dcc474 100644 > >>> --- a/include/linux/pci.h > >>> +++ b/include/linux/pci.h > >>> @@ -1916,6 +1916,8 @@ enum pci_fixup_pass { > >>> pci_fixup_suspend_late, /* pci_device_suspend_late() */ > >>> }; > >>> > >>> +void pci_quirk_broken_parity(struct pci_dev *dev); > >>> + > >>> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS > >>> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \ > >>> class_shift, hook) \ > >>> -- > >>> 2.30.0 > >>> > >>> > >>> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] ARM: iop32x: improve N2100 PCI broken parity quirk 2021-01-06 17:49 [PATCH v3 0/3] PCI: Disable parity checking if broken_parity is set Heiner Kallweit 2021-01-06 17:50 ` [PATCH v3 1/3] " Heiner Kallweit @ 2021-01-06 17:51 ` Heiner Kallweit 2021-01-06 17:52 ` [PATCH v3 3/3] r8169: simplify broken parity handling now that PCI core takes care Heiner Kallweit 2 siblings, 0 replies; 9+ messages in thread From: Heiner Kallweit @ 2021-01-06 17:51 UTC (permalink / raw) To: Bjorn Helgaas, Russell King - ARM Linux, Realtek linux nic maintainers, David Miller, Jakub Kicinski Cc: linux-pci, linux-arm-kernel, netdev Use new PCI core function pci_quirk_broken_parity() to disable parity checking. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- v2: - remove additional changes from this patch v3: - improve commit message --- arch/arm/mach-iop32x/n2100.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-iop32x/n2100.c b/arch/arm/mach-iop32x/n2100.c index 78b9a5ee4..9f2aae3cd 100644 --- a/arch/arm/mach-iop32x/n2100.c +++ b/arch/arm/mach-iop32x/n2100.c @@ -125,7 +125,7 @@ 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_quirk_broken_parity(dev); } DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, PCI_ANY_ID, n2100_fixup_r8169); -- 2.30.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] r8169: simplify broken parity handling now that PCI core takes care 2021-01-06 17:49 [PATCH v3 0/3] PCI: Disable parity checking if broken_parity is set Heiner Kallweit 2021-01-06 17:50 ` [PATCH v3 1/3] " Heiner Kallweit 2021-01-06 17:51 ` [PATCH v3 2/3] ARM: iop32x: improve N2100 PCI broken parity quirk Heiner Kallweit @ 2021-01-06 17:52 ` Heiner Kallweit 2 siblings, 0 replies; 9+ messages in thread From: Heiner Kallweit @ 2021-01-06 17:52 UTC (permalink / raw) To: Bjorn Helgaas, Russell King - ARM Linux, Realtek linux nic maintainers, David Miller, Jakub Kicinski Cc: linux-pci, linux-arm-kernel, netdev Meanwhile the PCI core disables parity checking for a device that has broken_parity_status set. Therefore we don't need the quirk any longer to disable parity checking on the first parity error interrupt. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index c9abc7ccb..024042f37 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -4329,20 +4329,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.30.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-01-14 12:51 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-06 17:49 [PATCH v3 0/3] PCI: Disable parity checking if broken_parity is set Heiner Kallweit 2021-01-06 17:50 ` [PATCH v3 1/3] " Heiner Kallweit 2021-01-06 19:22 ` Bjorn Helgaas 2021-01-06 19:34 ` Heiner Kallweit 2021-01-13 20:52 ` Heiner Kallweit 2021-01-13 21:25 ` Bjorn Helgaas 2021-01-14 12:42 ` Lennert Buytenhek 2021-01-06 17:51 ` [PATCH v3 2/3] ARM: iop32x: improve N2100 PCI broken parity quirk Heiner Kallweit 2021-01-06 17:52 ` [PATCH v3 3/3] r8169: simplify broken parity handling now that PCI core takes care Heiner Kallweit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).