diff mbox

[U-Boot] net: phy: do not read configuration register on reset

Message ID 1449688885-17037-1-git-send-email-stefan@agner.ch
State Accepted
Commit a058052c358c3ecf5f394ff37def6a45eb26768c
Delegated to: Joe Hershberger
Headers show

Commit Message

Stefan Agner Dec. 9, 2015, 7:21 p.m. UTC
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 <stefan@agner.ch>
---
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(-)

Comments

Michael Welling Dec. 9, 2015, 7:32 p.m. UTC | #1
On Wed, Dec 09, 2015 at 11:21:25AM -0800, 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 <stefan@agner.ch>

Acked-by: Michael Welling <mwelling@ieee.org>

> ---
> 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);
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Bin Meng Dec. 10, 2015, 12:52 a.m. UTC | #2
On Thu, Dec 10, 2015 at 3:21 AM, Stefan Agner <stefan@agner.ch> 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 <stefan@agner.ch>
> ---
> 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(-)

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Joe Hershberger Dec. 10, 2015, 8:28 p.m. UTC | #3
Hi Stefan,

On Wed, Dec 9, 2015 at 1:21 PM, Stefan Agner <stefan@agner.ch> 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 <stefan@agner.ch>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Joe Hershberger Jan. 29, 2016, 9:25 p.m. UTC | #4
Hi Stefan,

https://patchwork.ozlabs.org/patch/554780/ was applied to u-boot-net.git.

Thanks!
-Joe
Joe Hershberger Jan. 29, 2016, 9:26 p.m. UTC | #5
Hi Stefan,

https://patchwork.ozlabs.org/patch/554780/ was applied to u-boot-net.git.

Thanks!
-Joe
diff mbox

Patch

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);