[U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs
Changes in v2: * Removed patch 1/2 (promote phy_{read,write}_mmd_indirect from ti.c to generic code) * Adapted to PHY API changes introduced by Carlo Caione * Cleaned up Atheros PHY driver to remove duplicated code and use macros where possible
Vladimir Oltean (3): net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature net: phy: ar803x: Use common functions for RGMII internal delays
drivers/net/phy/Kconfig | 21 +++++++++ drivers/net/phy/atheros.c | 110 +++++++++++++++++++++++++++++++--------------- 2 files changed, 95 insertions(+), 36 deletions(-)
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com --- Changes in v2: * Patch added in this version.
drivers/net/phy/atheros.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d15..b4066e3 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
- phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); + /* CLK_25M output clock select: 125 MHz */ + regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); + phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com
Changes in v2:
- Patch added in this version.
drivers/net/phy/atheros.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d15..b4066e3 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
/* CLK_25M output clock select: 125 MHz */regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016);phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);
I think I see how the old code accomplished the same thing. It was woefully undocumented. Can you improve on the magic 0x18 number? What about 0x8016? Is that a register whose name you know and can assign a constant?
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 1/24/19 8:16 AM, Joe Hershberger wrote:
On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com
Changes in v2:
- Patch added in this version.
drivers/net/phy/atheros.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d15..b4066e3 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
/* CLK_25M output clock select: 125 MHz */regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016);phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);I think I see how the old code accomplished the same thing. It was woefully undocumented. Can you improve on the magic 0x18 number? What about 0x8016? Is that a register whose name you know and can assign a constant?
Register MMD7 8016 has no name in the documentation that I can see. It is simply referred to as that. I'd rather keep it like this rather than create confusion by making up a name for it.
-Vladimir
On Thu, Jan 24, 2019 at 6:35 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
On 1/24/19 8:16 AM, Joe Hershberger wrote:
On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com
Changes in v2:
- Patch added in this version.
drivers/net/phy/atheros.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d15..b4066e3 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
/* CLK_25M output clock select: 125 MHz */regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016);phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);I think I see how the old code accomplished the same thing. It was woefully undocumented. Can you improve on the magic 0x18 number? What about 0x8016? Is that a register whose name you know and can assign a constant?
Register MMD7 8016 has no name in the documentation that I can see. It is simply referred to as that. I'd rather keep it like this rather than create confusion by making up a name for it.
OK
-Vladimir _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
* According to the AR8031 and AR8035 datasheets, smartEEE mode (active by default) makes the PHY enter sleep after a configurable idle time. It does this autonomously, without LPI (Low Power Idle) signals coming from MAC. AR8021 does not appear to support this. * This patch allows disabling the SmartEEE feature of above PHYs. * Tested with ping (default of 1 second interval) over back-to-back RGMII between 2 boards having AR8035 at both ends: - Without SmartEEE: 225 packets transmitted, 145 received, 35% packet loss, time 229334ms - With SmartEEE: 144 packets transmitted, 144 received, 0% packet loss, time 146378ms
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com Acked-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v2: * Adapted to use phy_read_mmd and phy_write_mmd functions.
drivers/net/phy/Kconfig | 21 +++++++++++++++++++++ drivers/net/phy/atheros.c | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 3dc0822..6abe8c5 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -94,6 +94,27 @@ config PHY_AQUANTIA_FW_NAME config PHY_ATHEROS bool "Atheros Ethernet PHYs support"
+config PHY_ATHEROS_SMART_EEE + depends on PHY_ATHEROS + default y + tristate "SmartEEE feature for Atheros PHYs" + help + Enables the Atheros SmartEEE feature (not IEEE 802.3az). When 2 PHYs + which support this feature are connected back-to-back, they may + negotiate a low-power sleep mode autonomously, without the Ethernet + controller's knowledge. This setting may cause issues under 2 known + circumstances (both noticed at low traffic rates): + - If the voltage rails on the PHY are unstable, then the PHY can + actually reset as it enters the low power state. This means that + the frames it is supposed to buffer until it wakes up are going + to be dropped instead. + - If 1588/PTP synchronization is the only traffic source over this + PHY, the delays caused by the sleep/wakeup time are going to add + to the synchronization error between the master and the slave. + Default y, which means that the PHY's out-of-reset state is not + changed (SmartEEE active). To work around the issues described + above, change to n. + config PHY_BROADCOM bool "Broadcom Ethernet PHYs support"
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index b4066e3..750c11b 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -5,6 +5,7 @@ * Copyright 2011, 2013 Freescale Semiconductor, Inc. * author Andy Fleming */ +#include <linux/bitops.h> #include <common.h> #include <phy.h>
@@ -17,6 +18,21 @@ #define AR803x_DEBUG_REG_0 0x0 #define AR803x_RGMII_RX_CLK_DLY 0x8000
+#define AR803X_LPI_EN BIT(8) + +static void ar803x_enable_smart_eee(struct phy_device *phydev, bool on) +{ + int regval; + + /* 5.1.11 Smart_eee control3 */ + regval = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x805D); + if (on) + regval |= AR803X_LPI_EN; + else + regval &= ~AR803X_LPI_EN; + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x805D, regval); +} + static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200); @@ -29,6 +45,11 @@ static int ar8021_config(struct phy_device *phydev)
static int ar8031_config(struct phy_device *phydev) { +#ifdef CONFIG_PHY_ATHEROS_SMART_EEE + ar803x_enable_smart_eee(phydev, true); +#else + ar803x_enable_smart_eee(phydev, false); +#endif 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, @@ -57,6 +78,11 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
+#ifdef CONFIG_PHY_ATHEROS_SMART_EEE + ar803x_enable_smart_eee(phydev, true); +#else + ar803x_enable_smart_eee(phydev, false); +#endif /* CLK_25M output clock select: 125 MHz */ regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);
On Wed, Jan 23, 2019 at 5:45 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
- According to the AR8031 and AR8035 datasheets, smartEEE mode (active by default) makes the PHY enter sleep after a configurable idle time. It does this autonomously, without LPI (Low Power Idle) signals coming from MAC. AR8021 does not appear to support this.
- This patch allows disabling the SmartEEE feature of above PHYs.
- Tested with ping (default of 1 second interval) over back-to-back RGMII between 2 boards having AR8035 at both ends:
225 packets transmitted, 145 received, 35% packet loss, time 229334ms
- Without SmartEEE:
144 packets transmitted, 144 received, 0% packet loss, time 146378ms
- With SmartEEE:
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com Acked-by: Joe Hershberger joe.hershberger@ni.com
Changes in v2:
- Adapted to use phy_read_mmd and phy_write_mmd functions.
drivers/net/phy/Kconfig | 21 +++++++++++++++++++++ drivers/net/phy/atheros.c | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 3dc0822..6abe8c5 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -94,6 +94,27 @@ config PHY_AQUANTIA_FW_NAME config PHY_ATHEROS bool "Atheros Ethernet PHYs support"
+config PHY_ATHEROS_SMART_EEE
depends on PHY_ATHEROSdefault ytristate "SmartEEE feature for Atheros PHYs"helpEnables the Atheros SmartEEE feature (not IEEE 802.3az). When 2 PHYswhich support this feature are connected back-to-back, they maynegotiate a low-power sleep mode autonomously, without the Ethernetcontroller's knowledge. This setting may cause issues under 2 knowncircumstances (both noticed at low traffic rates):- If the voltage rails on the PHY are unstable, then the PHY canactually reset as it enters the low power state. This means thatthe frames it is supposed to buffer until it wakes up are goingto be dropped instead.- If 1588/PTP synchronization is the only traffic source over thisPHY, the delays caused by the sleep/wakeup time are going to addto the synchronization error between the master and the slave.Default y, which means that the PHY's out-of-reset state is notchanged (SmartEEE active). To work around the issues describedabove, change to n.config PHY_BROADCOM bool "Broadcom Ethernet PHYs support"
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index b4066e3..750c11b 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -5,6 +5,7 @@
- Copyright 2011, 2013 Freescale Semiconductor, Inc.
- author Andy Fleming
*/ +#include <linux/bitops.h> #include <common.h> #include <phy.h>
@@ -17,6 +18,21 @@ #define AR803x_DEBUG_REG_0 0x0 #define AR803x_RGMII_RX_CLK_DLY 0x8000
+#define AR803X_LPI_EN BIT(8)
+static void ar803x_enable_smart_eee(struct phy_device *phydev, bool on) +{
int regval;/* 5.1.11 Smart_eee control3 */regval = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x805D);
Can you add a #define for this or comment where it comes from / what it means in a data sheet or both?
if (on)regval |= AR803X_LPI_EN;elseregval &= ~AR803X_LPI_EN;phy_write_mmd(phydev, MDIO_MMD_PCS, 0x805D, regval);+}
static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200); @@ -29,6 +45,11 @@ static int ar8021_config(struct phy_device *phydev)
static int ar8031_config(struct phy_device *phydev) { +#ifdef CONFIG_PHY_ATHEROS_SMART_EEE
ar803x_enable_smart_eee(phydev, true);+#else
ar803x_enable_smart_eee(phydev, false);+#endif 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, @@ -57,6 +78,11 @@ static int ar8035_config(struct phy_device *phydev) { int regval;
+#ifdef CONFIG_PHY_ATHEROS_SMART_EEE
ar803x_enable_smart_eee(phydev, true);+#else
ar803x_enable_smart_eee(phydev, false);+#endif /* CLK_25M output clock select: 125 MHz */ regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018); -- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com --- Changes in v2: * Patch added in this version.
drivers/net/phy/atheros.c | 76 ++++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 31 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 750c11b..9eb40f7 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -13,10 +13,10 @@ #define AR803x_PHY_DEBUG_DATA_REG 0x1e
#define AR803x_DEBUG_REG_5 0x5 -#define AR803x_RGMII_TX_CLK_DLY 0x100 +#define AR803x_RGMII_TX_CLK_DLY BIT(8)
#define AR803x_DEBUG_REG_0 0x0 -#define AR803x_RGMII_RX_CLK_DLY 0x8000 +#define AR803x_RGMII_RX_CLK_DLY BIT(15)
#define AR803X_LPI_EN BIT(8)
@@ -33,11 +33,40 @@ static void ar803x_enable_smart_eee(struct phy_device *phydev, bool on) phy_write_mmd(phydev, MDIO_MMD_PCS, 0x805D, regval); }
+static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on) +{ + int regval; + + phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, + AR803x_DEBUG_REG_0); + regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG); + if (on) + regval |= AR803x_RGMII_RX_CLK_DLY; + else + regval &= ~AR803x_RGMII_RX_CLK_DLY; + phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval); +} + +static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on) +{ + int regval; + + 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); + if (on) + regval |= AR803x_RGMII_TX_CLK_DLY; + else + regval &= ~AR803x_RGMII_TX_CLK_DLY; + phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval); +} + static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47); + 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, 0x3D47);
phydev->supported = phydev->drv->features; return 0; @@ -51,20 +80,12 @@ static int ar8031_config(struct phy_device *phydev) ar803x_enable_smart_eee(phydev, false); #endif 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); - } + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) + ar803x_enable_tx_delay(phydev, true);
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->interface == PHY_INTERFACE_MODE_RGMII_ID) + ar803x_enable_rx_delay(phydev, true);
phydev->supported = phydev->drv->features;
@@ -87,25 +108,18 @@ static int ar8035_config(struct phy_device *phydev) regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);
- phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e); - phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100)); + phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, 0x05); + regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG); + phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, + regval | 0x0100);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || - (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) { - /* select debug reg 5 */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x5); - /* enable tx delay */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x0100); - } + (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) + ar803x_enable_tx_delay(phydev, true);
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) || - (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) { - /* select debug reg 0 */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x0); - /* enable rx delay */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x8000); - } + (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) + ar803x_enable_rx_delay(phydev, true);
phydev->supported = phydev->drv->features;
On Wed, Jan 23, 2019 at 5:47 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
Signed-off-by: Vladimir Oltean vladimir.oltean@nxp.com
Changes in v2:
- Patch added in this version.
drivers/net/phy/atheros.c | 76 ++++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 31 deletions(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 750c11b..9eb40f7 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -13,10 +13,10 @@ #define AR803x_PHY_DEBUG_DATA_REG 0x1e
#define AR803x_DEBUG_REG_5 0x5 -#define AR803x_RGMII_TX_CLK_DLY 0x100 +#define AR803x_RGMII_TX_CLK_DLY BIT(8)
#define AR803x_DEBUG_REG_0 0x0 -#define AR803x_RGMII_RX_CLK_DLY 0x8000 +#define AR803x_RGMII_RX_CLK_DLY BIT(15)
#define AR803X_LPI_EN BIT(8)
@@ -33,11 +33,40 @@ static void ar803x_enable_smart_eee(struct phy_device *phydev, bool on) phy_write_mmd(phydev, MDIO_MMD_PCS, 0x805D, regval); }
+static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on) +{
int regval;phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,AR803x_DEBUG_REG_0);regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);if (on)regval |= AR803x_RGMII_RX_CLK_DLY;elseregval &= ~AR803x_RGMII_RX_CLK_DLY;phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);+}
+static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on) +{
int regval;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);if (on)regval |= AR803x_RGMII_TX_CLK_DLY;elseregval &= ~AR803x_RGMII_TX_CLK_DLY;phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);+}
static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
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, 0x3D47);
While this patch is an improvement on the former state, can we add a comment on the magic number or define a constant?
phydev->supported = phydev->drv->features; return 0;@@ -51,20 +80,12 @@ static int ar8031_config(struct phy_device *phydev) ar803x_enable_smart_eee(phydev, false); #endif 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);}
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)ar803x_enable_tx_delay(phydev, true); 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->interface == PHY_INTERFACE_MODE_RGMII_ID)ar803x_enable_rx_delay(phydev, true); phydev->supported = phydev->drv->features;@@ -87,25 +108,18 @@ static int ar8035_config(struct phy_device *phydev) regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018);
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG, 0x05);
What is 0x5?
regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,regval | 0x0100);
What is 0x100?
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {/* select debug reg 5 */phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x5);/* enable tx delay */phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x0100);}
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))ar803x_enable_tx_delay(phydev, true); if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) {/* select debug reg 0 */phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x0);/* enable rx delay */phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x8000);}
(phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))ar803x_enable_rx_delay(phydev, true); phydev->supported = phydev->drv->features;-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 1/24/19 8:20 AM, Joe Hershberger wrote:
On Wed, Jan 23, 2019 at 5:47 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
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, 0x3D47);While this patch is an improvement on the former state, can we add a comment on the magic number or define a constant?
I have no idea what 0x3D47 is, sorry. I couldn't find any public AR8021 register map either. For the other PHYs in this family, debug register 5 holds TX_CLK_DELAY at bit 8. No idea of the implications of writing the other bits. Will act on the rest of your comments.
-Vladimir
On Thu, Jan 24, 2019 at 6:35 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
On 1/24/19 8:20 AM, Joe Hershberger wrote:
On Wed, Jan 23, 2019 at 5:47 PM Vladimir Oltean vladimir.oltean@nxp.com wrote:
static int ar8021_config(struct phy_device *phydev) { phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
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, 0x3D47);While this patch is an improvement on the former state, can we add a comment on the magic number or define a constant?
I have no idea what 0x3D47 is, sorry. I couldn't find any public AR8021 register map either. For the other PHYs in this family, debug register 5 holds TX_CLK_DELAY at bit 8. No idea of the implications of writing the other bits. Will act on the rest of your comments.
OK, thanks.
-Vladimir _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (2)
-
Joe Hershberger -
Vladimir Oltean