From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Tue, 9 Feb 2016 19:42:25 +0100 Subject: [U-Boot] [PATCH] net: phy: do not read configuration register on reset In-Reply-To: <1449688885-17037-1-git-send-email-stefan@agner.ch> References: <1449688885-17037-1-git-send-email-stefan@agner.ch> Message-ID: <56BA3311.30407@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09.12.2015 20:21, Stefan Agner wrote: > When doing a software reset, the reset flag should be written without > other bits set. Writing the current state will lead to restoring the > state of the PHY (e.g. Powerdown), which is not what is expected from > a software reset. > > Signed-off-by: Stefan Agner > --- > This lead to the PHY staying powered down on a reboot when using a > Micrel PHY and not using hardware reset... > > It also aligns with the behavior of Linux: > http://lxr.free-electrons.com/source/drivers/net/phy/phy_device.c#L1122 > > -- > Stefan > > drivers/net/phy/phy.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index 51b5746..ef9f13b 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -717,15 +717,7 @@ int phy_reset(struct phy_device *phydev) > } > #endif > > - reg = phy_read(phydev, devad, MII_BMCR); > - if (reg < 0) { > - debug("PHY status read failed\n"); > - return -1; > - } > - > - reg |= BMCR_RESET; > - > - if (phy_write(phydev, devad, MII_BMCR, reg) < 0) { > + if (phy_write(phydev, devad, MII_BMCR, BMCR_RESET) < 0) { > debug("PHY reset failed\n"); > return -1; > } > @@ -738,6 +730,7 @@ int phy_reset(struct phy_device *phydev) > * auto-clearing). This should happen within 0.5 seconds per the > * IEEE spec. > */ > + reg = phy_read(phydev, devad, MII_BMCR); > while ((reg & BMCR_RESET) && timeout--) { > reg = phy_read(phydev, devad, MII_BMCR); This breaks PHY link auto negotiation support on Marvell Armada 388 ClearFog for me. As with this patch, bit 12 (A/N enable) will get cleared. And the link is not established correctly. The problem here seems to be, that the Marvell PHY driver calls phy_reset() again at the end of m88e1111s_config(). Resulting in the loss of the configuration of the MII_BMCR register. I'm currently thinking if this patchbelow would be the correct fix for this problem? diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index eab1558..8ede927 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -270,8 +270,7 @@ static int m88e1111s_config(struct phy_device *phydev) printf("%s: phy soft reset timeout\n", __func__); genphy_config_aneg(phydev); - - phy_reset(phydev); + genphy_restart_aneg(phydev); Thanks, Stefan