[U-Boot] [PATCH v2 3/5] net: phy: Marvell 88E151x: Add support for RGMII

[Phil Edworthy]

Hi Joe

On 12 December 2016 10:15, Phil Edworthy wrote:
> On 12 December 2016 09:25, Phil Edworthy wrote:
> > On 09 December 2016 18:59, Joe Hershberger wrote:
> > > On Fri, Dec 9, 2016 at 7:38 AM, Phil Edworthy <phil.edworthy at renesas.com>
> > > wrote:
> > > > This has been tested with a Marvell 88E1512 PHY.
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com>
> > > > Reviewed-by: Stefan Roese <sr at denx.de>
> > > > ---
> > > > v2:
> > > >  Rebased on top of Joe's code to use macros
> > > > ---
> > > >  drivers/net/phy/marvell.c | 17 +++++++++++++----
> > > >  1 file changed, 13 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > > > index 5adfe7d..646b00d 100644
> > > > --- a/drivers/net/phy/marvell.c
> > > > +++ b/drivers/net/phy/marvell.c
> > > > @@ -94,6 +94,7 @@
> > > >  #define MIIM_88E151x_INT_EN_OFFS       7
> > > >  /* Page 18 registers */
> > > >  #define MIIM_88E151x_GENERAL_CTRL      20
> > > > +#define MIIM_88E151x_MODE_RGMII                0
> > >
> > > I'm not sure why this is needed. The HW reset value for the mode is
> > > 0x7 (RGMII auto detect mode). Would it not be sufficient to simply not
> > > change it if the mode is RGMII?
> > I don’t have a manual for any Marvell devices, just some small bits
> > of information. All I can tell you is that without this patch, RGMII on
> > 1512 doesn't work.
> I messed around with the code a bit and found that I don’t need to
> write to the MIIM_88E151x_GENERAL_CTRL register to set it to RGMII,
> but I must do a soft reset of the PHY before calling m88e1111s_config().
> 
> Would you prefer something like this instead?
> @@ -340,6 +333,9 @@ static int m88e1518_config(struct phy_device *phydev)
>                 udelay(100);
>         }
> 
> +       if (phy_interface_is_rgmii(phydev))
> +               phy_reset(phydev);
> +
>         return m88e1111s_config(phydev);
>  }

Sometimes I hate PHYs! My v2 patch for detecting the 1512 is wrong; the
PHY still worked to some extent since there was still power on the board.
I _thought_ the power switch was cutting power to the PHY, but no it's
not.

After fixing up the 1512 detection again, there doesn't appear to be a
problem with detecting RGMII. The PHY was only gets a 10M link,  I was
only getting a 1Gb link by forcing MIIM_88E151x_MODE bits to 0, as per
this patch. And the reason for the wrong speed is that I used the wrong
PHY mode; RGMII instead of RGMII_ID.

So the upshot of all this is that this patch is not needed at all... I'll send
a new patch that changes the uid/mask to support the 88E1512.


> > > >  #define MIIM_88E151x_MODE_SGMII                1
> > > >  #define MIIM_88E151x_RESET_OFFS                15
> > > >
> > > > @@ -315,14 +316,22 @@ static int m88e1518_config(struct phy_device
> > > *phydev)
> > > >         phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > > >         phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> > > 0x0000);
> > > >
> > > > -       /* SGMII-to-Copper mode initialization */
> > > > -       if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > > > +       /* SGMII/RGMII-to-Copper mode initialization */
> > > > +       if ((phydev->interface == PHY_INTERFACE_MODE_SGMII) ||
> > > > +                       phy_interface_is_rgmii(phydev)) {
> > > > +               u16 mode;
> > > > +
> > > > +               if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
> > > > +                       mode = MIIM_88E151x_MODE_SGMII;
> > > > +               else
> > > > +                       mode = MIIM_88E151x_MODE_RGMII;
> > > > +
> > > >                 /* Select page 18 */
> > > >                 phy_write(phydev, MDIO_DEVAD_NONE,
> > MIIM_88E1118_PHY_PAGE,
> > > 18);
> > > >
> > > > -               /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > > > +               /* In reg 20, write MODE[2:0], SGMII or RGMII to Copper */
> > > >                 m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > > > -                                      0, 3, MIIM_88E151x_MODE_SGMII);
> > > > +                                      0, 3, mode);
> > > >
> > > >                 /* PHY reset is necessary after changing MODE[2:0] */
> > > >                 m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> > > > --
> > > > 2.7.4

Thanks
Phil