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

[Joe Hershberger]

On Wed, Jan 23, 2019 at 5:47 PM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean at 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