Re: [U-Boot] [PATCH v2 3/3] net: phy: ar803x: Use common functions for RGMII internal delays

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@lists.denx.de https://lists.denx.de/listinfo/u-boot