linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PCI: Fix Switchtec DMA aliasing quirk dmesg noise
@ 2018-10-05 15:49 Logan Gunthorpe
  2018-10-10 21:08 ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: Logan Gunthorpe @ 2018-10-05 15:49 UTC (permalink / raw)
  To: linux-kernel, linux-pci
  Cc: Logan Gunthorpe, Doug Meyer, Bjorn Helgaas, Kurt Schwemmer

Currently the Switchtec quirk runs on all endpoints in the Switch
which includes all the upstream and downstream ports. Seeing these
other functions do not contain BARs the quirk fails when trying to
map the BAR and prints the error "Cannot iomap Switchtec device".
The user will see a few of these useless and scary errors, one for
each port in the switch.

At most, the quirk should only run on either a management endpoint
(class=PCI_CLASS_MEMORY_OTHER) or an NTB endpoint
(PCI_CLASS_BRIDGE_OTHER). However, seeing the quirk is useless except
in NTB applications, we will only run it when the class is
PCI_CLASS_BRIDGE_OTHER.

Thus, switch to using DECLARE_PCI_FIXUP_CLASS_FINAL and clean up
the list with a define (so we don't have to change as much code if
we ever have to adjust the list).

Reported-by: Stephen  Bates <sbates@raithlin.com>
Cc: Doug Meyer <dmeyer@gigaio.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
Fixes: ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---

* v2: Changes comment style, per feedback from Christoph

 drivers/pci/quirks.c | 90 +++++++++++++++++---------------------------
 1 file changed, 34 insertions(+), 56 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6bc27b7fd452..0f072aed30f5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5057,59 +5057,37 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev)
 	pci_iounmap(pdev, mmio);
 	pci_disable_device(pdev);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8531,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8532,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8533,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8534,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8535,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8536,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8543,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8544,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8545,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8546,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8551,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8552,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8553,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8554,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8555,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8556,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8561,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8562,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8563,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8564,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8565,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8566,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8571,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8572,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8573,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8574,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8575,
-			quirk_switchtec_ntb_dma_alias);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8576,
-			quirk_switchtec_ntb_dma_alias);
+
+#define SWITCHTEC_QUIRK(vid) \
+	DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_MICROSEMI, vid, \
+		PCI_CLASS_BRIDGE_OTHER, 8, quirk_switchtec_ntb_dma_alias)
+SWITCHTEC_QUIRK(0x8531);  /* PFX 24xG3 */
+SWITCHTEC_QUIRK(0x8532);  /* PFX 32xG3 */
+SWITCHTEC_QUIRK(0x8533);  /* PFX 48xG3 */
+SWITCHTEC_QUIRK(0x8534);  /* PFX 64xG3 */
+SWITCHTEC_QUIRK(0x8535);  /* PFX 80xG3 */
+SWITCHTEC_QUIRK(0x8536);  /* PFX 96xG3 */
+SWITCHTEC_QUIRK(0x8541);  /* PSX 24xG3 */
+SWITCHTEC_QUIRK(0x8542);  /* PSX 32xG3 */
+SWITCHTEC_QUIRK(0x8543);  /* PSX 48xG3 */
+SWITCHTEC_QUIRK(0x8544);  /* PSX 64xG3 */
+SWITCHTEC_QUIRK(0x8545);  /* PSX 80xG3 */
+SWITCHTEC_QUIRK(0x8546);  /* PSX 96xG3 */
+SWITCHTEC_QUIRK(0x8551);  /* PAX 24XG3 */
+SWITCHTEC_QUIRK(0x8552);  /* PAX 32XG3 */
+SWITCHTEC_QUIRK(0x8553);  /* PAX 48XG3 */
+SWITCHTEC_QUIRK(0x8554);  /* PAX 64XG3 */
+SWITCHTEC_QUIRK(0x8555);  /* PAX 80XG3 */
+SWITCHTEC_QUIRK(0x8556);  /* PAX 96XG3 */
+SWITCHTEC_QUIRK(0x8561);  /* PFXL 24XG3 */
+SWITCHTEC_QUIRK(0x8562);  /* PFXL 32XG3 */
+SWITCHTEC_QUIRK(0x8563);  /* PFXL 48XG3 */
+SWITCHTEC_QUIRK(0x8564);  /* PFXL 64XG3 */
+SWITCHTEC_QUIRK(0x8565);  /* PFXL 80XG3 */
+SWITCHTEC_QUIRK(0x8566);  /* PFXL 96XG3 */
+SWITCHTEC_QUIRK(0x8571);  /* PFXI 24XG3 */
+SWITCHTEC_QUIRK(0x8572);  /* PFXI 32XG3 */
+SWITCHTEC_QUIRK(0x8573);  /* PFXI 48XG3 */
+SWITCHTEC_QUIRK(0x8574);  /* PFXI 64XG3 */
+SWITCHTEC_QUIRK(0x8575);  /* PFXI 80XG3 */
+SWITCHTEC_QUIRK(0x8576);  /* PFXI 96XG3 */
--
2.19.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI: Fix Switchtec DMA aliasing quirk dmesg noise
  2018-10-05 15:49 [PATCH v2] PCI: Fix Switchtec DMA aliasing quirk dmesg noise Logan Gunthorpe
@ 2018-10-10 21:08 ` Bjorn Helgaas
  2018-10-10 23:13   ` Logan Gunthorpe
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2018-10-10 21:08 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-pci, Doug Meyer, Bjorn Helgaas, Kurt Schwemmer

On Fri, Oct 05, 2018 at 09:49:40AM -0600, Logan Gunthorpe wrote:
> Currently the Switchtec quirk runs on all endpoints in the Switch
> which includes all the upstream and downstream ports. Seeing these
> other functions do not contain BARs the quirk fails when trying to
> map the BAR and prints the error "Cannot iomap Switchtec device".
> The user will see a few of these useless and scary errors, one for
> each port in the switch.
> 
> At most, the quirk should only run on either a management endpoint
> (class=PCI_CLASS_MEMORY_OTHER) or an NTB endpoint
> (PCI_CLASS_BRIDGE_OTHER). However, seeing the quirk is useless except
> in NTB applications, we will only run it when the class is
> PCI_CLASS_BRIDGE_OTHER.
> 
> Thus, switch to using DECLARE_PCI_FIXUP_CLASS_FINAL and clean up
> the list with a define (so we don't have to change as much code if
> we ever have to adjust the list).
> 
> Reported-by: Stephen  Bates <sbates@raithlin.com>
> Cc: Doug Meyer <dmeyer@gigaio.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> Fixes: ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Applied to pci/misc for v4.20, thanks!

I split this into two patches so the important change doesn't get lost in
the noise of the SWITCHTEC_QUIRK() addition:

  - Add the SWITCHTEC_QUIRK() macro, but don't change anything else
  - Change SWITCHTEC_QUIRK() to use DECLARE_PCI_FIXUP_CLASS_FINAL

> ---
> 
> * v2: Changes comment style, per feedback from Christoph
> 
>  drivers/pci/quirks.c | 90 +++++++++++++++++---------------------------
>  1 file changed, 34 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6bc27b7fd452..0f072aed30f5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5057,59 +5057,37 @@ static void quirk_switchtec_ntb_dma_alias(struct pci_dev *pdev)
>  	pci_iounmap(pdev, mmio);
>  	pci_disable_device(pdev);
>  }
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8531,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8532,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8533,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8534,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8535,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8536,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8543,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8544,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8545,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8546,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8551,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8552,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8553,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8554,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8555,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8556,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8561,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8562,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8563,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8564,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8565,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8566,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8571,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8572,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8573,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8574,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8575,
> -			quirk_switchtec_ntb_dma_alias);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MICROSEMI, 0x8576,
> -			quirk_switchtec_ntb_dma_alias);
> +
> +#define SWITCHTEC_QUIRK(vid) \
> +	DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_MICROSEMI, vid, \
> +		PCI_CLASS_BRIDGE_OTHER, 8, quirk_switchtec_ntb_dma_alias)
> +SWITCHTEC_QUIRK(0x8531);  /* PFX 24xG3 */
> +SWITCHTEC_QUIRK(0x8532);  /* PFX 32xG3 */
> +SWITCHTEC_QUIRK(0x8533);  /* PFX 48xG3 */
> +SWITCHTEC_QUIRK(0x8534);  /* PFX 64xG3 */
> +SWITCHTEC_QUIRK(0x8535);  /* PFX 80xG3 */
> +SWITCHTEC_QUIRK(0x8536);  /* PFX 96xG3 */
> +SWITCHTEC_QUIRK(0x8541);  /* PSX 24xG3 */
> +SWITCHTEC_QUIRK(0x8542);  /* PSX 32xG3 */
> +SWITCHTEC_QUIRK(0x8543);  /* PSX 48xG3 */
> +SWITCHTEC_QUIRK(0x8544);  /* PSX 64xG3 */
> +SWITCHTEC_QUIRK(0x8545);  /* PSX 80xG3 */
> +SWITCHTEC_QUIRK(0x8546);  /* PSX 96xG3 */
> +SWITCHTEC_QUIRK(0x8551);  /* PAX 24XG3 */
> +SWITCHTEC_QUIRK(0x8552);  /* PAX 32XG3 */
> +SWITCHTEC_QUIRK(0x8553);  /* PAX 48XG3 */
> +SWITCHTEC_QUIRK(0x8554);  /* PAX 64XG3 */
> +SWITCHTEC_QUIRK(0x8555);  /* PAX 80XG3 */
> +SWITCHTEC_QUIRK(0x8556);  /* PAX 96XG3 */
> +SWITCHTEC_QUIRK(0x8561);  /* PFXL 24XG3 */
> +SWITCHTEC_QUIRK(0x8562);  /* PFXL 32XG3 */
> +SWITCHTEC_QUIRK(0x8563);  /* PFXL 48XG3 */
> +SWITCHTEC_QUIRK(0x8564);  /* PFXL 64XG3 */
> +SWITCHTEC_QUIRK(0x8565);  /* PFXL 80XG3 */
> +SWITCHTEC_QUIRK(0x8566);  /* PFXL 96XG3 */
> +SWITCHTEC_QUIRK(0x8571);  /* PFXI 24XG3 */
> +SWITCHTEC_QUIRK(0x8572);  /* PFXI 32XG3 */
> +SWITCHTEC_QUIRK(0x8573);  /* PFXI 48XG3 */
> +SWITCHTEC_QUIRK(0x8574);  /* PFXI 64XG3 */
> +SWITCHTEC_QUIRK(0x8575);  /* PFXI 80XG3 */
> +SWITCHTEC_QUIRK(0x8576);  /* PFXI 96XG3 */
> --
> 2.19.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI: Fix Switchtec DMA aliasing quirk dmesg noise
  2018-10-10 21:08 ` Bjorn Helgaas
@ 2018-10-10 23:13   ` Logan Gunthorpe
  0 siblings, 0 replies; 3+ messages in thread
From: Logan Gunthorpe @ 2018-10-10 23:13 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, Doug Meyer, Bjorn Helgaas, Kurt Schwemmer



On 2018-10-10 3:08 p.m., Bjorn Helgaas wrote:
> Applied to pci/misc for v4.20, thanks!
> 
> I split this into two patches so the important change doesn't get lost in
> the noise of the SWITCHTEC_QUIRK() addition:
> 
>   - Add the SWITCHTEC_QUIRK() macro, but don't change anything else
>   - Change SWITCHTEC_QUIRK() to use DECLARE_PCI_FIXUP_CLASS_FINAL

Perfect, thanks!

Logan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-10-10 23:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 15:49 [PATCH v2] PCI: Fix Switchtec DMA aliasing quirk dmesg noise Logan Gunthorpe
2018-10-10 21:08 ` Bjorn Helgaas
2018-10-10 23:13   ` Logan Gunthorpe

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