On Thu, Nov 15, 2012 at 10:31:44AM +0100, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> as we to have specific phy init to fix chip issue link detection support
This patch was introduced to make the LAN8720 work with phylib. When
appplying this patch I believed that...
> +
> +static int lan87xx_config_init(struct phy_device *phydev)
> +{
> + /*
> + * Make sure the EDPWRDOWN bit is NOT set. Setting this bit on
> + * LAN8710/LAN8720 PHY causes the PHY to misbehave, likely due
> + * to a bug on the chip.
> + *
> + * When the system is powered on with the network cable being
> + * disconnected all the way until after ifconfig ethX up is
> + * issued for the LAN port with this PHY, connecting the cable
> + * afterwards does not cause LINK change detection, while the
> + * expected behavior is the Link UP being detected.
> + */
> + int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> + if (rc < 0)
> + return rc;
> +
> + rc &= ~MII_LAN83C185_EDPWRDOWN;
> +
> + rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, rc);
> + if (rc < 0)
> + return rc;
... this change was responsible to let the phy driver correctly wait for
a link. As it just turned out, this change os totally irrelevant. The
change which makes this driver work is...
> +}, {
> + .phy_id = 0x0007c0f0, /* OUI=0x00800f, Model#=0x0f */
> + .phy_id_mask = 0xfffffff0,
> + .drv.name = "SMSC LAN8710/LAN8720",
> +
> + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
> + | SUPPORTED_Asym_Pause),
... this one. The phy now advertises SUPPORTED_Pause and
SUPPORTED_Asym_Pause which the generic phy driver does not do. This
causes the adv != oldadv check in genphy_config_advert to become true.
genphy_config_advert then returns '1' for advertise changed, which
causes genphy_config_aneg to call genphy_restart_aneg which calls
phy_aneg_done to wait for a link.
So this patch is bogus and by accident fixes a problem. The real problem
is that current phylib does not wait (reliably) for a link. This problem still
persists and is still very annoying. I just have a USB network adapter
here which needs a three second delay after discovering it to make it
work with barebox.
We need a phy_aneg_done() before trying to do network transfers and we
need that *unconditionally*.
Jean-Christophe, you can choose where you want to have this call, but it
will go in. We are carrying these phylib regressions for several months
now.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
