[U-Boot] [PATCH 1/2] net: mv88e61xx: add configuration for RGMII delay

[Chris Packham]

On Tue, Jun 26, 2018 at 6:10 AM Joe Hershberger <joe.hershberger at ni.com> wrote:
>
> On Mon, Jun 25, 2018 at 5:34 AM, Chris Packham <judge.packham at gmail.com> wrote:
> > Some hardware designs connect a CPU MAC directly to the RGMII interface
> > of a mv88e61xx device. On such devices a delay on the RX/TX lines is
> > required, this can either be achieved by adding extra length to the
> > traces on the PCB or by implementing the delay in silicon. This is
> > an implementation of the latter.
> >
> > Signed-off-by: Chris Packham <judge.packham at gmail.com>
> > ---
> >
> >  drivers/net/phy/Kconfig     |  4 ++++
> >  drivers/net/phy/mv88e61xx.c | 26 ++++++++++++++++++++++++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index f5821dfed96d..98cd57eea977 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -59,6 +59,10 @@ config MV88E61XX_PHY_PORTS
> >  config MV88E61XX_FIXED_PORTS
> >         hex "Bitmask of PHYless serdes Ports"
> >
> > +config MV88E61XX_RGMII_DELAY
> > +       bool "Add delay to RGMII outputs"
> > +       default n
>
> Not sure why you would need this line. default n is implied, no?
>

I'll remove it in v2.

> > +
> >  endif # MV88E61XX_SWITCH
> >
> >  config PHYLIB_10G
> > diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c
> > index ea54a1531053..d258ba1ef0f3 100644
> > --- a/drivers/net/phy/mv88e61xx.c
> > +++ b/drivers/net/phy/mv88e61xx.c
> > @@ -69,6 +69,7 @@
> >  #define PORT_REG_CTRL                  0x04
> >  #define PORT_REG_VLAN_MAP              0x06
> >  #define PORT_REG_VLAN_ID               0x07
> > +#define PORT_REG_RGMII_TIMING          0x1A
> >
> >  /* Phy registers */
> >  #define PHY_REG_CTRL1                  0x10
> > @@ -122,6 +123,9 @@
> >  #define PORT_REG_VLAN_MAP_TABLE_SHIFT  0
> >  #define PORT_REG_VLAN_MAP_TABLE_WIDTH  11
> >
> > +#define PORT_REG_RGMII_TIMING_RX_DELAY BIT(10)
> > +#define PORT_REG_RGMII_TIMING_TX_DELAY BIT(9)
> > +
> >  #define SERDES_REG_CTRL_1_FORCE_LINK   BIT(10)
> >
> >  #define PHY_REG_CTRL1_ENERGY_DET_SHIFT 8
> > @@ -705,6 +709,24 @@ unforce:
> >         return res;
> >  }
> >
> > +static int mv88e61xx_rgmii_timing_cfg(struct phy_device *phydev)
> > +{
> > +#ifdef CONFIG_MV88E61XX_RGMII_DELAY
>
> It seems like this would be more appropriate as a device tree property
> rather than a board config.
>

Yes, as would the assignment of ports. It would make sense to follow
the dsa bindings used by Linux which would be a fairly significant
piece of work.

> > +       int val;
> > +
> > +       val = mv88e61xx_port_read(phydev, 6, PORT_REG_RGMII_TIMING);
> > +       if (val < 0)
> > +               return val;
> > +
> > +       val |= PORT_REG_RGMII_TIMING_RX_DELAY |
> > +              PORT_REG_RGMII_TIMING_TX_DELAY;
> > +
> > +       return mv88e61xx_port_write(phydev, 6, PORT_REG_RGMII_TIMING, val);
> > +#else
> > +       return 0;
> > +#endif
> > +}
> > +
> >  static int mv88e61xx_fixed_port_setup(struct phy_device *phydev, u8 port)
> >  {
> >         int val;
> > @@ -774,6 +796,10 @@ static int mv88e61xx_set_cpu_port(struct phy_device *phydev)
> >                                 return val;
> >                 }
> >         } else {
> > +               val = mv88e61xx_rgmii_timing_cfg(phydev);
> > +               if (val < 0)
> > +                       return val;
> > +
> >                 val = mv88e61xx_fixed_port_setup(phydev,
> >                                                  CONFIG_MV88E61XX_CPU_PORT);
> >                 if (val < 0)
> > --
> > 2.18.0
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot