Re: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-05 Thread Andrew Lunn 
On Mon, Apr 05, 2021 at 09:07:34AM +, Voon, Weifeng wrote:
> > On Fri, Apr 02, 2021 at 07:45:04AM +, Voon, Weifeng wrote:
> > > > > + /* 2.5G mode only support 2500baseT full duplex only */
> > > > > + if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> > > > > + phylink_set(mac_supported, 2500baseT_Full);
> > > > > + phylink_set(mask, 10baseT_Half);
> > > > > + phylink_set(mask, 10baseT_Full);
> > > > > + phylink_set(mask, 100baseT_Half);
> > > > > + phylink_set(mask, 100baseT_Full);
> > > > > + phylink_set(mask, 1000baseT_Half);
> > > > > + phylink_set(mask, 1000baseT_Full);
> > > > > + phylink_set(mask, 1000baseKX_Full);
> > > >
> > > > Why? This seems at odds to the comment above?
> > >
> > > > What about 2500baseX_Full ?
> > >
> > > The comments explain that the PCS<->PHY link is in 2500BASE-X and why
> > > 10/100/1000 link speed is mutually exclusive with 2500.
> > > But the connected external PHY are twisted pair cable which only
> > > supports 2500baseT_full.
> > 
> > The PHY should indicate what modes its supports. The PHY drivers
> > get_features() call should set supported to only 2500baseT_Full, if that is
> > all it supports.
> > 
> > What modes are actually used should then be the intersect of what both the
> > MAC and the PHY indicate they can do.
> 
> Noted Andrew. Instead of masking the 10/100/1000 mode support in the MAC, we 
> will
> set the supported modes in the PCS.

PCS?

You said:

> > > But the connected external PHY are twisted pair cable which only
> > > supports 2500baseT_full.

So it should be the PHY, not the PCS, which indicates it only supports
2500baseT_full.

Andrew

RE: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-05 Thread Voon, Weifeng 
> On Fri, Apr 02, 2021 at 07:45:04AM +, Voon, Weifeng wrote:
> > > > +   /* 2.5G mode only support 2500baseT full duplex only */
> > > > +   if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> > > > +   phylink_set(mac_supported, 2500baseT_Full);
> > > > +   phylink_set(mask, 10baseT_Half);
> > > > +   phylink_set(mask, 10baseT_Full);
> > > > +   phylink_set(mask, 100baseT_Half);
> > > > +   phylink_set(mask, 100baseT_Full);
> > > > +   phylink_set(mask, 1000baseT_Half);
> > > > +   phylink_set(mask, 1000baseT_Full);
> > > > +   phylink_set(mask, 1000baseKX_Full);
> > >
> > > Why? This seems at odds to the comment above?
> >
> > > What about 2500baseX_Full ?
> >
> > The comments explain that the PCS<->PHY link is in 2500BASE-X and why
> > 10/100/1000 link speed is mutually exclusive with 2500.
> > But the connected external PHY are twisted pair cable which only
> > supports 2500baseT_full.
> 
> The PHY should indicate what modes its supports. The PHY drivers
> get_features() call should set supported to only 2500baseT_Full, if that is
> all it supports.
> 
> What modes are actually used should then be the intersect of what both the
> MAC and the PHY indicate they can do.

Noted Andrew. Instead of masking the 10/100/1000 mode support in the MAC, we 
will
set the supported modes in the PCS.

Re: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-02 Thread Andrew Lunn 
On Fri, Apr 02, 2021 at 07:45:04AM +, Voon, Weifeng wrote:
> > > + /* 2.5G mode only support 2500baseT full duplex only */
> > > + if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> > > + phylink_set(mac_supported, 2500baseT_Full);
> > > + phylink_set(mask, 10baseT_Half);
> > > + phylink_set(mask, 10baseT_Full);
> > > + phylink_set(mask, 100baseT_Half);
> > > + phylink_set(mask, 100baseT_Full);
> > > + phylink_set(mask, 1000baseT_Half);
> > > + phylink_set(mask, 1000baseT_Full);
> > > + phylink_set(mask, 1000baseKX_Full);
> > 
> > Why? This seems at odds to the comment above?
> 
> > What about 2500baseX_Full ?
> 
> The comments explain that the PCS<->PHY link is in 2500BASE-X
> and why 10/100/1000 link speed is mutually exclusive with 2500.
> But the connected external PHY are twisted pair cable which only
> supports 2500baseT_full.

The PHY should indicate what modes its supports. The PHY drivers
get_features() call should set supported to only 2500baseT_Full, if
that is all it supports.

What modes are actually used should then be the intersect of what both
the MAC and the PHY indicate they can do.

 Andrew

RE: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-02 Thread Voon, Weifeng 
> > +   /* 2.5G mode only support 2500baseT full duplex only */
> > +   if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> > +   phylink_set(mac_supported, 2500baseT_Full);
> > +   phylink_set(mask, 10baseT_Half);
> > +   phylink_set(mask, 10baseT_Full);
> > +   phylink_set(mask, 100baseT_Half);
> > +   phylink_set(mask, 100baseT_Full);
> > +   phylink_set(mask, 1000baseT_Half);
> > +   phylink_set(mask, 1000baseT_Full);
> > +   phylink_set(mask, 1000baseKX_Full);
> 
> Why? This seems at odds to the comment above?

> What about 2500baseX_Full ?

The comments explain that the PCS<->PHY link is in 2500BASE-X
and why 10/100/1000 link speed is mutually exclusive with 2500.
But the connected external PHY are twisted pair cable which only
supports 2500baseT_full.

Weifeng

Re: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-01 Thread Russell King - ARM Linux admin 
On Thu, Apr 01, 2021 at 11:01:51PM +0800, Michael Sit Wei Hong wrote:
> + /* 2.5G mode only support 2500baseT full duplex only */
> + if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> + phylink_set(mac_supported, 2500baseT_Full);
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseKX_Full);

Why? This seems at odds to the comment above?

What about 2500baseX_Full ?

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

[PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed

2021-04-01 Thread Michael Sit Wei Hong 
From: Voon Weifeng 

Intel mgbe supports 2.5G mode operation when PCS is in 1000BASE-X mode.
The 2.5G mode of operation is functionally same as 1000BASE-X mode,
except that the clock rate is 2.5 times the original rate. In 2.5G mode,
the link will operate as 2500Mbps/250Mbps/25Mbps. Hence, 2.5Gbps
link speed will be mutually exclusive with 1000Mbps/100Mbps/10Mbps.

Signed-off-by: Voon Weifeng 
Signed-off-by: Michael Sit Wei Hong 
---
 .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 44 ++-
 .../net/ethernet/stmicro/stmmac/dwmac-intel.h | 13 ++
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 26 ++-
 include/linux/stmmac.h|  2 +
 5 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 3d9a57043af2..4f70a12b42f9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -102,6 +102,22 @@ static int intel_serdes_powerup(struct net_device *ndev, 
void *priv_data)
 
serdes_phy_addr = intel_priv->mdio_adhoc_addr;
 
+   /* Set the serdes rate and the PCLK rate */
+   data = mdiobus_read(priv->mii, serdes_phy_addr,
+   SERDES_GCR0);
+
+   data &= ~SERDES_RATE_MASK;
+   data &= ~SERDES_PCLK_MASK;
+
+   if (priv->plat->speed_2500_en)
+   data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT |
+   SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT;
+   else
+   data |= SERDES_RATE_PCIE_GEN1 << SERDES_RATE_PCIE_SHIFT |
+   SERDES_PCLK_70MHZ << SERDES_PCLK_SHIFT;
+
+   mdiobus_write(priv->mii, serdes_phy_addr, SERDES_GCR0, data);
+
/* assert clk_req */
data = mdiobus_read(priv->mii, serdes_phy_addr, SERDES_GCR0);
data |= SERDES_PLL_CLK;
@@ -220,6 +236,28 @@ static void intel_serdes_powerdown(struct net_device 
*ndev, void *intel_data)
}
 }
 
+static bool intel_speed_mode_2500(struct net_device *ndev, void *intel_data)
+{
+   struct intel_priv_data *intel_priv = intel_data;
+   struct stmmac_priv *priv = netdev_priv(ndev);
+   int serdes_phy_addr = 0;
+   u32 data = 0;
+
+   serdes_phy_addr = intel_priv->mdio_adhoc_addr;
+
+   /* Determine the link speed mode: 2.5Gbps/1Gbps */
+   data = mdiobus_read(priv->mii, serdes_phy_addr,
+   SERDES_GCR);
+
+   if (((data & SERDES_LINK_MODE_MASK) >> SERDES_LINK_MODE_SHIFT) ==
+   SERDES_LINK_MODE_2G5) {
+   dev_info(priv->device, "Link Speed Mode: 2.5Gbps\n");
+   return true;
+   } else {
+   return false;
+   }
+}
+
 /* Program PTP Clock Frequency for different variant of
  * Intel mGBE that has slightly different GPO mapping
  */
@@ -540,7 +578,7 @@ static int ehl_sgmii_data(struct pci_dev *pdev,
 {
plat->bus_id = 1;
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
-
+   plat->speed_mode_2500 = intel_speed_mode_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
 
@@ -593,6 +631,7 @@ static int ehl_pse0_sgmii1g_data(struct pci_dev *pdev,
 struct plat_stmmacenet_data *plat)
 {
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+   plat->speed_mode_2500 = intel_speed_mode_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
return ehl_pse0_common_data(pdev, plat);
@@ -631,6 +670,7 @@ static int ehl_pse1_sgmii1g_data(struct pci_dev *pdev,
 struct plat_stmmacenet_data *plat)
 {
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+   plat->speed_mode_2500 = intel_speed_mode_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
return ehl_pse1_common_data(pdev, plat);
@@ -655,6 +695,7 @@ static int tgl_sgmii_phy0_data(struct pci_dev *pdev,
 {
plat->bus_id = 1;
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+   plat->speed_mode_2500 = intel_speed_mode_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
return tgl_common_data(pdev, plat);
@@ -669,6 +710,7 @@ static int tgl_sgmii_phy1_data(struct pci_dev *pdev,
 {
plat->bus_id = 2;
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+   plat->speed_mode_2500 = intel_speed_mode_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
return tgl_common_data(pdev, plat);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
index e723096c0b15..021a5c178d97 100644
---