diff mbox series

[U-Boot,v2,2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature

Message ID 1548262809-19361-3-git-send-email-vladimir.oltean@nxp.com
State Superseded
Delegated to: Joe Hershberger
Headers show
Series net: phy: allow disabling SmartEEE for Atheros PHYs | expand

Commit Message

Vladimir Oltean Jan. 23, 2019, 5 p.m. UTC
* 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(+)

Comments

Joe Hershberger Jan. 24, 2019, 6:22 a.m. UTC | #1
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:
>     - 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);

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;
> +       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);
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
diff mbox series

Patch

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