[U-Boot] [PATCH] net/phy: refactor RTL8211F initialization

[Shengzhou.Liu at freescale.com]

> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershberger at gmail.com]
> Sent: Thursday, April 23, 2015 10:42 PM
> To: Liu Shengzhou-B36685
> Cc: u-boot
> Subject: Re: [PATCH] net/phy: refactor RTL8211F initialization
> 
> Hi Shengzhou Liu,
> 
> On Wed, Apr 22, 2015 at 5:22 AM, Shengzhou Liu <Shengzhou.Liu at freescale.com>
> wrote:
> > RTL8211F needs to enalbe TXDLY for RGMII during phy initialization, so
> > move it to rtl8211f_config for early initialization.
> >
> > Signed-off-by: Shengzhou Liu <Shengzhou.Liu at freescale.com>
> > cc: Joe Hershberger <joe.hershberger at gmail.com>
> > ---
> >  drivers/net/phy/realtek.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index 3917c82..d48095b 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -43,6 +43,22 @@ static int rtl8211x_config(struct phy_device *phydev)
> >         return 0;
> >  }
> >
> > +static int rtl8211f_config(struct phy_device *phydev) {
> > +       phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> 
> Do you not need to disable the phy interrupt here?
No need, It's disabled by default.

> > +
> > +       if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> > +               /* enable TXDLY */
> > +               phy_write(phydev, MDIO_DEVAD_NONE,
> > +                         MIIM_RTL8211F_PAGE_SELECT, 0xd08);
> 
> Why do you not need to change the page back to default? Does it only apply to
> one following command or something? I haven't read the data sheet for this
> phy to understand its behavior, but want to make sure it's clear here.
> Please at least add a comment describing why the page need not be changed
> back.
There is no other command, so it's working without back to default. 
To avoid potential problem if one not set expected page, will have the page back to default 0 in v2.
 
> > +               phy_write(phydev, MDIO_DEVAD_NONE, 0x11, 0x109);
> 
> Is this TX delay board specific? Seems like it would be. Should it be
> parameterized to come from a board CONFIG_? If not, at least add a comment
> describing these magic numbers.

It is not board specific. Will replace the magic number.