Skip to contentSkip to conversation

drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

PATCH New ken 5 participants 11 messages Feb 2, 2017 – Mar 3, 2017 2 messages not in the archive

Subsystems (2)
Network drivers Ethernet PHY
lore
Share & tools
lore

ken

author
#0 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register to avoid the voltage peak issue while we do the IEEE PHY comformance test

Signed-off-by ken Lin <[email protected]>

Tested on Advantech DMS-BA16 board

Tested-by Ken Lin <[email protected]>
---
 drivers/net/phy/atheros.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/atheros.c +6 -0
index b34cdd3d87..82fe228604 100644
@@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,

@@ -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
 			  AR803x_RGMII_RX_CLK_DLY);
 	}
 
+        phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);
+        regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+        phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval | 0x3C47);
+
 	phydev->supported = phydev->drv->features;
 
 	genphy_config_aneg(phydev);
Signature · 1 line
2.11.0
#1 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 24 lines · ken

Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register to avoid the voltage peak issue while we do the IEEE PHY comformance test

Signed-off-by ken Lin <[email protected]>

Tested on Advantech DMS-BA16 board

Tested-by Ken Lin <[email protected]>
---
 drivers/net/phy/atheros.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/atheros.c +2 -0
index b34cdd3d87..82fe228604 100644
@@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,

drivers/net/phy/atheros.c static int ar8031_config(struct phy_device *phydev) · line 51

Quoted 6 lines
drivers/net/phy/atheros.c +3 -0
@@ -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
 			  AR803x_RGMII_RX_CLK_DLY);
 	}
 
+        phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);
+        regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+        phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval | 0x3C47);

I thought I had responded to this, but probably the mail did not go through.

What you have defeats the purpose of reading back the register. The idea of reading back was to avoid using any hardcoded reset state of the register.

Also, your patch does nothing to the existing code writing to AR803x_DEBUG_REG_5. So you have two unneeded writes now. Does the following work for you (not compiled or tested).

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +5 -1
index b34cdd3d87dc..aff9eb3d18c6 100644
@@ -28,12 +28,16 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
 			  AR803x_DEBUG_REG_5);
+		regval = phy_read(phydev, MDIO_DEVAD_NONE,
+				  AR803x_PHY_DEBUG_DATA_REG);
 		phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-			  AR803x_RGMII_TX_CLK_DLY);
+			  regval | AR803x_RGMII_TX_CLK_DLY);
 	}
 
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

A message that is not in the archive (sent off-list or not yet received): WM!f9618cf32facd23d1ea4e55a864e7ac2d4f6c90b9ab6a5bc400bffdc71f37d617dcce145a695b30549664eaa9cca7fbd!@dg.advantech.com

#3 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 51 lines

-----Original Message-----
From: Sekhar Nori [mailto:nsekhar at ti.com]
Sent: Thursday, February 2, 2017 9:44 PM
To: ken <Ken Lin>; sbabic at denx.de
Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Ken.Lin; Peter.Chiang; Peter.Stretz; akshay.bhat at timesys.com; joe.hershberger at ni.com; albert.u.uboot at aribaud.net
Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 16 lines · ken

Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register to avoid the voltage peak issue while we do the IEEE PHY comformance test

Signed-off-by ken Lin <[email protected]> Tested on Advantech

DMS-BA16 board

Tested-by Ken Lin <[email protected]>
---
 drivers/net/phy/atheros.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/atheros.c +0 -0
index b34cdd3d87..82fe228604 100644

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 34

Quoted 7 lines
drivers/net/phy/atheros.c +2 -0
@@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, @@

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev)

Quoted 5 lines
drivers/net/phy/atheros.c +1 -1
-44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
 			  AR803x_RGMII_RX_CLK_DLY);
 	}
 
+        phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);

Quoted 1 line

+ regval = phy_read(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG);

Quoted 1 line

+ phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG,

Quoted 1 line

+ regval | 0x3C47);

I thought I had responded to this, but probably the mail did not go through.

Quoted 8 lines

What you have defeats the purpose of reading back the register. The idea of reading back was to avoid using any hardcoded reset state of the register.

Also, your patch does nothing to the existing code writing to AR803x_DEBUG_REG_5.
So you have two unneeded writes now. Does the following work for you (not compiled or tested).

The register setting would turn out to be 0x3D47 on our project boards and our signal measurement results show the patch (v2 version, https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue. The proposed solution is to follow the implementation in previous commits, which include the reserved bits settings in SerDes Test and System Mode Control register.

Quoted 31 lines

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c index renamed from drivers/net/phy/atheros.c +0 -0

b34cdd3d87dc..aff9eb3d18c6 100644

drivers/net/phy/atheros.c +5 -1
@@ -28,12 +28,16 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,
 AR803x_PHY_DEBUG_ADDR_REG,
 			  AR803x_DEBUG_REG_5);
+		regval = phy_read(phydev, MDIO_DEVAD_NONE,
+				  AR803x_PHY_DEBUG_DATA_REG);
 		phy_write(phydev, MDIO_DEVAD_NONE,
 AR803x_PHY_DEBUG_DATA_REG,
-			  AR803x_RGMII_TX_CLK_DLY);
+			  regval | AR803x_RGMII_TX_CLK_DLY);
 	}

if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

--
This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.

Signature · 3 lines
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
#4 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 54 lines · Ken.Lin
Quoted 54 lines

-----Original Message-----
From: Sekhar Nori [mailto:nsekhar at ti.com]
Sent: Thursday, February 2, 2017 9:44 PM
To: ken <Ken Lin>; sbabic at denx.de
Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Ken.Lin; Peter.Chiang; Peter.Stretz; akshay.bhat at timesys.com; joe.hershberger at ni.com; albert.u.uboot at aribaud.net
Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 16 lines · ken

Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register to avoid the voltage peak issue while we do the IEEE PHY comformance test

Signed-off-by ken Lin <[email protected]> Tested on Advantech

DMS-BA16 board

Tested-by Ken Lin <[email protected]>
---
 drivers/net/phy/atheros.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/atheros.c +0 -0
index b34cdd3d87..82fe228604 100644

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 34

Quoted 7 lines
drivers/net/phy/atheros.c +2 -0
@@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, @@

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev)

Quoted 5 lines
drivers/net/phy/atheros.c +1 -1
-44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
 			  AR803x_RGMII_RX_CLK_DLY);
 	}
 
+        phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);

Quoted 1 line

+ regval = phy_read(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG);

Quoted 1 line

+ phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG,

Quoted 1 line

+ regval | 0x3C47);

I thought I had responded to this, but probably the mail did not go through.

What you have defeats the purpose of reading back the register. The idea of reading back was to avoid using any hardcoded reset state of the register.

Quoted 8 lines
Quoted 5 lines

Also, your patch does nothing to the existing code writing to AR803x_DEBUG_REG_5.
So you have two unneeded writes now. Does the following work for you (not compiled or tested).

The register setting would turn out to be 0x3D47 on our project boards and our signal measurement results show the patch (v2 version, https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue. The proposed solution is to follow the implementation in previous commits, which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Quoted 26 lines
Quoted 26 lines

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c index renamed from drivers/net/phy/atheros.c +0 -0

b34cdd3d87dc..aff9eb3d18c6 100644

drivers/net/phy/atheros.c +5 -1
@@ -28,12 +28,16 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,
 AR803x_PHY_DEBUG_ADDR_REG,
 			  AR803x_DEBUG_REG_5);
+		regval = phy_read(phydev, MDIO_DEVAD_NONE,
+				  AR803x_PHY_DEBUG_DATA_REG);
 		phy_write(phydev, MDIO_DEVAD_NONE,
 AR803x_PHY_DEBUG_DATA_REG,
-			  AR803x_RGMII_TX_CLK_DLY);
+			  regval | AR803x_RGMII_TX_CLK_DLY);
 	}
Quoted 1 line
Quoted 1 line

if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Thanks,
Sekhar

A message that is not in the archive (sent off-list or not yet received): WM!b0984f49f91c78992075ca3153bf1010d61a480f76f5f59d07c0babd7010153f51dda37c13eae264da77aa3d9f01e73a!@dg.advantech.com

#6 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 78 lines

-----Original Message-----
From: Sekhar Nori [mailto:nsekhar at ti.com]
Sent: Monday, February 6, 2017 12:53 AM
To: Ken.Lin; ken <Ken Lin>; sbabic at denx.de
Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Peter.Chiang; Peter.Stretz; akshay.bhat at timesys.com; joe.hershberger at ni.com;
albert.u.uboot at aribaud.net
Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 46 lines · Ken.Lin
Quoted 46 lines

-----Original Message-----
From: Sekhar Nori [mailto:nsekhar at ti.com]
Sent: Thursday, February 2, 2017 9:44 PM
To: ken <Ken Lin>; sbabic at denx.de
Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Ken.Lin; Peter.Chiang; Peter.Stretz; akshay.bhat at timesys.com; joe.hershberger at ni.com; albert.u.uboot at aribaud.net
Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 16 lines · ken

Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register to avoid the voltage peak issue while we do the IEEE PHY comformance test

Signed-off-by ken Lin <[email protected]> Tested on Advantech

DMS-BA16 board

Tested-by Ken Lin <[email protected]>
---
 drivers/net/phy/atheros.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/net/phy/atheros.c +0 -0
index b34cdd3d87..82fe228604 100644

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device · line 35

Quoted 8 lines
drivers/net/phy/atheros.c +2 -0
@@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device
 *phydev)
 
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, @@

drivers/net/phy/atheros.c

Quoted 5 lines

-44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)

drivers/net/phy/atheros.c +1 -0
 			  AR803x_RGMII_RX_CLK_DLY);
 	}
 
+        phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);

Quoted 1 line

+ regval = phy_read(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG);

Quoted 8 lines
Quoted 8 lines
Quoted 1 line

+ phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG,

Quoted 1 line

+ regval | 0x3C47);

I thought I had responded to this, but probably the mail did not go through.

What you have defeats the purpose of reading back the register. The idea of reading back was to avoid using any hardcoded reset state of the

register.

Quoted 8 lines
Quoted 6 lines

Also, your patch does nothing to the existing code writing to AR803x_DEBUG_REG_5.
So you have two unneeded writes now. Does the following work for you (not compiled or tested).

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 8 lines
Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

When I dig around the commit history, I could find the related config change info for AR8031. The original setting for the reserved bits in SerDes Test and System Mode Control register are not 0 (0x3C47).

commit d584c68ce0f5bf2f430ccfb2ba00bd506206fb91

Author: Fabio Estevam <[email protected]>
Date:   Tue Jan 5 17:02:52 2016 -0200

phy: atheros: Use ar8035_config for AR8031

Commit 08ad9b068afb88 (" ar8031: modify the config func of ar8031 to ar8021_config") selected 'ar8021_config' as the configuration function for AR8031.

The correct would be to use 'ar8035_config' instead as AR8031/AR8035 have the same programming model and even share the same phy driver in the linux kernel: drivers/net/phy/at803x.c.

Tested on a mx6qsabresd and wandboard, which now can work without any PHY setup code in the board files.

Signed-off-by Fabio Estevam <[email protected]>
Acked-by Joe Hershberger <[email protected]>
drivers/net/phy/atheros.c +1 -1
index d509e30..ba57b1a 100644
@@ -51,7 +51,7 @@ static struct phy_driver AR8031_driver =  {
        .uid = 0x4dd074,
        .mask = 0xffffffef,
        .features = PHY_GBIT_FEATURES,
-       .config = ar8021_config,
+       .config = ar8035_config,
        .startup = genphy_startup,
        .shutdown = genphy_shutdown,

};

commit ce412b79e72557702185443501478646a938b5fe

Author: Mugunthan V N <[email protected]>
Date:   Thu Oct 13 19:33:36 2016 +0530

drivers: net: phy: atheros: add separate config for AR8031

In the current driver implementation, config() callback is common for AR8035 and AR8031 phy. In config() callback, driver tries to configure MMD Access Control Register and MMD Access Address Data Register unconditionally for both phy versions which leads to auto negotiation failure in AM335x EVMsk second port which uses AR8031 Giga bit RGMII phy. Fixing this by adding separate config for AR8031 phy.

Reviewed-by Sekhar Nori <[email protected]>
Signed-off-by Mugunthan V N <[email protected]>
Acked-by Joe Hershberger <[email protected]>
drivers/net/phy/atheros.c +0 -0
index 694a338..b34cdd3 100644

@@ -8,6 +8,15 @@
*/
#include <phy.h>

+#define AR803x_PHY_DEBUG_ADDR_REG 0x1d
+#define AR803x_PHY_DEBUG_DATA_REG 0x1e
+
+#define AR803x_DEBUG_REG_5 0x5
+#define AR803x_RGMII_TX_CLK_DLY 0x100
+
+#define AR803x_DEBUG_REG_0 0x0
+#define AR803x_RGMII_RX_CLK_DLY 0x8000
+
static int ar8021_config(struct phy_device *phydev)

{
        phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
@@ -17,6 +26,32 @@ static int ar8021_config(struct phy_device *phydev)
        return 0;
}

+static int ar8031_config(struct phy_device *phydev) +{
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+ AR803x_RGMII_TX_CLK_DLY);
+ }
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_0);
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+ AR803x_RGMII_RX_CLK_DLY);
+ }
+
+ phydev->supported = phydev->drv->features;
+
+ genphy_config_aneg(phydev);
+ genphy_restart_aneg(phydev);
+
+ return 0;
+}
+

When I checked the NXP u-boot codebase (git://git.freescale.com/imx/uboot-imx.git, branch l5.1.1_2.1.0-ga), It seems NXP worked with the PHY vendors and set the reserved bits to 0x3D47.

commit 9082eeac5de1335d663016668c9b89c290f5c79b
Author: Andy Fleming <[email protected]>
Date: Thu Apr 7 21:56:05 2011 -0500
phylib: Add a bunch of PHY drivers from tsec
The tsec driver had a bunch of PHY drivers already written. This converts them all into PHY Lib drivers, and serves as the first set of PHY drivers for PHY Lib.
While doing that, cleaned up a number of magic numbers (though not all of them, as PHY vendors like to keep their numbers as magical as possible). Also, noticed that almost all of the vitesse/cicada PHYs had the same config/parse/startup functions, so those have been collapsed into one.

Signed-off-by Andy Fleming <[email protected]>
Signed-off-by Kumar Gala <[email protected]>
Acked-by Detlev Zundel <[email protected]>

+/*
+ * Atheros PHY drivers
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * author Andy Fleming
+ *
+ */
+#include <phy.h>
+
+static int ar8021_config(struct phy_device *phydev)
+{
+ phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
+ phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
+
+ return 0;
+}
+
+struct phy_driver AR8021_driver = {
+ .name = "AR8021",
+ .uid = 0x4dd040,
+ .mask = 0xfffff0,
+ .features = PHY_GBIT_FEATURES,
+ .config = ar8021_config,
+ .startup = genphy_startup,
+ .shutdown = genphy_shutdown,
+};
+
+int phy_atheros_init(void)
+{
+ phy_register(&AR8021_driver);
+
+ return 0;
+}

commit 626ee1e32eeb4fc89e0f406d6067ed6e71d8302f
Author: Shengzhou Liu <[email protected]>
Date: Thu Aug 8 16:33:35 2013 +0800
phylib: update atheros ar803x phy
As AR8031 and AR8033 have same PHY ID 0x4dd074, they use the common driver. Currently AR8031_driver didn't work for AR8033, hence updated it to have it work on AR8031/AR8033.

Signed-off-by Shengzhou Liu <[email protected]>
drivers/net/phy/atheros.c +0 -0
index 0f2dfd6..7a1453f 100644
@@ -48,11 +48,11 @@ static struct phy_driver AR8021_driver = {

};
static struct phy_driver AR8031_driver = {
- .name = "AR8031",
+ .name = "AR8031/AR8033",
.uid = 0x4dd074,
.mask = 0xfffff0,
.features = PHY_GBIT_FEATURES,
- .config = genphy_config,
+ .config = ar8021_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
};

Quoted 37 lines
Quoted 12 lines
Quoted 12 lines

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +0 -0

index
b34cdd3d87dc..aff9eb3d18c6 100644

drivers/net/phy/atheros.c +0 -0
@@ -28,12 +28,16 @@ static int ar8021_config(struct phy_device
 *phydev)
Quoted 6 lines

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device · line 35

Quoted 6 lines
drivers/net/phy/atheros.c +2 -0
 static int ar8031_config(struct phy_device *phydev)  {
+	int regval;
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
 		phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_ADDR_REG,

Quoted 4 lines

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device · line 39

Quoted 4 lines
drivers/net/phy/atheros.c +2 -0
 			  AR803x_DEBUG_REG_5);
+		regval = phy_read(phydev, MDIO_DEVAD_NONE,
+				  AR803x_PHY_DEBUG_DATA_REG);
 		phy_write(phydev, MDIO_DEVAD_NONE,

AR803x_PHY_DEBUG_DATA_REG,

Quoted 5 lines

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device · line 43

Quoted 5 lines
drivers/net/phy/atheros.c +1 -1
-			  AR803x_RGMII_TX_CLK_DLY);
+			  regval | AR803x_RGMII_TX_CLK_DLY);
 	}
 
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Thanks,
Sekhar

--
This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.

Signature · 3 lines
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
#7 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +7 -1
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
#8 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 54 lines · Sekhar Nori
Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +2 -0
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 45

Quoted 10 lines
drivers/net/phy/atheros.c +5 -1
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Hi ,

Your patch doesn't work for the issue case on our project board. I added more debug messages for your reference.

drivers/net/fec_mxc.c +1 -0
index 3304fddc69..e7a8680a7f 100644
@@ -7,6 +7,7 @@
  *
  * SPDX-License-Identifier:    GPL-2.0+
  */
+#define DEBUG
 
 #include <common.h>
 #include <dm.h>
drivers/net/phy/atheros.c +19 -1
index b34cdd3d87..0d9380f833 100644
@@ -28,12 +28,22 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);
+
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval |
+AR803x_RGMII_TX_CLK_DLY);
+
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

@@ -44,6 +54,21 @@ static int ar8031_config(struct phy_device *phydev)
                          AR803x_RGMII_RX_CLK_DLY);
        }
 
+
+#if 1
+                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+                          AR803x_DEBUG_REG_5);
+
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+

U-Boot 2017.03-rc1-00057-gc83a824e62-dirty (Feb 08 2017 - 02:54:43 +0800)

CPU:   Freescale i.MX6D rev1.5 at 792 MHz
Reset cause: POR
BOARD: General Electric B850v3
I2C:   ready
DRAM:  2 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
SF: Detected n25q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB

*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   eth_init: fec_probe(bd, -1, 4) @ 02188000
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_read: phy: 04 reg:02 val:0x4d
fec_mdio_read: phy: 04 reg:03 val:0xd074
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_write: phy: 04 reg:00 val:0x8000
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:0d val:0x7
fec_mdio_write: phy: 04 reg:0e val:0x8016
fec_mdio_write: phy: 04 reg:0d val:0x4007
fec_mdio_write: phy: 04 reg:0e val:0x18
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_write: phy: 04 reg:1e val:0x100
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_read: phy: 04 reg:1e val:0x100
ar8031_config check: value read back 0x100, written: 0x100
fec_mdio_read: phy: 04 reg:04 val:0x1de1
fec_mdio_write: phy: 04 reg:04 val:0x11e1
fec_mdio_read: phy: 04 reg:01 val:0x7949
fec_mdio_read: phy: 04 reg:09 val:0x200
fec_mdio_write: phy: 04 reg:09 val:0x300
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:00 val:0x1200
fec_mdio_read: phy: 04 reg:00 val:0x1000
fec_mdio_write: phy: 04 reg:00 val:0x1200

FEC [PRIME]
Error: FEC address not set.

Hit any key to stop autoboot: 1 0 =>

#9 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 73 lines · Yung-Ching LIN
Quoted 54 lines · Sekhar Nori
Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +2 -0
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 45

Quoted 10 lines
drivers/net/phy/atheros.c +5 -1
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Hi ,

Your patch doesn't work for the issue case on our project board. I added more debug messages for your reference.

[...]

Quoted 23 lines

U-Boot 2017.03-rc1-00057-gc83a824e62-dirty (Feb 08 2017 - 02:54:43 +0800)

CPU:   Freescale i.MX6D rev1.5 at 792 MHz
Reset cause: POR
BOARD: General Electric B850v3
I2C:   ready
DRAM:  2 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
SF: Detected n25q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB

*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   eth_init: fec_probe(bd, -1, 4) @ 02188000
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_read: phy: 04 reg:02 val:0x4d
fec_mdio_read: phy: 04 reg:03 val:0xd074
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_write: phy: 04 reg:00 val:0x8000
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:0d val:0x7
Quoted 8 lines
fec_mdio_write: phy: 04 reg:0e val:0x8016
fec_mdio_write: phy: 04 reg:0d val:0x4007
fec_mdio_write: phy: 04 reg:0e val:0x18
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_write: phy: 04 reg:1e val:0x100
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_read: phy: 04 reg:1e val:0x100
ar8031_config check: value read back 0x100, written: 0x100

Hmm, so in effect you are forced to use the magic value of 0x3c47 as the reset default when actually it is 0x100 on your board. And there is no backing public documentation on why the reset default should be 0x3c47. And whether it will work for all boards that use this PHY.

Thats pretty unmaintainable in my opinion. If this value does not work for the next person that comes along, we will be in trouble again.

I guess the best thing that can be done at this point is to use this magic reset default value in a board specific way. By specifying it using device-tree, perhaps. I would wait for some feedback from Joe Hershberger though.

BTW, do you have the same problem in kernel ? Because AFAICS, even drivers/net/phy/at803x.c in kernel does not have any such provision for magic reset default values.

Quoted 12 lines

fec_mdio_read: phy: 04 reg:04 val:0x1de1
fec_mdio_write: phy: 04 reg:04 val:0x11e1
fec_mdio_read: phy: 04 reg:01 val:0x7949
fec_mdio_read: phy: 04 reg:09 val:0x200
fec_mdio_write: phy: 04 reg:09 val:0x300
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:00 val:0x1200
fec_mdio_read: phy: 04 reg:00 val:0x1000
fec_mdio_write: phy: 04 reg:00 val:0x1200 FEC [PRIME]
Error: FEC address not set.

Quoted 1 line

Hit any key to stop autoboot: 1 0

Thanks,
Sekhar

#10 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 109 lines · Sekhar Nori
Quoted 73 lines · Yung-Ching LIN
Quoted 54 lines · Sekhar Nori
Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +2 -0
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 45

Quoted 10 lines
drivers/net/phy/atheros.c +5 -1
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Hi ,

Your patch doesn't work for the issue case on our project board. I added more debug messages for your reference.

[...]

Quoted 23 lines

U-Boot 2017.03-rc1-00057-gc83a824e62-dirty (Feb 08 2017 - 02:54:43 +0800)

CPU:   Freescale i.MX6D rev1.5 at 792 MHz
Reset cause: POR
BOARD: General Electric B850v3
I2C:   ready
DRAM:  2 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
SF: Detected n25q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB

*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   eth_init: fec_probe(bd, -1, 4) @ 02188000
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_read: phy: 04 reg:02 val:0x4d
fec_mdio_read: phy: 04 reg:03 val:0xd074
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_write: phy: 04 reg:00 val:0x8000
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:0d val:0x7
Quoted 8 lines
fec_mdio_write: phy: 04 reg:0e val:0x8016
fec_mdio_write: phy: 04 reg:0d val:0x4007
fec_mdio_write: phy: 04 reg:0e val:0x18
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_write: phy: 04 reg:1e val:0x100
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_read: phy: 04 reg:1e val:0x100
ar8031_config check: value read back 0x100, written: 0x100
Quoted 4 lines

Hmm, so in effect you are forced to use the magic value of 0x3c47 as the reset default when actually it is 0x100 on your board. And there is no backing public documentation on why the reset default should be 0x3c47. And whether it will work for all boards that use this PHY.

Well that's quite unfortunate.

Quoted 7 lines

Thats pretty unmaintainable in my opinion. If this value does not work for the next person that comes along, we will be in trouble again.

I guess the best thing that can be done at this point is to use this magic reset default value in a board specific way. By specifying it using device-tree, perhaps. I would wait for some feedback from Joe Hershberger though.

I think we can wait until someone else says they have a problem with this value before making it a board option.

Quoted 3 lines

BTW, do you have the same problem in kernel ? Because AFAICS, even drivers/net/phy/at803x.c in kernel does not have any such provision for magic reset default values.

I'm interested to know this too. Is the kernel depending on the bootloader to set this and it just avoids changing it?

Quoted 20 lines
Quoted 12 lines

fec_mdio_read: phy: 04 reg:04 val:0x1de1
fec_mdio_write: phy: 04 reg:04 val:0x11e1
fec_mdio_read: phy: 04 reg:01 val:0x7949
fec_mdio_read: phy: 04 reg:09 val:0x200
fec_mdio_write: phy: 04 reg:09 val:0x300
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:00 val:0x1200
fec_mdio_read: phy: 04 reg:00 val:0x1000
fec_mdio_write: phy: 04 reg:00 val:0x1200 FEC [PRIME]
Error: FEC address not set.

Quoted 1 line

Hit any key to stop autoboot: 1 0

Thanks,
Sekhar
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#11 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 128 lines · Joe Hershberger
Quoted 109 lines · Sekhar Nori
Quoted 73 lines · Yung-Ching LIN
Quoted 54 lines · Sekhar Nori
Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +2 -0
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 45

Quoted 10 lines
drivers/net/phy/atheros.c +5 -1
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
 
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Hi ,

Your patch doesn't work for the issue case on our project board. I added more debug messages for your reference.

[...]

Quoted 23 lines

U-Boot 2017.03-rc1-00057-gc83a824e62-dirty (Feb 08 2017 - 02:54:43 +0800)

CPU:   Freescale i.MX6D rev1.5 at 792 MHz
Reset cause: POR
BOARD: General Electric B850v3
I2C:   ready
DRAM:  2 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
SF: Detected n25q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB

*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   eth_init: fec_probe(bd, -1, 4) @ 02188000
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_read: phy: 04 reg:02 val:0x4d
fec_mdio_read: phy: 04 reg:03 val:0xd074
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_write: phy: 04 reg:00 val:0x8000
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:0d val:0x7
Quoted 8 lines
fec_mdio_write: phy: 04 reg:0e val:0x8016
fec_mdio_write: phy: 04 reg:0d val:0x4007
fec_mdio_write: phy: 04 reg:0e val:0x18
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_write: phy: 04 reg:1e val:0x100
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_read: phy: 04 reg:1e val:0x100
ar8031_config check: value read back 0x100, written: 0x100
Quoted 4 lines

Hmm, so in effect you are forced to use the magic value of 0x3c47 as the reset default when actually it is 0x100 on your board. And there is no backing public documentation on why the reset default should be 0x3c47. And whether it will work for all boards that use this PHY.

Well that's quite unfortunate.

Quoted 7 lines

Thats pretty unmaintainable in my opinion. If this value does not work for the next person that comes along, we will be in trouble again.

I guess the best thing that can be done at this point is to use this magic reset default value in a board specific way. By specifying it using device-tree, perhaps. I would wait for some feedback from Joe Hershberger though.

I think we can wait until someone else says they have a problem with this value before making it a board option.

Quoted 7 lines
Quoted 3 lines

BTW, do you have the same problem in kernel ? Because AFAICS, even drivers/net/phy/at803x.c in kernel does not have any such provision for magic reset default values.

I'm interested to know this too. Is the kernel depending on the bootloader to set this and it just avoids changing it?

Yes, I think so.Here are the code snippets in drivers/net/phy/at803x.c. I found it wouldn't change the bootloader setting when I dumped the value in kernel.

static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,

                                 u16 clear, u16 set)
{
        u16 val;
        int ret;

        ret = at803x_debug_reg_read(phydev, reg);
        if (ret < 0)
                return ret;

        val = ret & 0xffff;
        val &= ~clear;
        val |= set;

        return phy_write(phydev, AT803X_DEBUG_DATA, val);
}

static inline int at803x_enable_tx_delay(struct phy_device *phydev)

{
        return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
                                        AT803X_DEBUG_TX_CLK_DLY_EN);
}
Quoted 20 lines
Quoted 20 lines
Quoted 12 lines

fec_mdio_read: phy: 04 reg:04 val:0x1de1
fec_mdio_write: phy: 04 reg:04 val:0x11e1
fec_mdio_read: phy: 04 reg:01 val:0x7949
fec_mdio_read: phy: 04 reg:09 val:0x200
fec_mdio_write: phy: 04 reg:09 val:0x300
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:00 val:0x1200
fec_mdio_read: phy: 04 reg:00 val:0x1000
fec_mdio_write: phy: 04 reg:00 val:0x1200 FEC [PRIME]
Error: FEC address not set.

Quoted 1 line

Hit any key to stop autoboot: 1 0

Thanks,
Sekhar
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#12 lore raw

PATCH 1/1 drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Quoted 67 lines · Yung-Ching LIN
Quoted 65 lines · Joe Hershberger
Quoted 64 lines · Sekhar Nori
Quoted 63 lines · Yung-Ching LIN
Quoted 62 lines · Sekhar Nori
Quoted 10 lines · Ken.Lin
Quoted 10 lines
Quoted 1 line

The register setting would turn out to be 0x3D47 on our project boards and

our signal measurement results show the patch (v2 version,
https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.

Quoted 1 line

The proposed solution is to follow the implementation in previous commits,

which include the reserved bits settings in SerDes Test and System Mode Control register.

So what does the register setting turn out to be with my patch below ?

What are the "previous commits" you refer to ?

Thanks for the references to the commits. You left out answering my question about what register settings you see with my patch.

I have included another patch now with some debug enabled. Can you apply this patch to latest u-boot master, run on your board and send me the log ? Here is what I see on AM335x EVM-SK:

U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530)

CPU  : AM335X-GP rev 1.0
Model: TI AM335x EVM-SK
DRAM:  256 MiB
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1

reading uboot.env
ERROR: No USB device found

at ../drivers/usb/gadget/ether.c:2709/usb_ether_init()

Net:   ar8031_config: value read back 0x2c47, written: 0x2d47
eth0: ethernet at 4a100000
Hit any key to stop autoboot:  0

Thanks,
Sekhar

---8<---

drivers/net/phy/atheros.c +7 -1
index b34cdd3d87dc..5c0a36676ce9 100644
@@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev)
 
 static int ar8031_config(struct phy_device *phydev)
 {
+       int regval;
+
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
            phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
                          AR803x_DEBUG_REG_5);
+               regval = phy_read(phydev, MDIO_DEVAD_NONE,
+                                 AR803x_PHY_DEBUG_DATA_REG);
+               printf("%s: value read back 0x%x, written: 0x%x\n",
+                               __func__, regval, regval | AR803x_RGMII_TX_CLK_DLY);
                phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
-                         AR803x_RGMII_TX_CLK_DLY);
+                         regval | AR803x_RGMII_TX_CLK_DLY);
        }
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/net/phy/atheros.c static int ar8021_config(struct phy_device *phydev) · line 49

Quoted 2 lines
drivers/net/phy/atheros.c +0 -0
        if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||

Your read-modify-write patch makes sense to me at some point.
I added some debug messages in the u-boot source.
ar8031_config gets called after the board specific mx6_rgmii_rework callback In our board test case, the phydev interface uses PHY_INTERFACE_MODE_RGMII, so it won't hit the condition and run the code statements you implemented.

U-Boot 2016.11-g922cf19526-dirty (Mar 03 2017 - 07:13:39 +0800)

CPU:   Freescale i.MX6D rev1.5 at 792 MHz
Reset cause: POR
BOARD: General Electric B850v3
I2C:   ready
DRAM:  2 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
SF: Detected N25Q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB

*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   eth_init: fec_probe(bd, -1, 4) @ 02188000
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_read: phy: 04 reg:02 val:0x4d
fec_mdio_read: phy: 04 reg:03 val:0xd074
fec_mii_setspeed: mii_speed 0000001a
fec_mdio_write: phy: 04 reg:00 val:0x8000
fec_mdio_read: phy: 04 reg:00 val:0x0

++mx6_rgmii_rework
fec_mdio_write: phy: 04 reg:0d val:0x7
fec_mdio_write: phy: 04 reg:0e val:0x8016
fec_mdio_write: phy: 04 reg:0d val:0x4007
fec_mdio_write: phy: 04 reg:0e val:0x18
fec_mdio_write: phy: 04 reg:1d val:0x5
fec_mdio_write: phy: 04 reg:1e val:0x3d47
--mx6_rgmii_rework
++ar8031_config
fec_mdio_read: phy: 04 reg:04 val:0x1de1
fec_mdio_write: phy: 04 reg:04 val:0x11e1
fec_mdio_read: phy: 04 reg:01 val:0x7949
fec_mdio_read: phy: 04 reg:09 val:0x200
fec_mdio_write: phy: 04 reg:09 val:0x300
fec_mdio_read: phy: 04 reg:00 val:0x0
fec_mdio_write: phy: 04 reg:00 val:0x1200
fec_mdio_read: phy: 04 reg:00 val:0x1000
fec_mdio_write: phy: 04 reg:00 val:0x1200
--ar8031_config
FEC [PRIME]
Error: FEC address not set.

Quoted 11 lines
Quoted 11 lines
Quoted 5 lines

Hmm, so in effect you are forced to use the magic value of 0x3c47 as the reset default when actually it is 0x100 on your board. And there is no backing public documentation on why the reset default should be 0x3c47. And whether it will work for all boards that use this PHY.

Well that's quite unfortunate.

Quoted 3 lines

Thats pretty unmaintainable in my opinion. If this value does not work for the next person that comes along, we will be in trouble again.

Quoted 8 lines
Quoted 8 lines
Quoted 4 lines

I guess the best thing that can be done at this point is to use this magic reset default value in a board specific way. By specifying it using device-tree, perhaps. I would wait for some feedback from Joe Hershberger though.

I think we can wait until someone else says they have a problem with this value before making it a board option.

Sorry for causing the confusion.
Our board specific requirement(issue) for setting proper register value could be done in the board file.
I resubmitted the patch at
https://lists.denx.de/pipermail/u-boot/2017-February/281894.html

Quoted 47 lines
Quoted 7 lines
Quoted 3 lines

BTW, do you have the same problem in kernel ? Because AFAICS, even drivers/net/phy/at803x.c in kernel does not have any such provision for magic reset default values.

I'm interested to know this too. Is the kernel depending on the bootloader to set this and it just avoids changing it?

Yes, I think so.Here are the code snippets in drivers/net/phy/at803x.c. I found it wouldn't change the bootloader setting when I dumped the value in kernel.

static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,

                                 u16 clear, u16 set)
{
        u16 val;
        int ret;

        ret = at803x_debug_reg_read(phydev, reg);
        if (ret < 0)
                return ret;

        val = ret & 0xffff;
        val &= ~clear;
        val |= set;

        return phy_write(phydev, AT803X_DEBUG_DATA, val);
}

static inline int at803x_enable_tx_delay(struct phy_device *phydev)

{
        return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
                                        AT803X_DEBUG_TX_CLK_DLY_EN);
}
Quoted 7 lines
Quoted 7 lines

Thanks,
Sekhar
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot