diff mbox series

net: ethernet: mvneta: add support for 2.5G DRSGMII mode

Message ID 20200608074716.9975-1-s.hauer@pengutronix.de
State Changes Requested
Headers show
Series net: ethernet: mvneta: add support for 2.5G DRSGMII mode | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch warning total: 0 errors, 1 warnings, 56 lines checked

Commit Message

Sascha Hauer June 8, 2020, 7:47 a.m. UTC
The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
called DRSGMII.

This patch adds a corresponding phy-mode string 'drsgmii' and parses it
from DT. The MVNETA then configures the SERDES protocol value
accordingly.

It was successfully tested on a MV78460 connected to a FPGA.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/net/ethernet-controller.yaml       | 1 +
 drivers/net/ethernet/marvell/mvneta.c                      | 7 ++++++-
 include/linux/phy.h                                        | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

This patch has already been sent 3 years ago here:
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20170123142206.5390-1-jlu@pengutronix.de/
Since then the driver has evolved a lot. 2.5Gbps is properly configured in the
MAC now.

Comments

Andrew Lunn June 8, 2020, 2:57 p.m. UTC | #1
On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> called DRSGMII.
> 
> This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> from DT. The MVNETA then configures the SERDES protocol value
> accordingly.
> 
> It was successfully tested on a MV78460 connected to a FPGA.

Hi Sascha

Is this really overclocked SGMII, or 2500BaseX? How does it differ
from 2500BaseX, which mvneta already supports?

Also, does comphy need extensions to support this?

      Andrew
Russell King - ARM Linux admin June 8, 2020, 4:08 p.m. UTC | #2
On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> called DRSGMII.
> 
> This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> from DT. The MVNETA then configures the SERDES protocol value
> accordingly.
> 
> It was successfully tested on a MV78460 connected to a FPGA.

Digging around, this is Armada XP?  Which SoCs is this mode supported?
There's no mention of DRSGMII in the A38x nor A37xx documentation which
are later than Armada XP.

What exactly is "drsgmii"?  It can't be "double-rate" SGMII because that
would give you 2Gbps max instead of the 1Gbps, but this gives 2.5Gbps,
so I'm really not sure using "drsgmii" is a good idea.  It may be what
Marvell call it, but we really need to know if there's some vendor
neutral way to refer to it.

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../devicetree/bindings/net/ethernet-controller.yaml       | 1 +
>  drivers/net/ethernet/marvell/mvneta.c                      | 7 ++++++-
>  include/linux/phy.h                                        | 3 +++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> This patch has already been sent 3 years ago here:
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20170123142206.5390-1-jlu@pengutronix.de/
> Since then the driver has evolved a lot. 2.5Gbps is properly configured in the
> MAC now.

Nevertheless, adding a new interface mode needs properly documenting to
describe exactly what it is - see Documentation/networking/phy.rst, the
section "PHY interface modes".  The above point about "what is this"
illustrates why we need these documented.
Sascha Hauer June 9, 2020, 12:11 p.m. UTC | #3
On Mon, Jun 08, 2020 at 05:08:01PM +0100, Russell King - ARM Linux admin wrote:
> On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> > The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> > called DRSGMII.
> > 
> > This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> > from DT. The MVNETA then configures the SERDES protocol value
> > accordingly.
> > 
> > It was successfully tested on a MV78460 connected to a FPGA.
> 
> Digging around, this is Armada XP?  Which SoCs is this mode supported?
> There's no mention of DRSGMII in the A38x nor A37xx documentation which
> are later than Armada XP.

It's an Armada XP MV78460 in my case. I have no idea what other SoCs
this mode is supported on.

> 
> What exactly is "drsgmii"?  It can't be "double-rate" SGMII because that
> would give you 2Gbps max instead of the 1Gbps, but this gives 2.5Gbps,
> so I'm really not sure using "drsgmii" is a good idea.  It may be what
> Marvell call it, but we really need to know if there's some vendor
> neutral way to refer to it.

The abbreviation really is for "Double Rated SGMII". It seems it has 2.5
times the clock rate than ordinary SGMII. Another term I found is HSGMII
(High serial gigabit media-independent interface) which also has
2.5Gbps.

Anyway, I just learned from the paragraph you added to
Documentation/networking/phy.rst that 1000BASEX differs from SGMII in
the format of the control word. As we have a fixed link to a FPGA the
control word seems to be unused, at least the Port MAC Control Register0
PortType setting bit doesn't change anything. So I can equally well use
the existing 2500BASEX mode.

Sascha
Sascha Hauer June 9, 2020, 12:55 p.m. UTC | #4
On Mon, Jun 08, 2020 at 04:57:37PM +0200, Andrew Lunn wrote:
> On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> > The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> > called DRSGMII.
> > 
> > This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> > from DT. The MVNETA then configures the SERDES protocol value
> > accordingly.
> > 
> > It was successfully tested on a MV78460 connected to a FPGA.
> 
> Hi Sascha
> 
> Is this really overclocked SGMII, or 2500BaseX? How does it differ
> from 2500BaseX, which mvneta already supports?

I think it is overclocked SGMII or 2500BaseX depending on the Port MAC
Control Register0 PortType setting bit.
As said to Russell we have a fixed link so nobody really cares if it's
SGMII or 2500BaseX. This boils down the patch to fixing the Serdes
configuration setting for 2500BaseX.

Sascha
Andrew Lunn June 9, 2020, 1:12 p.m. UTC | #5
On Tue, Jun 09, 2020 at 02:55:35PM +0200, Sascha Hauer wrote:
> On Mon, Jun 08, 2020 at 04:57:37PM +0200, Andrew Lunn wrote:
> > On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> > > The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> > > called DRSGMII.
> > > 
> > > This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> > > from DT. The MVNETA then configures the SERDES protocol value
> > > accordingly.
> > > 
> > > It was successfully tested on a MV78460 connected to a FPGA.
> > 
> > Hi Sascha
> > 
> > Is this really overclocked SGMII, or 2500BaseX? How does it differ
> > from 2500BaseX, which mvneta already supports?
> 
> I think it is overclocked SGMII or 2500BaseX depending on the Port MAC
> Control Register0 PortType setting bit.
> As said to Russell we have a fixed link so nobody really cares if it's
> SGMII or 2500BaseX. This boils down the patch to fixing the Serdes
> configuration setting for 2500BaseX.

Hi Sascha

Does 2500BaseX work for your use case? Since this drsmgii mode is not
well defined, i would prefer to not add it, unless it is really
needed.

	Andrew
Sascha Hauer June 9, 2020, 1:14 p.m. UTC | #6
Hi Andrew,

On Tue, Jun 09, 2020 at 03:12:16PM +0200, Andrew Lunn wrote:
> On Tue, Jun 09, 2020 at 02:55:35PM +0200, Sascha Hauer wrote:
> > On Mon, Jun 08, 2020 at 04:57:37PM +0200, Andrew Lunn wrote:
> > > On Mon, Jun 08, 2020 at 09:47:16AM +0200, Sascha Hauer wrote:
> > > > The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> > > > called DRSGMII.
> > > > 
> > > > This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> > > > from DT. The MVNETA then configures the SERDES protocol value
> > > > accordingly.
> > > > 
> > > > It was successfully tested on a MV78460 connected to a FPGA.
> > > 
> > > Hi Sascha
> > > 
> > > Is this really overclocked SGMII, or 2500BaseX? How does it differ
> > > from 2500BaseX, which mvneta already supports?
> > 
> > I think it is overclocked SGMII or 2500BaseX depending on the Port MAC
> > Control Register0 PortType setting bit.
> > As said to Russell we have a fixed link so nobody really cares if it's
> > SGMII or 2500BaseX. This boils down the patch to fixing the Serdes
> > configuration setting for 2500BaseX.
> 
> Hi Sascha
> 
> Does 2500BaseX work for your use case? Since this drsmgii mode is not
> well defined, i would prefer to not add it, unless it is really
> needed.

Yes, it does, see updated patch I just sent.

Sascha
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index ac471b60ed6ae..4eead3c89bd3e 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -66,6 +66,7 @@  properties:
       - gmii
       - sgmii
       - qsgmii
+      - drsgmii
       - tbi
       - rev-mii
       - rmii
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 51889770958d8..807c698576c74 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -109,6 +109,7 @@ 
 #define MVNETA_SERDES_CFG			 0x24A0
 #define      MVNETA_SGMII_SERDES_PROTO		 0x0cc7
 #define      MVNETA_QSGMII_SERDES_PROTO		 0x0667
+#define      MVNETA_DRSGMII_SERDES_PROTO	 0x1107
 #define MVNETA_TYPE_PRIO                         0x24bc
 #define      MVNETA_FORCE_UNI                    BIT(21)
 #define MVNETA_TXQ_CMD_1                         0x24e4
@@ -3734,10 +3735,11 @@  static void mvneta_validate(struct phylink_config *config,
 	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
-	/* We only support QSGMII, SGMII, 802.3z and RGMII modes */
+	/* We only support QSGMII, SGMII, DRSGMII, 802.3z and RGMII modes */
 	if (state->interface != PHY_INTERFACE_MODE_NA &&
 	    state->interface != PHY_INTERFACE_MODE_QSGMII &&
 	    state->interface != PHY_INTERFACE_MODE_SGMII &&
+	    state->interface != PHY_INTERFACE_MODE_DRSGMII &&
 	    !phy_interface_mode_is_8023z(state->interface) &&
 	    !phy_interface_mode_is_rgmii(state->interface)) {
 		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
@@ -3851,6 +3853,7 @@  static void mvneta_mac_config(struct phylink_config *config, unsigned int mode,
 
 	if (state->interface == PHY_INTERFACE_MODE_QSGMII ||
 	    state->interface == PHY_INTERFACE_MODE_SGMII ||
+	    state->interface == PHY_INTERFACE_MODE_DRSGMII ||
 	    phy_interface_mode_is_8023z(state->interface))
 		new_ctrl2 |= MVNETA_GMAC2_PCS_ENABLE;
 
@@ -4968,6 +4971,8 @@  static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	else if (phy_mode == PHY_INTERFACE_MODE_SGMII ||
 		 phy_interface_mode_is_8023z(phy_mode))
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
+	else if (phy_mode == PHY_INTERFACE_MODE_DRSGMII)
+		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_DRSGMII_SERDES_PROTO);
 	else if (!phy_interface_mode_is_rgmii(phy_mode))
 		return -EINVAL;
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2432ca463ddc0..bf3276b330f9e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -109,6 +109,7 @@  typedef enum {
 	PHY_INTERFACE_MODE_USXGMII,
 	/* 10GBASE-KR - with Clause 73 AN */
 	PHY_INTERFACE_MODE_10GKR,
+	PHY_INTERFACE_MODE_DRSGMII,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -190,6 +191,8 @@  static inline const char *phy_modes(phy_interface_t interface)
 		return "usxgmii";
 	case PHY_INTERFACE_MODE_10GKR:
 		return "10gbase-kr";
+	case PHY_INTERFACE_MODE_DRSGMII:
+		return "drsgmii";
 	default:
 		return "unknown";
 	}