> Date: Mon, 27 Dec 2010 02:02:03 -0500 (EST)
> From: logana...@devio.us (Loganaden Velvindron)
Looks like your diff has some issues with tabs versus spaces.
> void
> mlphy_attach(struct device *parent, struct device *self, void *aux)
> {
> struct mlphy_softc *msc = (struct mlphy_softc *)self;
> struct mii_softc *sc = (struct mii_softc *)self;
> struct mii_attach_args *ma = aux;
> struct mii_data *mii = ma->mii_data;
>
> printf(":ML phy \n");
To be consistent with other PHY drivers, this should probably be
something like:
printf(": ML6692 100baseTX PHY\n");
> sc->mii_inst = mii->mii_instance;
> sc->mii_phy = ma->mii_phyno;
> sc->mii_funcs = &mlphy_funcs;
> sc->mii_pdata = mii;
> sc->mii_flags = ma->mii_flags;
> msc->ml_dev = parent;
>
> mii_phy_reset(sc);
>
> sc->mii_capabilities = PHY_READ(sc, MII_BMSR) & ma->mii_capmask;
> #define ADD(m, c) ifmedia_add(&mii->mii_media, (m), (c), NULL)
> ADD(IFM_MAKEWORD(IFM_ETHER, IFM_100_TX, IFM_LOOP, sc->mii_inst),
> MII_MEDIA_100_TX);
> ma->mii_capmask = ~sc->mii_capabilities;
> #undef ADD
> if(sc->mii_capabilities & BMSR_MEDIAMASK)
> mii_phy_add_media(sc);
> }
>
> int
> mlphy_service(struct mii_softc *sc, struct mii_data *mii, int cmd)
> {
> struct ifmedia_entry *ife = mii->mii_media.ifm_cur;
> struct mii_softc *other = NULL;
> struct mlphy_softc *msc = (struct mlphy_softc *)sc;
> int other_inst, reg;
>
> other = LIST_FIRST(&mii->mii_phys);
> for (; other != NULL ; other = LIST_NEXT(other, mii_list)) {
> /*going through list of PHYs*/
> }
Ehm, what does this actually do? This will always result in "other"
being set to NULL isn't it?
It looks like this driver is written to deal with a 10baseT companion
PHY. Does your tl(4) have such a companion PHY? Did you test 10baseT
mode?
>
> if ((sc->mii_dev.dv_flags & DVF_ACTIVE) == 0) {
> return (ENXIO);
> }
Curly braces are redundant.
> switch (cmd) {
> case MII_POLLSTAT:
> /*
> * If we're not polling our PHY instance, just return.
> */
> if (IFM_INST(ife->ifm_media) != sc->mii_inst)
> return (0);
> break;
>
> case MII_MEDIACHG:
> /*
> * If the interface is not up, don't do anything.
> */
> if ((mii->mii_ifp->if_flags & IFF_UP) == 0)
> break;
>
> switch (IFM_SUBTYPE(ife->ifm_media)) {
> case IFM_AUTO:
> msc->ml_state = ML_STATE_AUTO_SELF;
> if (other != NULL) {
> mii_phy_reset(other);
> PHY_WRITE(other, MII_BMCR, BMCR_ISO);
> }
> (void)mii_phy_auto(sc, 0);
> msc->ml_linked = 0;
> break;
>
> case IFM_10_T:
> /*
> * For 10baseT modes, reset and program the
> * companion PHY (of any), then setup ourselves
> * to match. This will put us in pass-through
> * mode and let the companion PHY do all the
> * work.
> * BMCR data is stored in the ifmedia entry.
> */
> if (other != NULL) {
> mii_phy_reset(other);
> PHY_WRITE(other, MII_BMCR,
> ife->ifm_data);
> }
> mii_phy_setmedia(sc);
> msc->ml_state = 0;
> break;
>
> case IFM_100_TX:
> /*
> * For 100baseTX modes, reset and isolate the
> * companion PHY (if any), then setup ourselves
> * accordingly.
> *
> * BMCR data is stored in the ifmedia entry.
> */
> if (other != NULL) {
> mii_phy_reset(other);
> PHY_WRITE(other, MII_BMCR, BMCR_ISO);
> }
> mii_phy_setmedia(sc);
> msc->ml_state = 0;
> break;
>
> default:
> return (EINVAL);
> }
> break;
>
> case MII_TICK:
> /*
> * If interface is not up, don't do anything
> */
> if ((mii->mii_ifp->if_flags & IFF_UP) == 0)
> return (0);
> /*
> * Only used for autonegotiation.
> */
> if (IFM_SUBTYPE(ife->ifm_media) != IFM_AUTO)
> break;
> /*
> * Check to see if we have link. If we do, we don't
> * need to restart the autonegotiation process. Read
> * the BMSR twice in case it's latched.
> * If we're in a 10Mbps mode, check the link of the
> * 10Mbps PHY. Sometimes the Micro Linear PHY's
> * linkstat bit will clear while the linkstat bit of
> * the companion PHY will remain set.
> */
> if (msc->ml_state == ML_STATE_AUTO_OTHER) {
> reg = PHY_READ(other, MII_BMSR) |
> PHY_READ(other, MII_BMSR);
> } else {
> reg = PHY_READ(sc, MII_BMSR) |
> PHY_READ(sc, MII_BMSR);
> }
>
> if (reg & BMSR_LINK) {
> if (!msc->ml_linked) {
> msc->ml_linked = 1;
> mlphy_status(sc);
> }
> break;
> }
> /*
> * Only retry autonegotiation every 5 seconds.
> */
> if (++sc->mii_ticks <= MII_ANEGTICKS)
> break;
>
> sc->mii_ticks = 0;
> msc->ml_linked = 0;
> mii->mii_media_active = IFM_NONE;
> mii_phy_reset(sc);
> msc->ml_state = ML_STATE_AUTO_SELF;
> if (other != NULL) {
> mii_phy_reset(other);
> PHY_WRITE(other, MII_BMCR, BMCR_ISO);
> }
> mii_phy_auto(sc, 0);
> break;
>
> case MII_DOWN:
> mii_phy_down(sc);
> return (0);
> }
>
> /* Update the media status. */
> if (msc->ml_state == ML_STATE_AUTO_OTHER) {
> other_inst = other->mii_inst;
> other->mii_inst = sc->mii_inst;
> if (IFM_INST(ife->ifm_media) == other->mii_inst)
> (void) PHY_SERVICE(other, mii, MII_POLLSTAT);
> other->mii_inst = other_inst;
> sc->mii_media_active = other->mii_media_active;
> sc->mii_media_status = other->mii_media_status;
> } else
> ukphy_status(sc);
>
> /* Callback if something changed. */
> mii_phy_update(sc, cmd);
> return (0);
> }
>
> void
> mlphy_reset(struct mii_softc *sc)
> {
> int reg;
A blank line between local variable declarations and code please.
> mii_phy_reset(sc);
> reg = PHY_READ(sc, MII_BMCR);
> reg &= ~BMCR_AUTOEN;
> PHY_WRITE(sc, MII_BMCR, reg);
> }
>
> /*
> * If we negotiate a 10Mbps mode, we need to check for an alternate
> * PHY and make sure it's enabled and set correctly.
> */
> void
> mlphy_status(struct mii_softc *sc)
> {
> struct mlphy_softc *msc = (struct mlphy_softc *)sc;
> struct mii_data *mii = sc->mii_pdata;
> struct mii_softc *other = NULL;
Same here.
> /* See if there's another PHY on the bus with us. */
> other = LIST_FIRST(&mii->mii_phys);
> for(; other !=NULL; other = LIST_NEXT(other, mii_list)){
Spaces around != please.
> /* cycling through PHYs */
> }
> if (other == NULL)
> return;
> ukphy_status(sc);
>
> if (IFM_SUBTYPE(mii->mii_media_active) != IFM_10_T) {
> msc->ml_state = ML_STATE_AUTO_SELF;
> mii_phy_reset(other);
> PHY_WRITE(other, MII_BMCR, BMCR_ISO);
> }
>
> if (IFM_SUBTYPE(mii->mii_media_active) == IFM_10_T) {
> msc->ml_state = ML_STATE_AUTO_OTHER;
> mlphy_reset(&msc->ml_mii);
> PHY_WRITE(&msc->ml_mii, MII_BMCR, BMCR_ISO);
> mii_phy_reset(other);
> mii_phy_auto(other, 1);
> }
> }
