[PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Raju Lakkaraju]

Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
Update the port mode and autonegotiation

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
---
 drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
 drivers/net/pcs/pcs-xpcs.h |  4 ++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 4dbc21f604f2..4f89dcedf0fc 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
 	return 0;
 }
 
+static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
+				    struct phylink_link_state *state)
+{
+	int sts, lpa;
+
+	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
+	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
+	if (sts < 0 || lpa < 0) {
+		state->link = false;
+		return sts;
+	}
+
+	state->link = !!(sts & DW_VR_MII_MMD_STS_LINK_STS);
+	state->an_complete = !!(sts & DW_VR_MII_MMD_STS_AN_CMPL);
+	if (!state->link)
+		return 0;
+
+	state->speed = SPEED_2500;
+	state->pause |= MLO_PAUSE_TX | MLO_PAUSE_RX;
+	state->duplex = DUPLEX_FULL;
+
+	return 0;
+}
+
 static void xpcs_get_state(struct phylink_pcs *pcs,
 			   struct phylink_link_state *state)
 {
@@ -1127,6 +1151,13 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
 			       ERR_PTR(ret));
 		}
 		break;
+	case DW_2500BASEX:
+		ret = xpcs_get_state_2500basex(xpcs, state);
+		if (ret) {
+			pr_err("xpcs_get_state_2500basex returned %pe\n",
+			       ERR_PTR(ret));
+		}
+		break;
 	default:
 		return;
 	}
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 39a90417e535..92c838f4b251 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -55,6 +55,10 @@
 /* Clause 37 Defines */
 /* VR MII MMD registers offsets */
 #define DW_VR_MII_MMD_CTRL		0x0000
+#define DW_VR_MII_MMD_STS		0x0001
+#define DW_VR_MII_MMD_STS_LINK_STS	BIT(2)
+#define DW_VR_MII_MMD_STS_AN_CMPL	BIT(5)
+#define DW_VR_MII_MMD_LP_BABL		0x0005
 #define DW_VR_MII_DIG_CTRL1		0x8000
 #define DW_VR_MII_AN_CTRL		0x8001
 #define DW_VR_MII_AN_INTR_STS		0x8002
-- 
2.34.1

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Serge Semin]

Hi Raju

On Mon, Sep 25, 2023 at 01:21:42PM +0530, Raju Lakkaraju wrote:
> Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
> Update the port mode and autonegotiation
> 
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> ---
>  drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
>  drivers/net/pcs/pcs-xpcs.h |  4 ++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 4dbc21f604f2..4f89dcedf0fc 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
>  	return 0;
>  }
>  
> +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> +				    struct phylink_link_state *state)
> +{
> +	int sts, lpa;
> +
> +	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);

> +	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
> +	if (sts < 0 || lpa < 0) {
> +		state->link = false;
> +		return sts;
> +	}

The HW manual says: "The host uses this page to know the link
partner's ability when the base page is received through Clause 37
auto-negotiation." Seeing xpcs_config_2500basex() disables
auto-negotiation and lpa value is unused anyway why do you even need
to read the LP_BABL register?

> +
> +	state->link = !!(sts & DW_VR_MII_MMD_STS_LINK_STS);

> +	state->an_complete = !!(sts & DW_VR_MII_MMD_STS_AN_CMPL);

Similarly AN is disabled in the xpcs_config_2500basex() method. Why do
you need the parsing above? It isn't supposed to indicate any useful
value since AN is disabled.

-Serge(y)

> +	if (!state->link)
> +		return 0;
> +
> +	state->speed = SPEED_2500;
> +	state->pause |= MLO_PAUSE_TX | MLO_PAUSE_RX;
> +	state->duplex = DUPLEX_FULL;
> +
> +	return 0;
> +}
> +
>  static void xpcs_get_state(struct phylink_pcs *pcs,
>  			   struct phylink_link_state *state)
>  {
> @@ -1127,6 +1151,13 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
>  			       ERR_PTR(ret));
>  		}
>  		break;
> +	case DW_2500BASEX:
> +		ret = xpcs_get_state_2500basex(xpcs, state);
> +		if (ret) {
> +			pr_err("xpcs_get_state_2500basex returned %pe\n",
> +			       ERR_PTR(ret));
> +		}
> +		break;
>  	default:
>  		return;
>  	}
> diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
> index 39a90417e535..92c838f4b251 100644
> --- a/drivers/net/pcs/pcs-xpcs.h
> +++ b/drivers/net/pcs/pcs-xpcs.h
> @@ -55,6 +55,10 @@
>  /* Clause 37 Defines */
>  /* VR MII MMD registers offsets */
>  #define DW_VR_MII_MMD_CTRL		0x0000
> +#define DW_VR_MII_MMD_STS		0x0001
> +#define DW_VR_MII_MMD_STS_LINK_STS	BIT(2)
> +#define DW_VR_MII_MMD_STS_AN_CMPL	BIT(5)
> +#define DW_VR_MII_MMD_LP_BABL		0x0005
>  #define DW_VR_MII_DIG_CTRL1		0x8000
>  #define DW_VR_MII_AN_CTRL		0x8001
>  #define DW_VR_MII_AN_INTR_STS		0x8002
> -- 
> 2.34.1
> 
> 

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Russell King (Oracle)]

On Tue, Sep 26, 2023 at 02:39:21PM +0300, Serge Semin wrote:
> Hi Raju
> 
> On Mon, Sep 25, 2023 at 01:21:42PM +0530, Raju Lakkaraju wrote:
> > Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
> > Update the port mode and autonegotiation
> > 
> > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > ---
> >  drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
> >  drivers/net/pcs/pcs-xpcs.h |  4 ++++
> >  2 files changed, 35 insertions(+)
> > 
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > index 4dbc21f604f2..4f89dcedf0fc 100644
> > --- a/drivers/net/pcs/pcs-xpcs.c
> > +++ b/drivers/net/pcs/pcs-xpcs.c
> > @@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> >  	return 0;
> >  }
> >  
> > +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> > +				    struct phylink_link_state *state)
> > +{
> > +	int sts, lpa;
> > +
> > +	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
> 
> > +	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
> > +	if (sts < 0 || lpa < 0) {
> > +		state->link = false;
> > +		return sts;
> > +	}
> 
> The HW manual says: "The host uses this page to know the link
> partner's ability when the base page is received through Clause 37
> auto-negotiation." Seeing xpcs_config_2500basex() disables
> auto-negotiation and lpa value is unused anyway why do you even need
> to read the LP_BABL register?

Since you have access to the hardware manual, what does it say about
clause 37 auto-negotiation when operating in 2500base-X mode?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Serge Semin]

Hi Russell

On Tue, Sep 26, 2023 at 12:45:47PM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 26, 2023 at 02:39:21PM +0300, Serge Semin wrote:
> > Hi Raju
> > 
> > On Mon, Sep 25, 2023 at 01:21:42PM +0530, Raju Lakkaraju wrote:
> > > Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
> > > Update the port mode and autonegotiation
> > > 
> > > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > > ---
> > >  drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
> > >  drivers/net/pcs/pcs-xpcs.h |  4 ++++
> > >  2 files changed, 35 insertions(+)
> > > 
> > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > > index 4dbc21f604f2..4f89dcedf0fc 100644
> > > --- a/drivers/net/pcs/pcs-xpcs.c
> > > +++ b/drivers/net/pcs/pcs-xpcs.c
> > > @@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> > >  	return 0;
> > >  }
> > >  
> > > +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> > > +				    struct phylink_link_state *state)
> > > +{
> > > +	int sts, lpa;
> > > +
> > > +	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
> > 
> > > +	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
> > > +	if (sts < 0 || lpa < 0) {
> > > +		state->link = false;
> > > +		return sts;
> > > +	}
> > 
> > The HW manual says: "The host uses this page to know the link
> > partner's ability when the base page is received through Clause 37
> > auto-negotiation." Seeing xpcs_config_2500basex() disables
> > auto-negotiation and lpa value is unused anyway why do you even need
> > to read the LP_BABL register?
> 

> Since you have access to the hardware manual, what does it say about
> clause 37 auto-negotiation when operating in 2500base-X mode?

Here are the parts which mention 37 & SGMII AN in the 2.5G context:

1. "Clause 37 (& SGMII) auto-negotiation is supported in 2.5G mode if
    the link partner is also operating in the equivalent 2.5G mode."

2. "During the Clause 37/SGMII as the auto-negotiation link timer
    operates with a faster clock in the 2.5G mode, the timeout duration
    reduces by a factor of 2.5. To restore the standard specified timeout
    period, the respective registers must be re-programmed."

I guess the entire 2.5G-thing understanding could be generalized by
the next sentence from the HW manual: "The 2.5G mode of operation is
functionally the same as 1000BASE-X/KX mode, except that the
clock-rate is 2.5 times the original rate. In this mode, the
Serdes/PHY operates at a serial baud-rate of 3.125 Gbps and DWC_xpcs
data-path and the GMII interface to MAC operates at 312.5 MHz (instead
of 125 MHz)." Thus here is another info regarding AN in that context:

3. "The DWC_xpcs operates either in 10/100/1000Mbps rate or
25/250/2500Mbps rates respectively with SGMII auto-negotiation. The
DWC_xpcs cannot support switching or negotiation between 1G and 2.5G
rates using auto-negotiation."

-Serge(y)

> 
> Thanks.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Russell King (Oracle)]

On Tue, Sep 26, 2023 at 03:09:47PM +0300, Serge Semin wrote:
> Hi Russell
> 
> On Tue, Sep 26, 2023 at 12:45:47PM +0100, Russell King (Oracle) wrote:
> > On Tue, Sep 26, 2023 at 02:39:21PM +0300, Serge Semin wrote:
> > > Hi Raju
> > > 
> > > On Mon, Sep 25, 2023 at 01:21:42PM +0530, Raju Lakkaraju wrote:
> > > > Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
> > > > Update the port mode and autonegotiation
> > > > 
> > > > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > > > ---
> > > >  drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
> > > >  drivers/net/pcs/pcs-xpcs.h |  4 ++++
> > > >  2 files changed, 35 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > > > index 4dbc21f604f2..4f89dcedf0fc 100644
> > > > --- a/drivers/net/pcs/pcs-xpcs.c
> > > > +++ b/drivers/net/pcs/pcs-xpcs.c
> > > > @@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> > > > +				    struct phylink_link_state *state)
> > > > +{
> > > > +	int sts, lpa;
> > > > +
> > > > +	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
> > > 
> > > > +	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
> > > > +	if (sts < 0 || lpa < 0) {
> > > > +		state->link = false;
> > > > +		return sts;
> > > > +	}
> > > 
> > > The HW manual says: "The host uses this page to know the link
> > > partner's ability when the base page is received through Clause 37
> > > auto-negotiation." Seeing xpcs_config_2500basex() disables
> > > auto-negotiation and lpa value is unused anyway why do you even need
> > > to read the LP_BABL register?
> > 
> 
> > Since you have access to the hardware manual, what does it say about
> > clause 37 auto-negotiation when operating in 2500base-X mode?
> 
> Here are the parts which mention 37 & SGMII AN in the 2.5G context:
> 
> 1. "Clause 37 (& SGMII) auto-negotiation is supported in 2.5G mode if
>     the link partner is also operating in the equivalent 2.5G mode."
> 
> 2. "During the Clause 37/SGMII as the auto-negotiation link timer
>     operates with a faster clock in the 2.5G mode, the timeout duration
>     reduces by a factor of 2.5. To restore the standard specified timeout
>     period, the respective registers must be re-programmed."
> 
> I guess the entire 2.5G-thing understanding could be generalized by
> the next sentence from the HW manual: "The 2.5G mode of operation is
> functionally the same as 1000BASE-X/KX mode, except that the
> clock-rate is 2.5 times the original rate. In this mode, the
> Serdes/PHY operates at a serial baud-rate of 3.125 Gbps and DWC_xpcs
> data-path and the GMII interface to MAC operates at 312.5 MHz (instead
> of 125 MHz)." Thus here is another info regarding AN in that context:
> 
> 3. "The DWC_xpcs operates either in 10/100/1000Mbps rate or
> 25/250/2500Mbps rates respectively with SGMII auto-negotiation. The
> DWC_xpcs cannot support switching or negotiation between 1G and 2.5G
> rates using auto-negotiation."

Thanks for the clarification.

So this hardware, just like Marvell hardware, operates 2500BASE-X merely
by up-clocking, and all the features that were available at 1000BASE-X
are also available at 2500BASE-X, including the in-band signalling.

Therefore, I think rather than disabling AN outright, the
PHYLINK_PCS_NEG_* mode passed in should be checked to determine whether
inband should be used or not.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Serge Semin]

On Tue, Sep 26, 2023 at 03:46:23PM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 26, 2023 at 03:09:47PM +0300, Serge Semin wrote:
> > Hi Russell
> > 
> > On Tue, Sep 26, 2023 at 12:45:47PM +0100, Russell King (Oracle) wrote:
> > > On Tue, Sep 26, 2023 at 02:39:21PM +0300, Serge Semin wrote:
> > > > Hi Raju
> > > > 
> > > > On Mon, Sep 25, 2023 at 01:21:42PM +0530, Raju Lakkaraju wrote:
> > > > > Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
> > > > > Update the port mode and autonegotiation
> > > > > 
> > > > > Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> > > > > ---
> > > > >  drivers/net/pcs/pcs-xpcs.c | 31 +++++++++++++++++++++++++++++++
> > > > >  drivers/net/pcs/pcs-xpcs.h |  4 ++++
> > > > >  2 files changed, 35 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > > > > index 4dbc21f604f2..4f89dcedf0fc 100644
> > > > > --- a/drivers/net/pcs/pcs-xpcs.c
> > > > > +++ b/drivers/net/pcs/pcs-xpcs.c
> > > > > @@ -1090,6 +1090,30 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> > > > >  	return 0;
> > > > >  }
> > > > >  
> > > > > +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> > > > > +				    struct phylink_link_state *state)
> > > > > +{
> > > > > +	int sts, lpa;
> > > > > +
> > > > > +	sts = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
> > > > 
> > > > > +	lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_LP_BABL);
> > > > > +	if (sts < 0 || lpa < 0) {
> > > > > +		state->link = false;
> > > > > +		return sts;
> > > > > +	}
> > > > 
> > > > The HW manual says: "The host uses this page to know the link
> > > > partner's ability when the base page is received through Clause 37
> > > > auto-negotiation." Seeing xpcs_config_2500basex() disables
> > > > auto-negotiation and lpa value is unused anyway why do you even need
> > > > to read the LP_BABL register?
> > > 
> > 
> > > Since you have access to the hardware manual, what does it say about
> > > clause 37 auto-negotiation when operating in 2500base-X mode?
> > 
> > Here are the parts which mention 37 & SGMII AN in the 2.5G context:
> > 
> > 1. "Clause 37 (& SGMII) auto-negotiation is supported in 2.5G mode if
> >     the link partner is also operating in the equivalent 2.5G mode."
> > 
> > 2. "During the Clause 37/SGMII as the auto-negotiation link timer
> >     operates with a faster clock in the 2.5G mode, the timeout duration
> >     reduces by a factor of 2.5. To restore the standard specified timeout
> >     period, the respective registers must be re-programmed."
> > 
> > I guess the entire 2.5G-thing understanding could be generalized by
> > the next sentence from the HW manual: "The 2.5G mode of operation is
> > functionally the same as 1000BASE-X/KX mode, except that the
> > clock-rate is 2.5 times the original rate. In this mode, the
> > Serdes/PHY operates at a serial baud-rate of 3.125 Gbps and DWC_xpcs
> > data-path and the GMII interface to MAC operates at 312.5 MHz (instead
> > of 125 MHz)." Thus here is another info regarding AN in that context:
> > 
> > 3. "The DWC_xpcs operates either in 10/100/1000Mbps rate or
> > 25/250/2500Mbps rates respectively with SGMII auto-negotiation. The
> > DWC_xpcs cannot support switching or negotiation between 1G and 2.5G
> > rates using auto-negotiation."
> 
> Thanks for the clarification.
> 
> So this hardware, just like Marvell hardware, operates 2500BASE-X merely
> by up-clocking, and all the features that were available at 1000BASE-X
> are also available at 2500BASE-X, including the in-band signalling.
> 
> Therefore, I think rather than disabling AN outright, the
> PHYLINK_PCS_NEG_* mode passed in should be checked to determine whether
> inband should be used or not.

Just an additional note which might be relevant in the context of the
DW XPCS 1G/2.5G C37 AN. The C37 auto-negotiation feature will be
unavailable for 1000BASE-X and thus for 2500BASE-X if the IP-core is
synthesized with no support of one. It is determined by the CL37_AN
IP-core synthesize parameter state:

Enable SGMII Clause 37 | Description: Configures DWC_xpcs to support the
Auto-Negotiation       |              Clause 37 auto-negotiation
                       |
                       | Dependencies: This option is available in the
                       |   following configurations:
                       |   - SUPPORT_1G = Enabled
                       |   - MAIN_MODE = Backplane Ethernet PCS and
                       |     BACKPLANE_ETH_CONFIG = KX_Only or KX4_KX
                       |     or KR_KX or KR_KX4_KX mode
                       |
                       | Default Value: Enabled for configurations with
                       |   MAIN_MODE = 1000BASEX-Only PCS and Disabled
                       |   for all other configurations
                       |
                       | HDL Parameter Name: CL37_AN

So depending on the particular (vendor-specific) device configuration
the C37 AN might still unavailable even though the device supports the
1000BASE-X and 2500BASE-X interfaces.

-Serge(y)

> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Russell King (Oracle)]

On Tue, Sep 26, 2023 at 06:27:45PM +0300, Serge Semin wrote:
> On Tue, Sep 26, 2023 at 03:46:23PM +0100, Russell King (Oracle) wrote:
> > On Tue, Sep 26, 2023 at 03:09:47PM +0300, Serge Semin wrote:
> > > Hi Russell
> > > 
> > > > Since you have access to the hardware manual, what does it say about
> > > > clause 37 auto-negotiation when operating in 2500base-X mode?
> > > 
> > > Here are the parts which mention 37 & SGMII AN in the 2.5G context:
> > > 
> > > 1. "Clause 37 (& SGMII) auto-negotiation is supported in 2.5G mode if
> > >     the link partner is also operating in the equivalent 2.5G mode."
> > > 
> > > 2. "During the Clause 37/SGMII as the auto-negotiation link timer
> > >     operates with a faster clock in the 2.5G mode, the timeout duration
> > >     reduces by a factor of 2.5. To restore the standard specified timeout
> > >     period, the respective registers must be re-programmed."
> > > 
> > > I guess the entire 2.5G-thing understanding could be generalized by
> > > the next sentence from the HW manual: "The 2.5G mode of operation is
> > > functionally the same as 1000BASE-X/KX mode, except that the
> > > clock-rate is 2.5 times the original rate. In this mode, the
> > > Serdes/PHY operates at a serial baud-rate of 3.125 Gbps and DWC_xpcs
> > > data-path and the GMII interface to MAC operates at 312.5 MHz (instead
> > > of 125 MHz)." Thus here is another info regarding AN in that context:
> > > 
> > > 3. "The DWC_xpcs operates either in 10/100/1000Mbps rate or
> > > 25/250/2500Mbps rates respectively with SGMII auto-negotiation. The
> > > DWC_xpcs cannot support switching or negotiation between 1G and 2.5G
> > > rates using auto-negotiation."
> > 
> > Thanks for the clarification.
> > 
> > So this hardware, just like Marvell hardware, operates 2500BASE-X merely
> > by up-clocking, and all the features that were available at 1000BASE-X
> > are also available at 2500BASE-X, including the in-band signalling.
> > 
> > Therefore, I think rather than disabling AN outright, the
> > PHYLINK_PCS_NEG_* mode passed in should be checked to determine whether
> > inband should be used or not.
> 
> Just an additional note which might be relevant in the context of the
> DW XPCS 1G/2.5G C37 AN. The C37 auto-negotiation feature will be
> unavailable for 1000BASE-X and thus for 2500BASE-X if the IP-core is
> synthesized with no support of one. It is determined by the CL37_AN
> IP-core synthesize parameter state:
> 
> Enable SGMII Clause 37 | Description: Configures DWC_xpcs to support the
> Auto-Negotiation       |              Clause 37 auto-negotiation
>                        |
>                        | Dependencies: This option is available in the
>                        |   following configurations:
>                        |   - SUPPORT_1G = Enabled
>                        |   - MAIN_MODE = Backplane Ethernet PCS and
>                        |     BACKPLANE_ETH_CONFIG = KX_Only or KX4_KX
>                        |     or KR_KX or KR_KX4_KX mode
>                        |
>                        | Default Value: Enabled for configurations with
>                        |   MAIN_MODE = 1000BASEX-Only PCS and Disabled
>                        |   for all other configurations
>                        |
>                        | HDL Parameter Name: CL37_AN
> 
> So depending on the particular (vendor-specific) device configuration
> the C37 AN might still unavailable even though the device supports the
> 1000BASE-X and 2500BASE-X interfaces.

Thanks. I guess there's no way to read the CL37_AN setting from
software?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: pcs: xpcs: Add 2500BASE-X case in get state for XPCS drivers

[Serge Semin]

On Tue, Sep 26, 2023 at 04:47:14PM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 26, 2023 at 06:27:45PM +0300, Serge Semin wrote:
> > On Tue, Sep 26, 2023 at 03:46:23PM +0100, Russell King (Oracle) wrote:
> > > On Tue, Sep 26, 2023 at 03:09:47PM +0300, Serge Semin wrote:
> > > > Hi Russell
> > > > 
> > > > > Since you have access to the hardware manual, what does it say about
> > > > > clause 37 auto-negotiation when operating in 2500base-X mode?
> > > > 
> > > > Here are the parts which mention 37 & SGMII AN in the 2.5G context:
> > > > 
> > > > 1. "Clause 37 (& SGMII) auto-negotiation is supported in 2.5G mode if
> > > >     the link partner is also operating in the equivalent 2.5G mode."
> > > > 
> > > > 2. "During the Clause 37/SGMII as the auto-negotiation link timer
> > > >     operates with a faster clock in the 2.5G mode, the timeout duration
> > > >     reduces by a factor of 2.5. To restore the standard specified timeout
> > > >     period, the respective registers must be re-programmed."
> > > > 
> > > > I guess the entire 2.5G-thing understanding could be generalized by
> > > > the next sentence from the HW manual: "The 2.5G mode of operation is
> > > > functionally the same as 1000BASE-X/KX mode, except that the
> > > > clock-rate is 2.5 times the original rate. In this mode, the
> > > > Serdes/PHY operates at a serial baud-rate of 3.125 Gbps and DWC_xpcs
> > > > data-path and the GMII interface to MAC operates at 312.5 MHz (instead
> > > > of 125 MHz)." Thus here is another info regarding AN in that context:
> > > > 
> > > > 3. "The DWC_xpcs operates either in 10/100/1000Mbps rate or
> > > > 25/250/2500Mbps rates respectively with SGMII auto-negotiation. The
> > > > DWC_xpcs cannot support switching or negotiation between 1G and 2.5G
> > > > rates using auto-negotiation."
> > > 
> > > Thanks for the clarification.
> > > 
> > > So this hardware, just like Marvell hardware, operates 2500BASE-X merely
> > > by up-clocking, and all the features that were available at 1000BASE-X
> > > are also available at 2500BASE-X, including the in-band signalling.
> > > 
> > > Therefore, I think rather than disabling AN outright, the
> > > PHYLINK_PCS_NEG_* mode passed in should be checked to determine whether
> > > inband should be used or not.
> > 
> > Just an additional note which might be relevant in the context of the
> > DW XPCS 1G/2.5G C37 AN. The C37 auto-negotiation feature will be
> > unavailable for 1000BASE-X and thus for 2500BASE-X if the IP-core is
> > synthesized with no support of one. It is determined by the CL37_AN
> > IP-core synthesize parameter state:
> > 
> > Enable SGMII Clause 37 | Description: Configures DWC_xpcs to support the
> > Auto-Negotiation       |              Clause 37 auto-negotiation
> >                        |
> >                        | Dependencies: This option is available in the
> >                        |   following configurations:
> >                        |   - SUPPORT_1G = Enabled
> >                        |   - MAIN_MODE = Backplane Ethernet PCS and
> >                        |     BACKPLANE_ETH_CONFIG = KX_Only or KX4_KX
> >                        |     or KR_KX or KR_KX4_KX mode
> >                        |
> >                        | Default Value: Enabled for configurations with
> >                        |   MAIN_MODE = 1000BASEX-Only PCS and Disabled
> >                        |   for all other configurations
> >                        |
> >                        | HDL Parameter Name: CL37_AN
> > 
> > So depending on the particular (vendor-specific) device configuration
> > the C37 AN might still unavailable even though the device supports the
> > 1000BASE-X and 2500BASE-X interfaces.
> 

> Thanks. I guess there's no way to read the CL37_AN setting from
> software?

I can't be 100% sure because my HW doesn't support neither
1000Base-X/SGMII nor C37 AN to test out what is described further, but
AFAICS from the HW manual it can be done by either on the next ways:

1. There is Vendor-Specific 1 MMD #30 (0x1e) which controls other MMDs
present in the device configuration. It exposes a register 0x0009 "SR
Control MMD Control Register" which aside with some stuff can be used
to switch on/off device MMDs including the MII MMD mapped to the
Vendor-Specific 2 MMD #31 (0x1f). Field 2 in that CSR is called
MII_MMD_EN and is available as RW only if "CL37_AN = Enabled"
otherwise it's RO and is supposed to contain the default value 0
(which is the case for my device).
Although further, in the MII MMD Standard Registers (SR) description,
HW-manual says that "The MII registers are present in 1000BASEX-Only
PCS configurations or configurations with 1G Support (1000BASE-X
support) and Clause 37 auto-negotiation support." It sounds vague and
from some point of view contradicting to what is said in the
MII_MMD_EN field description - whether C37 AN availability is required
to have the MII registers available.

2. If option 1 turns to be not working to detect the C37 AN
availability I guess it can be done by means of the 0x0001 CSR "SR MII
MMD Status Register" exposed in the MII MMD #31 (0x1f). It has AN_ABL
bit 3 "Auto-negotiation Ability". The bit description says that it's
RO and indicates: "1: The DWC_xpcs is able to perform
auto-negotiation." and "0: The DWC_xpcs is not able to perform
auto-negotiation." But it also says that "The DWC_xpcs always returns
this bit as 1.", which most likely implicitly confirms the option 1
described before. So in case if MII MMD #31 is available that flag
will be always 1, otherwise if MII MMD #31 is fully unavailable that
flag will return 0 despite to what is said in its description.

-Serge(y)

> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!