Rin Okuyama writes:
> Hi Andrius,
>
> If you still have this AQC100 in working condition, can you try this patch?
>
> https://gist.github.com/rokuyama/ab6ba1a0fac7fa15f243d63a99e14f33
>
> I've collected three fibre aq(4) variants (all rev 2), and link status
> interrupts work just fine for me. I think that link intr did not work for
> you, not due to fibre variant, but hardware revision. If this is correct,
> the patch above should work...
this reminded me that my aq(4) doesn't have working link and that
mlelstv suggested to me that the linux driver always uses a tick
timer to also check status, as well as interrupts. i implemented
this recently and now my aq(4) has link status correctly:
aq(4): always poll for link status
some devices don't have working link status and rather than have
a likely incomplete list of issues, always poll as well as use
the interrupt if possible.
fixes link status on this device:
aq0 at pci5 dev 0 function 0: Aquantia AQC107 10 Gigabit Network Adapter (rev.
0x02)
aq0: Atlantic revision B1, F/W version 3.1.88
(was otherwise functional, just didn't report status, which likely
meant eg, dhcpcd would be upset?)
idea via mlelstv@ from linux.
remove sc_detect_linkstat and rename sc_poll_linkstat to
sc_no_link_intr, as the meaning has changed. simplify the signature
for aq_setup_msix() and aq_establish_msix_intr(), removing forward
decls that aren't required. obsolete AQ_FORCE_POLL_LINKSTAT.
Index: if_aq.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_aq.c,v
retrieving revision 1.45
diff -p -u -r1.45 if_aq.c
--- if_aq.c 29 May 2023 08:00:05 -0000 1.45
+++ if_aq.c 26 Oct 2023 06:55:28 -0000
@@ -1330,8 +1330,7 @@ struct aq_softc {
int sc_rx_irq[AQ_RSSQUEUE_MAX];
int sc_linkstat_irq;
bool sc_use_txrx_independent_intr;
- bool sc_poll_linkstat;
- bool sc_detect_linkstat;
+ bool sc_no_link_intr;
#if NSYSMON_ENVSYS > 0
struct sysmon_envsys *sc_sme;
@@ -1443,11 +1442,9 @@ static int aq_match(device_t, cfdata_t,
static void aq_attach(device_t, device_t, void *);
static int aq_detach(device_t, int);
-static int aq_setup_msix(struct aq_softc *, struct pci_attach_args *, int,
- bool, bool);
+static int aq_setup_msix(struct aq_softc *, struct pci_attach_args *);
static int aq_setup_legacy(struct aq_softc *, struct pci_attach_args *,
pci_intr_type_t);
-static int aq_establish_msix_intr(struct aq_softc *, bool, bool);
static int aq_ifmedia_change(struct ifnet * const);
static void aq_ifmedia_status(struct ifnet * const, struct ifmediareq *);
@@ -1784,67 +1781,57 @@ aq_attach(device_t parent, device_t self
if (msixcount >= (sc->sc_nqueues * 2 + 1)) {
/* TX intrs + RX intrs + LINKSTAT intrs */
sc->sc_use_txrx_independent_intr = true;
- sc->sc_poll_linkstat = false;
sc->sc_msix = true;
} else if (msixcount >= (sc->sc_nqueues * 2)) {
/* TX intrs + RX intrs */
sc->sc_use_txrx_independent_intr = true;
- sc->sc_poll_linkstat = true;
sc->sc_msix = true;
} else
#endif
if (msixcount >= (sc->sc_nqueues + 1)) {
/* TX/RX intrs LINKSTAT intrs */
sc->sc_use_txrx_independent_intr = false;
- sc->sc_poll_linkstat = false;
sc->sc_msix = true;
} else if (msixcount >= sc->sc_nqueues) {
/* TX/RX intrs */
sc->sc_use_txrx_independent_intr = false;
- sc->sc_poll_linkstat = true;
+ sc->sc_no_link_intr = true;
sc->sc_msix = true;
} else {
/* giving up using MSI-X */
sc->sc_msix = false;
}
- /* on AQ1a0, AQ2, or FIBRE, linkstat interrupt doesn't work? */
- if (aqp->aq_media_type == AQ_MEDIA_TYPE_FIBRE ||
- (HWTYPE_AQ1_P(sc) && FW_VERSION_MAJOR(sc) == 1) ||
- HWTYPE_AQ2_P(sc))
- sc->sc_poll_linkstat = true;
-
-#ifdef AQ_FORCE_POLL_LINKSTAT
- sc->sc_poll_linkstat = true;
-#endif
-
aprint_debug_dev(sc->sc_dev,
"ncpu=%d, pci_msix_count=%d."
" allocate %d interrupts for %d%s queues%s\n",
ncpu, msixcount,
(sc->sc_use_txrx_independent_intr ?
(sc->sc_nqueues * 2) : sc->sc_nqueues) +
- (sc->sc_poll_linkstat ? 0 : 1),
+ (sc->sc_no_link_intr ? 0 : 1),
sc->sc_nqueues,
sc->sc_use_txrx_independent_intr ? "*2" : "",
- sc->sc_poll_linkstat ? "" : ", and link status");
+ (sc->sc_no_link_intr) ? "" : ", and link status");
if (sc->sc_msix)
- error = aq_setup_msix(sc, pa, sc->sc_nqueues,
- sc->sc_use_txrx_independent_intr, !sc->sc_poll_linkstat);
+ error = aq_setup_msix(sc, pa);
else
error = ENODEV;
if (error != 0) {
/* if MSI-X failed, fallback to MSI with single queue */
sc->sc_use_txrx_independent_intr = false;
- sc->sc_poll_linkstat = false;
sc->sc_msix = false;
sc->sc_nqueues = 1;
+ sc->sc_no_link_intr = false;
+ aprint_debug_dev(sc->sc_dev, "MSI-X failed: %d, trying MSI",
+ error);
error = aq_setup_legacy(sc, pa, PCI_INTR_TYPE_MSI);
}
if (error != 0) {
/* if MSI failed, fallback to INTx */
+ aprint_debug_dev(sc->sc_dev, "MSI failed: %d, trying legacy",
+ error);
error = aq_setup_legacy(sc, pa, PCI_INTR_TYPE_INTX);
}
if (error != 0)
@@ -1858,7 +1845,7 @@ aq_attach(device_t parent, device_t self
error = workqueue_create(&sc->sc_reset_wq, wqname,
aq_handle_reset_work, sc, PRI_SOFTNET, IPL_SOFTCLOCK,
WQ_MPSAFE);
- if (error) {
+ if (error != 0) {
aprint_error_dev(sc->sc_dev,
"unable to create reset workqueue\n");
goto attach_failure;
@@ -1979,6 +1966,8 @@ aq_attach(device_t parent, device_t self
if (sysmon_envsys_register(sc->sc_sme)) {
sysmon_envsys_destroy(sc->sc_sme);
sc->sc_sme = NULL;
+ aprint_debug_dev(sc->sc_dev, "failed to create envsys");
+ error = EINVAL;
goto attach_failure;
}
@@ -2025,6 +2014,7 @@ aq_attach(device_t parent, device_t self
return;
attach_failure:
+ aprint_debug_dev(sc->sc_dev, "attach failed: %d", error);
aq_detach(self, 0);
}
@@ -2152,8 +2142,7 @@ aq_establish_intr(struct aq_softc *sc, i
}
static int
-aq_establish_msix_intr(struct aq_softc *sc, bool txrx_independent,
- bool linkintr)
+aq_establish_msix_intr(struct aq_softc *sc)
{
kcpuset_t *affinity;
int error, intno, i;
@@ -2163,7 +2152,7 @@ aq_establish_msix_intr(struct aq_softc *
intno = 0;
- if (txrx_independent) {
+ if (sc->sc_use_txrx_independent_intr) {
for (i = 0; i < sc->sc_nqueues; i++) {
snprintf(intr_xname, sizeof(intr_xname), "%s RX%d",
device_xname(sc->sc_dev), i);
@@ -2195,7 +2184,7 @@ aq_establish_msix_intr(struct aq_softc *
}
}
- if (linkintr) {
+ if (!sc->sc_no_link_intr) {
snprintf(intr_xname, sizeof(intr_xname), "%s LINK",
device_xname(sc->sc_dev));
sc->sc_linkstat_irq = intno;
@@ -2221,9 +2210,11 @@ aq_establish_msix_intr(struct aq_softc *
}
static int
-aq_setup_msix(struct aq_softc *sc, struct pci_attach_args *pa, int nqueue,
- bool txrx_independent, bool linkintr)
+aq_setup_msix(struct aq_softc *sc, struct pci_attach_args *pa)
{
+ int nqueue = sc->sc_nqueues;
+ bool txrx_independent = sc->sc_use_txrx_independent_intr;
+ bool linkintr = !sc->sc_no_link_intr;
int error, nintr;
if (txrx_independent)
@@ -2241,7 +2232,7 @@ aq_setup_msix(struct aq_softc *sc, struc
goto fail;
}
- error = aq_establish_msix_intr(sc, txrx_independent, linkintr);
+ error = aq_establish_msix_intr(sc);
if (error == 0) {
sc->sc_nintrs = nintr;
} else {
@@ -5043,10 +5034,7 @@ aq_tick(void *arg)
return;
}
- if (sc->sc_poll_linkstat || sc->sc_detect_linkstat) {
- sc->sc_detect_linkstat = false;
- aq_update_link_status(sc);
- }
+ aq_update_link_status(sc);
#ifdef AQ_EVENT_COUNTERS
if (sc->sc_poll_statistics)
@@ -5094,7 +5082,6 @@ aq_legacy_intr(void *arg)
if (status & __BIT(sc->sc_linkstat_irq)) {
AQ_LOCK(sc);
- sc->sc_detect_linkstat = true;
if (!sc->sc_stopping)
callout_schedule(&sc->sc_tick_ch, 0);
AQ_UNLOCK(sc);
@@ -5150,7 +5137,6 @@ aq_link_intr(void *arg)
status = AQ_READ_REG(sc, AQ_INTR_STATUS_REG);
if (status & __BIT(sc->sc_linkstat_irq)) {
AQ_LOCK(sc);
- sc->sc_detect_linkstat = true;
if (!sc->sc_stopping)
callout_schedule(&sc->sc_tick_ch, 0);
AQ_UNLOCK(sc);
