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

[Chris Packham]

On Wed, Jun 27, 2018 at 2:25 AM Joe Hershberger <joe.hershberger at ni.com> wrote:
>
> On Tue, Jun 26, 2018 at 5:34 AM, Chris Packham <judge.packham at gmail.com> wrote:
> > 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.
>
> By that do you mean too much work?
>

Certainly more work than I have time for u-boot hacking. I can add it
to my "someday" list.

One pre-requisite for the full dsa switch binding would be the mdio
bus uclass (added Ken to the Cc list). We could come up with some
u-boot specific device tree bindings which would be easier to
implement at the cost of the device trees deviating from Linux.

> >
> >> > +       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
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot