On Mon, Dec 19, 2005 at 12:20:32PM -0200, Arnaldo Carvalho de Melo wrote:
> @@ -633,7 +643,7 @@ static int __devinit sundance_probe1 (st
>
> np->phys[0] = 1; /* Default setting */
> np->mii_preamble_required++;
> - for (phy = 1; phy <= 32 && phy_idx < MII_CNT; phy++) {
> + for (phy = 0; phy < 32 && phy_idx < MII_CNT; phy++) {
> int mii_status = mdio_read(dev, phy, MII_BMSR);
> int phyx = phy & 0x1f;
> if (mii_status != 0xffff && mii_status != 0x0000) {
(Your PHY is at address 0?) Can you add some debug here to see what
happens in both cases (f.e. print the returned MII_BMSR values for
both 'start at 0' and 'start at 1')? Presumably there's something
about starting at 1 that gets your hardware confused, I'd like to know
what that is..
> @@ -700,17 +710,41 @@ static int __devinit sundance_probe1 (st
> /* If flow control enabled, we need to advertise it.*/
> if (np->flowctrl)
> mdio_write (dev, np->phys[0], MII_ADVERTISE,
> np->mii_if.advertising | 0x0400);
> - mdio_write (dev, np->phys[0], MII_BMCR, BMCR_ANENABLE|BMCR_ANRESTART);
> +
> + if (!sundance_enl832_tx_icnt(pdev))
> + mdio_write(dev, np->phys[0], MII_BMCR,
> BMCR_ANENABLE|BMCR_ANRESTART);
> /* Force media type */
> if (!np->an_enable) {
> - mii_ctl = 0;
> - mii_ctl |= (np->speed == 100) ? BMCR_SPEED100 : 0;
> - mii_ctl |= (np->mii_if.full_duplex) ? BMCR_FULLDPLX : 0;
> - mdio_write (dev, np->phys[0], MII_BMCR, mii_ctl);
> - printk (KERN_INFO "Override speed=%d, %s duplex\n",
> - np->speed, np->mii_if.full_duplex ? "Full" : "Half");
> + if (!sundance_enl832_tx_icnt(pdev)) {
> + mii_ctl = 0;
> + mii_ctl |= np->speed == 100 ? BMCR_SPEED100 : 0;
> + mii_ctl |= np->mii_if.full_duplex ? BMCR_FULLDPLX : 0;
> + mdio_write(dev, np->phys[0], MII_BMCR, mii_ctl);
> + printk(KERN_INFO "Override speed=%d, %s duplex\n",
> + np->speed, np->mii_if.full_duplex ? "Full" :
> "Half");
> + } else {
> + u16 mii_avt;
>
> - }
> + mii_ctl = mdio_read(dev, np->phys[0],
> + MII_BMCR) & 0x0000ffff;
> + mii_avt = mdio_read(dev, np->phys[0], MII_ADVERTISE) &
> + ~(ADVERTISE_100FULL + ADVERTISE_100HALF +
> + ADVERTISE_10FULL + ADVERTISE_10HALF);
> + if (np->speed == 100)
> + mii_avt |= np->mii_if.full_duplex ?
> + ADVERTISE_100FULL :
> + ADVERTISE_100HALF;
> + else
> + mii_avt |= np->mii_if.full_duplex ?
> + ADVERTISE_10FULL :
> + ADVERTISE_10HALF;
> +
> + mdio_write(dev, np->phys[0], MII_ADVERTISE, mii_avt);
> + mdio_write(dev, np->phys[0], MII_BMCR,
> + mii_ctl | BMCR_ANENABLE | BMCR_ANRESTART);
> + }
> + } else if (sundance_enl832_tx_icnt(pdev))
> + mdio_write(dev, np->phys[0], MII_BMCR, BMCR_ANENABLE |
> BMCR_ANRESTART);
The old code did:
restart autonegotiation
if (forced speed/duplex)
force speed/duplex on local end
The new code does:
if (!en1832)
restart autonegotiation
if (forced speed/duplex) {
if (!en1832) {
disable autonegotiation
force speed/duplex on local end
} else {
configure phy to only advertise selected speed/duplex
restart autonegotiation
}
} else if (en1832)
restart autonegotiation
Whatever way is chosen to force speed/duplex (whether to force it on the
local end while disabling autonegotiation, or to enable autoneg and only
advertise the selected speed/duplex type to the link partner), to me it
would seem desirable to use the same method across the board. (Is there
a policy on this?)
So, there are two options:
1/ Decide to implement 'forced speed/duplex' by disabling autoneg
and forcing speed/duplex locally => keep the code the way it was.
2/ Decide to implement 'forced speed/duplex' via autonegotiation,
=> rewrite the code as:
if (forced speed/duplex)
configure phy to only advertise selected speed/duplex
restart autonegotiation
cheers,
Lennert
-
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
