> +static int bcm5461_enable_fiber(struct mii_phy* phy, int autoneg) > +{ > + /* select fiber mode, enable 1000 base-X registers */ > + phy_write(phy, MII_NCONFIG, 0xfc0b); > + > + if (autoneg) { > + /* enable fiber with no autonegotiation */ > + phy_write(phy, MII_ADVERTISE, 0x01e0); > + phy_write(phy, MII_BMCR, 0x1140); > + } else { > + /* enable fiber with autonegotiation */ > + phy_write(phy, MII_BMCR, 0x0140); > + } > + > + phy->autoneg = autoneg; > + > + return 0; > +}
Something is backward... either the code or the comments... Also, I dislike the autoneg bits without using any constants. Why not use the normal setup_aneg() ? I think what is needed is a set_link_mode or something like that that takes (fiber/copper) as an argument (or better, use the proper names as documented by the chip). Doesn't matter to much right now, if the code works. Have you had a chance to check you don't break G5s with 5421 sungem ? If yes, then the patch is good to go for now but we should revisit and/or try to merge that with the generic PHY layer at one point. Ben. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html