[src/trunk]: src/sys/dev/pci simplify intr establish code - rely on pci_intr_...

[jdolecek <jdolecek%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/bf7bbcba1de2
branches:  trunk
changeset: 446330:bf7bbcba1de2
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Fri Nov 30 17:52:11 2018 +0000

description:
simplify intr establish code - rely on pci_intr_alloc() to return
interrupt types which are possible for pci_intr_establish(); remove
fallbacks to retry with MSI explicitly disabled

bge(4) specifically needs to disable MSI on some boards which are
known to have broken MSI support, so this can't use pci_intr_alloc()
with just NULL counts

discussed on tech-kern@, and specifically bge(4) with Manuel

https://mail-index.netbsd.org/tech-kern/2018/11/27/msg024240.html

diffstat:

 sys/dev/pci/if_bge.c |  65 ++++++++++++++++++++++++----------------------------
 1 files changed, 30 insertions(+), 35 deletions(-)

diffs (106 lines):

diff -r 9007902c80b1 -r bf7bbcba1de2 sys/dev/pci/if_bge.c
--- a/sys/dev/pci/if_bge.c      Fri Nov 30 17:47:54 2018 +0000
+++ b/sys/dev/pci/if_bge.c      Fri Nov 30 17:52:11 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_bge.c,v 1.317 2018/11/27 19:17:02 bouyer Exp $      */
+/*     $NetBSD: if_bge.c,v 1.318 2018/11/30 17:52:11 jdolecek Exp $    */
 
 /*
  * Copyright (c) 2001 Wind River Systems
@@ -79,7 +79,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_bge.c,v 1.317 2018/11/27 19:17:02 bouyer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bge.c,v 1.318 2018/11/30 17:52:11 jdolecek Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -3372,8 +3372,6 @@
        const struct bge_product *bp;
        const struct bge_revision *br;
        pci_chipset_tag_t       pc;
-       int counts[PCI_INTR_TYPE_SIZE];
-       pci_intr_type_t intr_type, max_type;
        const char              *intrstr = NULL;
        uint32_t                hwcfg, hwcfg2, hwcfg3, hwcfg4, hwcfg5;
        uint32_t                command;
@@ -3761,17 +3759,18 @@
                }
        }
 
-       /* MSI-X will be used in future */
-       counts[PCI_INTR_TYPE_MSI] = 1;
-       counts[PCI_INTR_TYPE_INTX] = 1;
-       /* Check MSI capability */
-       if (bge_can_use_msi(sc) != 0) {
-               max_type = PCI_INTR_TYPE_MSI;
-               sc->bge_flags |= BGEF_MSI;
-       } else
+       int counts[PCI_INTR_TYPE_SIZE] = {
+               [PCI_INTR_TYPE_INTX] = 1,
+               [PCI_INTR_TYPE_MSI] = 1,
+               [PCI_INTR_TYPE_MSIX] = 0, /* MSI-X will be used in future */
+       };
+       int max_type = PCI_INTR_TYPE_MSIX;
+
+       if (!bge_can_use_msi(sc)) {
+               /* MSI broken, allow only INTx */
                max_type = PCI_INTR_TYPE_INTX;
-
-alloc_retry:
+       }
+
        if (pci_intr_alloc(pa, &sc->bge_pihp, counts, max_type) != 0) {
                aprint_error_dev(sc->bge_dev, "couldn't alloc interrupt\n");
                return;
@@ -3784,32 +3783,28 @@
        sc->bge_intrhand = pci_intr_establish_xname(pc, sc->bge_pihp[0],
            IPL_NET, bge_intr, sc, device_xname(sc->bge_dev));
        if (sc->bge_intrhand == NULL) {
-               intr_type = pci_intr_type(pc, sc->bge_pihp[0]);
-               aprint_error_dev(sc->bge_dev,"unable to establish %s\n",
-                   (intr_type == PCI_INTR_TYPE_MSI) ? "MSI" : "INTx");
                pci_intr_release(pc, sc->bge_pihp, 1);
-               switch (intr_type) {
-               case PCI_INTR_TYPE_MSI:
-                       /* The next try is for INTx: Disable MSI */
-                       max_type = PCI_INTR_TYPE_INTX;
-                       counts[PCI_INTR_TYPE_INTX] = 1;
-                       sc->bge_flags &= ~BGEF_MSI;
-                       goto alloc_retry;
-               case PCI_INTR_TYPE_INTX:
-               default:
-                       /* See below */
-                       break;
-               }
-       }
-
-       if (sc->bge_intrhand == NULL) {
-               aprint_error_dev(sc->bge_dev,
-                   "couldn't establish interrupt%s%s\n",
-                   intrstr ? " at " : "", intrstr ? intrstr : "");
+               sc->bge_pihp = NULL;
+
+               aprint_error_dev(self, "couldn't establish interrupt");
+               if (intrstr != NULL)
+                       aprint_error(" at %s", intrstr);
+               aprint_error("\n");
                return;
        }
        aprint_normal_dev(sc->bge_dev, "interrupting at %s\n", intrstr);
 
+       switch (pci_intr_type(pc, sc->bge_pihp[0])) {
+       case PCI_INTR_TYPE_MSIX:
+       case PCI_INTR_TYPE_MSI:
+               KASSERT(bge_can_use_msi(sc));
+               sc->bge_flags |= BGEF_MSI;
+               break;
+       default:
+               /* nothing to do */
+               break;
+       }
+
        /*
         * All controllers except BCM5700 supports tagged status but
         * we use tagged status only for MSI case on BCM5717. Otherwise