On Jan 14, 5:59pm, [email protected] (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
