| Message ID | 20180625103457.8674-1-judge.packham@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | [U-Boot,1/2] net: mv88e61xx: add configuration for RGMII delay | expand |
On Mon, Jun 25, 2018 at 5:34 AM, Chris Packham <judge.packham@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@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? > + > 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. > + 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@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Tue, Jun 26, 2018 at 6:10 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > > On Mon, Jun 25, 2018 at 5:34 AM, Chris Packham <judge.packham@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@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@lists.denx.de > > https://lists.denx.de/listinfo/u-boot
On Tue, Jun 26, 2018 at 5:34 AM, Chris Packham <judge.packham@gmail.com> wrote: > On Tue, Jun 26, 2018 at 6:10 AM Joe Hershberger <joe.hershberger@ni.com> wrote: >> >> On Mon, Jun 25, 2018 at 5:34 AM, Chris Packham <judge.packham@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@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@lists.denx.de >> > https://lists.denx.de/listinfo/u-boot > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Wed, Jun 27, 2018 at 2:25 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > > On Tue, Jun 26, 2018 at 5:34 AM, Chris Packham <judge.packham@gmail.com> wrote: > > On Tue, Jun 26, 2018 at 6:10 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > >> > >> On Mon, Jun 25, 2018 at 5:34 AM, Chris Packham <judge.packham@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@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@lists.denx.de > >> > https://lists.denx.de/listinfo/u-boot > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot
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 + 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 + 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)
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@gmail.com> --- drivers/net/phy/Kconfig | 4 ++++ drivers/net/phy/mv88e61xx.c | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)