| Message ID | 1440323220-20438-7-git-send-email-andrew@lunn.ch |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Le 08/23/15 02:46, Andrew Lunn a écrit : > Some Marvell switches allow the RGMII Rx and Tx clock to be delayed > when the port is using RGMII. Have the adjust_link function look at > the phy interface type and enable this delay as requested. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++ > drivers/net/dsa/mv88e6xxx.h | 2 ++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c > index 7901db6503b4..f5af368751b2 100644 > --- a/drivers/net/dsa/mv88e6xxx.c > +++ b/drivers/net/dsa/mv88e6xxx.c > @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port, > if (phydev->duplex == DUPLEX_FULL) > reg |= PORT_PCS_CTRL_DUPLEX_FULL; > > + if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) && > + (port >= ps->num_ports - 2)) { Are we positive that the last two ports of a switch are going to be RGMII capable or is this something that should be moved to Device Tree / platform data to account for different switch families? Maybe having a bitmask of RGMII capable ports stored in "ps" would be good enough? > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) > + reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK; > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) > + reg |= PORT_PCS_CTRL_RGMII_DELAY_TXCLK; > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) > + reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK | > + PORT_PCS_CTRL_RGMII_DELAY_TXCLK); > + } > _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg); > > out: > diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h > index 79003c55fe62..9b6f3d9d5ae1 100644 > --- a/drivers/net/dsa/mv88e6xxx.h > +++ b/drivers/net/dsa/mv88e6xxx.h > @@ -46,6 +46,8 @@ > #define PORT_STATUS_TX_PAUSED BIT(5) > #define PORT_STATUS_FLOW_CTRL BIT(4) > #define PORT_PCS_CTRL 0x01 > +#define PORT_PCS_CTRL_RGMII_DELAY_RXCLK BIT(15) > +#define PORT_PCS_CTRL_RGMII_DELAY_TXCLK BIT(14) > #define PORT_PCS_CTRL_FC BIT(7) > #define PORT_PCS_CTRL_FORCE_FC BIT(6) > #define PORT_PCS_CTRL_LINK_UP BIT(5) >
On Sun, Aug 23, 2015 at 11:44:01AM -0700, Florian Fainelli wrote: > Le 08/23/15 02:46, Andrew Lunn a écrit : > > Some Marvell switches allow the RGMII Rx and Tx clock to be delayed > > when the port is using RGMII. Have the adjust_link function look at > > the phy interface type and enable this delay as requested. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++ > > drivers/net/dsa/mv88e6xxx.h | 2 ++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c > > index 7901db6503b4..f5af368751b2 100644 > > --- a/drivers/net/dsa/mv88e6xxx.c > > +++ b/drivers/net/dsa/mv88e6xxx.c > > @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port, > > if (phydev->duplex == DUPLEX_FULL) > > reg |= PORT_PCS_CTRL_DUPLEX_FULL; > > > > + if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) && > > + (port >= ps->num_ports - 2)) { > > Are we positive that the last two ports of a switch are going to be > RGMII capable or is this something that should be moved to Device Tree / > platform data to account for different switch families? Maybe having a > bitmask of RGMII capable ports stored in "ps" would be good enough? Hi Florian For these two families, this is correct. And it is a property of the switch, not the board, so should not be in DT. Other families are different. Older ones are Fast Ethernet only. Some don't have any RGMII ports, etc. It could be with time, this condition gets messy, at which point, a bitmask in ps would make sense. But is it justified now? Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23/08/15 14:10, Andrew Lunn wrote: > On Sun, Aug 23, 2015 at 11:44:01AM -0700, Florian Fainelli wrote: >> Le 08/23/15 02:46, Andrew Lunn a écrit : >>> Some Marvell switches allow the RGMII Rx and Tx clock to be delayed >>> when the port is using RGMII. Have the adjust_link function look at >>> the phy interface type and enable this delay as requested. >>> >>> Signed-off-by: Andrew Lunn <andrew@lunn.ch> >>> --- >>> drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++ >>> drivers/net/dsa/mv88e6xxx.h | 2 ++ >>> 2 files changed, 12 insertions(+) >>> >>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c >>> index 7901db6503b4..f5af368751b2 100644 >>> --- a/drivers/net/dsa/mv88e6xxx.c >>> +++ b/drivers/net/dsa/mv88e6xxx.c >>> @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port, >>> if (phydev->duplex == DUPLEX_FULL) >>> reg |= PORT_PCS_CTRL_DUPLEX_FULL; >>> >>> + if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) && >>> + (port >= ps->num_ports - 2)) { >> >> Are we positive that the last two ports of a switch are going to be >> RGMII capable or is this something that should be moved to Device Tree / >> platform data to account for different switch families? Maybe having a >> bitmask of RGMII capable ports stored in "ps" would be good enough? > > Hi Florian > > For these two families, this is correct. And it is a property of the > switch, not the board, so should not be in DT. Other families are > different. Older ones are Fast Ethernet only. Some don't have any > RGMII ports, etc. It could be with time, this condition gets messy, at > which point, a bitmask in ps would make sense. But is it justified > now? Sure, I think for now this patch is good as-is, I was mostly curious whether the assumption about the last 2 ports of the switch being RGMII would hold for a while, and it looks like it will. With that: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index 7901db6503b4..f5af368751b2 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -612,6 +612,16 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port, if (phydev->duplex == DUPLEX_FULL) reg |= PORT_PCS_CTRL_DUPLEX_FULL; + if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) && + (port >= ps->num_ports - 2)) { + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) + reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK; + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) + reg |= PORT_PCS_CTRL_RGMII_DELAY_TXCLK; + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) + reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK | + PORT_PCS_CTRL_RGMII_DELAY_TXCLK); + } _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg); out: diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h index 79003c55fe62..9b6f3d9d5ae1 100644 --- a/drivers/net/dsa/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx.h @@ -46,6 +46,8 @@ #define PORT_STATUS_TX_PAUSED BIT(5) #define PORT_STATUS_FLOW_CTRL BIT(4) #define PORT_PCS_CTRL 0x01 +#define PORT_PCS_CTRL_RGMII_DELAY_RXCLK BIT(15) +#define PORT_PCS_CTRL_RGMII_DELAY_TXCLK BIT(14) #define PORT_PCS_CTRL_FC BIT(7) #define PORT_PCS_CTRL_FORCE_FC BIT(6) #define PORT_PCS_CTRL_LINK_UP BIT(5)
Some Marvell switches allow the RGMII Rx and Tx clock to be delayed when the port is using RGMII. Have the adjust_link function look at the phy interface type and enable this delay as requested. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++ drivers/net/dsa/mv88e6xxx.h | 2 ++ 2 files changed, 12 insertions(+)