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

[Joe Hershberger]

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?

>
>> > +       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