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

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jan 14,  5:59pm, iamleot%gmail.com@localhost (Leonardo Taccari) wrote:
-- Subject: Re: alc(4): add support for 816x devices (and request for review)

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

Great..

| 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?

Or we can just define IFM_UNKNOWN and use it.

| 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?

Yes, look for PCI_SUBSYS_ID_REG in /usr/src/sys/dev/pci on how to
get the subvendor/subdevice with pci_conf_read... Perhaps we should
grow convenience functions for those too.

christos