Re: alc(4): add support for 816x devices (and request for review)

[Leonardo Taccari <iamleot%gmail.com@localhost>]

Hello Christos and the entire NetBSD community,

Christos Zoulas writes:
> Looks pretty good, perhaps this:
> 
> -       if (phy != sc->alc_phyaddr)
> -               return;
> -
> 
> is causing your multiple phy issue?
Great catch! I can confirm that adding it to alc_mii_readreg_816x()
function (just after the variables declaration) fixes it (like in
alc_mii_readreg_813x()).

It is time to give a look to the two REVIEWME where I was unsure...
The first one is:

+		/* REVIEWME: can we access mii->mii_media_active here? */
+		alc_aspm(sc, 1, IFM_SUBTYPE(mii->mii_media_active));

FreeBSD instead of IFM_SUBTYPE() passes IFM_UNKNOWN that we do not have.
Can we safetely use IFM_SUBTYPE() here?

The second (and last) part where I am unsure is how to properly handle
the following case:

+	case PCI_PRODUCT_ATTANSIC_AR8161:
+		/*
+		 * REVIEWME: properly handle AR8161 like FreeBSD:
+		 * if (pci_get_subvendor(dev) == VENDORID_ATHEROS &&
+		 *    pci_get_subdevice(dev) == 0x0091 && sc->alc_rev == 0)
+		 *	sc->alc_flags |= ALC_FLAG_LINK_WAR;
+		 */
+		/* FALLTHROUGH */
+	case PCI_PRODUCT_ATTANSIC_E2200:

NetBSD's PCI_PRODUCT_ATTANSIC_AR8161 and FreeBSD's
DEVICEID_ATHEROS_AR8161 are both 0x1091... Can someone help me to
understand how can we handle it? Are the first two comparisons just
redundant and just sc->alc_rev comparison is needed?


Thank you!
Ciao,
L.