* [U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs
@ 2019-01-23 17:00 Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions Vladimir Oltean
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-23 17:00 UTC (permalink / raw)
To: u-boot
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(-)
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions
2019-01-23 17:00 [U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs Vladimir Oltean
@ 2019-01-23 17:00 ` Vladimir Oltean
2019-01-24 6:24 ` Joe Hershberger
2019-01-23 17:00 ` [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays Vladimir Oltean
2 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-23 17:00 UTC (permalink / raw)
To: u-boot
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);
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature
2019-01-23 17:00 [U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions Vladimir Oltean
@ 2019-01-23 17:00 ` Vladimir Oltean
2019-01-24 6:22 ` Joe Hershberger
2019-01-23 17:00 ` [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays Vladimir Oltean
2 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-23 17:00 UTC (permalink / raw)
To: 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);
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays
2019-01-23 17:00 [U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature Vladimir Oltean
@ 2019-01-23 17:00 ` Vladimir Oltean
2019-01-24 6:20 ` Joe Hershberger
2 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-23 17:00 UTC (permalink / raw)
To: 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;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays
2019-01-23 17:00 ` [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays Vladimir Oltean
@ 2019-01-24 6:20 ` Joe Hershberger
2019-01-24 21:08 ` Vladimir Oltean
0 siblings, 1 reply; 11+ messages in thread
From: Joe Hershberger @ 2019-01-24 6:20 UTC (permalink / raw)
To: u-boot
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;
> + 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);
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature
2019-01-23 17:00 ` [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature Vladimir Oltean
@ 2019-01-24 6:22 ` Joe Hershberger
0 siblings, 0 replies; 11+ messages in thread
From: Joe Hershberger @ 2019-01-24 6:22 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions
2019-01-23 17:00 ` [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions Vladimir Oltean
@ 2019-01-24 6:24 ` Joe Hershberger
2019-01-24 21:18 ` Vladimir Oltean
0 siblings, 1 reply; 11+ messages in thread
From: Joe Hershberger @ 2019-01-24 6:24 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays
2019-01-24 6:20 ` Joe Hershberger
@ 2019-01-24 21:08 ` Vladimir Oltean
2019-01-25 20:16 ` Joe Hershberger
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-24 21:08 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions
2019-01-24 6:24 ` Joe Hershberger
@ 2019-01-24 21:18 ` Vladimir Oltean
2019-01-25 20:15 ` Joe Hershberger
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2019-01-24 21:18 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions
2019-01-24 21:18 ` Vladimir Oltean
@ 2019-01-25 20:15 ` Joe Hershberger
0 siblings, 0 replies; 11+ messages in thread
From: Joe Hershberger @ 2019-01-25 20:15 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays
2019-01-24 21:08 ` Vladimir Oltean
@ 2019-01-25 20:16 ` Joe Hershberger
0 siblings, 0 replies; 11+ messages in thread
From: Joe Hershberger @ 2019-01-25 20:16 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-01-25 20:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-23 17:00 [U-Boot] [PATCH v2 0/3] net: phy: allow disabling SmartEEE for Atheros PHYs Vladimir Oltean
2019-01-23 17:00 ` [U-Boot] [PATCH v2 1/3] net: phy: ar803x: Use phy_read_mmd and phy_write_mmd functions Vladimir Oltean
2019-01-24 6:24 ` Joe Hershberger
2019-01-24 21:18 ` Vladimir Oltean
2019-01-25 20:15 ` Joe Hershberger
2019-01-23 17:00 ` [U-Boot] [PATCH v2 2/3] net: phy: ar803x: Address packet drops at low traffic rate due to SmartEEE feature Vladimir Oltean
2019-01-24 6:22 ` Joe Hershberger
2019-01-23 17:00 ` [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays Vladimir Oltean
2019-01-24 6:20 ` Joe Hershberger
2019-01-24 21:08 ` Vladimir Oltean
2019-01-25 20:16 ` Joe Hershberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.