[U-Boot] [PATCH v5 1/3] net: phy: Add generic helpers to access MMD PHY registers

[Joe Hershberger]

On Fri, Feb 8, 2019 at 11:29 AM Carlo Caione <ccaione at baylibre.com> wrote:
>
> Two new helper functions (phy_read_mmd() and phy_write_mmd()) are added
> to allow access to the MMD PHY registers.
>
> The MMD PHY registers can be accessed by several means:
>
> 1. Using two new MMD access function hooks in the PHY driver. These
> functions can be implemented when the PHY driver does not support the
> standard IEEE Compatible clause 45 access mechanism described in clause
> 22 or if the PHY uses its own non-standard access mechanism.
>
> 2. Direct access for C45 PHYs and C22 PHYs when accessing the reachable
> DEVADs.
>
> 3. The standard clause 45 access extensions to the MMD registers through
> the indirection registers (clause 22) in all the other cases.
>
> Signed-off-by: Carlo Caione <ccaione at baylibre.com>

Except for the off-by-ones below...
Acked-by: Joe Hershberger <joe.hershberger at ni.com>

> ---
>  drivers/net/phy/phy.c |  4 +++
>  include/phy.h         | 70 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index cda4caa803..6769047407 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -549,6 +549,10 @@ int phy_register(struct phy_driver *drv)
>                 drv->readext += gd->reloc_off;
>         if (drv->writeext)
>                 drv->writeext += gd->reloc_off;
> +       if (drv->read_mmd)
> +               drv->read_mmd += gd->reloc_off;
> +       if (drv->write_mmd)
> +               drv->write_mmd += gd->reloc_off;
>  #endif
>         return 0;
>  }
> diff --git a/include/phy.h b/include/phy.h
> index b86fdfb2ce..7ec2b4e86c 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -101,6 +101,14 @@ struct phy_driver {
>         int (*readext)(struct phy_device *phydev, int addr, int devad, int reg);
>         int (*writeext)(struct phy_device *phydev, int addr, int devad, int reg,
>                         u16 val);
> +
> +       /* Phy specific driver override for reading a MMD register */
> +       int (*read_mmd)(struct phy_device *phydev, int devad, int reg);
> +
> +       /* Phy specific driver override for writing a MMD register */
> +       int (*write_mmd)(struct phy_device *phydev, int devad, int reg,
> +                        u16 val);
> +
>         struct list_head list;
>  };
>
> @@ -164,6 +172,68 @@ static inline int phy_write(struct phy_device *phydev, int devad, int regnum,
>         return bus->write(bus, phydev->addr, devad, regnum, val);
>  }
>
> +static inline void phy_mmd_start_indirect(struct phy_device *phydev, int devad,
> +                                         int regnum)
> +{
> +       /* Write the desired MMD Devad */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL, devad);
> +
> +       /* Write the desired MMD register address */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, regnum);
> +
> +       /* Select the Function : DATA with no post increment */
> +       phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_CTRL,
> +                 (devad | MII_MMD_CTRL_NOINCR));
> +}
> +
> +static inline int phy_read_mmd(struct phy_device *phydev, int devad,
> +                              int regnum)
> +{
> +       struct phy_driver *drv = phydev->drv;
> +
> +       if (regnum > (u16)~0 || devad > 32)

Shouldn't this be >= 32?

> +               return -EINVAL;
> +
> +       /* driver-specific access */
> +       if (drv->read_mmd)
> +               return drv->read_mmd(phydev, devad, regnum);
> +
> +       /* direct C45 / C22 access */
> +       if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
> +           devad == MDIO_DEVAD_NONE || !devad)
> +               return phy_read(phydev, devad, regnum);
> +
> +       /* indirect C22 access */
> +       phy_mmd_start_indirect(phydev, devad, regnum);
> +
> +       /* Read the content of the MMD's selected register */
> +       return phy_read(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA);
> +}
> +
> +static inline int phy_write_mmd(struct phy_device *phydev, int devad,
> +                               int regnum, u16 val)
> +{
> +       struct phy_driver *drv = phydev->drv;
> +
> +       if (regnum > (u16)~0 || devad > 32)

Same here.

> +               return -EINVAL;
> +
> +       /* driver-specific access */
> +       if (drv->write_mmd)
> +               return drv->write_mmd(phydev, devad, regnum, val);
> +
> +       /* direct C45 / C22 access */
> +       if ((drv->features & PHY_10G_FEATURES) == PHY_10G_FEATURES ||
> +           devad == MDIO_DEVAD_NONE || !devad)
> +               return phy_write(phydev, devad, regnum, val);
> +
> +       /* indirect C22 access */
> +       phy_mmd_start_indirect(phydev, devad, regnum);
> +
> +       /* Write the data into MMD's selected register */
> +       return phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
> +}
> +
>  #ifdef CONFIG_PHYLIB_10G
>  extern struct phy_driver gen10g_driver;
>
> --
> 2.19.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot