All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 net-next 00/12] Preparing for phylib limkmodes
@ 2018-09-11 23:53 Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause Andrew Lunn
                   ` (12 more replies)
  0 siblings, 13 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

phylib currently makes us of a u32 bitmap for advertising, supported,
and link partner capabilities. For a long time, this has been
sufficient, for devices up to 1Gbps. With more MAC/PHY combinations
now supporting speeds greater than 1Gbps, we have run out of
bits. There is the need to replace this u32 with an
__ETHTOOL_DECLARE_LINK_MODE_MASK, which makes use of linux's generic
bitmaps.

This patchset does some of the work preparing for this change. A few
cleanups are applied to PHY drivers. Some MAC drivers directly access
members of phydev which are going to change type. These patches adds
some helpers and swaps MAC drivers to use them, mostly dealing with
Pause configuration.

v3:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Add missing at in commit message
Change Subject of patch 5
Fix return in from phy_set_asym_pause
Fix kerneldoc in phy_set_pause

v2:
Fixup bad indentation in tg3.c
Rename phy_support_pause() to phy_support_sym_pause()
Also trigger autoneg if the advertising settings have changed.
Rename phy_set_pause() to phy_set_sym_pause()
Use the bcm63xx_enet.c logic, not fec_main.c for validating pause


Andrew Lunn (12):
  net: phy: ste10Xp: Remove wrong SUPPORTED_Pause
  net: phy: et1011c: Remove incorrect missing 1000 Half
  net: phy: bcm63xx: Allow to be built with COMPILE_TEST
  net: ethernet: Use phy_set_max_speed() to limit advertised speed
  net: bcmgenet: Fix speed selection for reverse MII
  net: ethernet: Fix up drivers masking pause support
  net: ethernet: Add helper to remove a supported link mode
  net: ethernet: Add helper for MACs which support asym pause
  net: ethernet: Add helper for MACs which support pause
  net: ethernet: Add helper for set_pauseparam for Asym Pause
  net: ethernet: Add helper for set_pauseparam for Pause
  net: ethernet: Add helper to determine if pause configuration is
    supported

 drivers/net/ethernet/8390/ax88796.c           |   4 +-
 drivers/net/ethernet/aeroflex/greth.c         |   4 +-
 drivers/net/ethernet/agere/et131x.c           |  12 +-
 drivers/net/ethernet/allwinner/sun4i-emac.c   |   3 +-
 drivers/net/ethernet/altera/altera_tse_main.c |   5 +-
 drivers/net/ethernet/amd/au1000_eth.c         |  12 +-
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c   |   4 +-
 .../ethernet/apm/xgene/xgene_enet_ethtool.c   |  30 +----
 .../net/ethernet/apm/xgene/xgene_enet_hw.c    |  10 +-
 drivers/net/ethernet/aurora/nb8800.c          |   9 +-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c  |  17 +--
 drivers/net/ethernet/broadcom/genet/bcmmii.c  |   9 +-
 drivers/net/ethernet/broadcom/sb1250-mac.c    |  14 +--
 drivers/net/ethernet/broadcom/tg3.c           |  59 +++------
 drivers/net/ethernet/cadence/macb_main.c      |   9 +-
 drivers/net/ethernet/cortina/gemini.c         |   5 +-
 drivers/net/ethernet/dnet.c                   |   8 +-
 drivers/net/ethernet/ethoc.c                  |   5 +-
 drivers/net/ethernet/faraday/ftgmac100.c      |  20 +--
 .../net/ethernet/freescale/dpaa/dpaa_eth.c    |   3 +-
 .../ethernet/freescale/dpaa/dpaa_ethtool.c    |  27 +---
 drivers/net/ethernet/freescale/fec_main.c     |  20 ++-
 drivers/net/ethernet/freescale/gianfar.c      |   4 +-
 .../net/ethernet/freescale/gianfar_ethtool.c  |  53 +++-----
 drivers/net/ethernet/freescale/ucc_geth.c     |   7 +-
 .../hisilicon/hns3/hns3pf/hclge_main.c        |   8 +-
 .../hisilicon/hns3/hns3pf/hclge_mdio.c        |   4 +-
 drivers/net/ethernet/lantiq_etop.c            |  11 +-
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |   8 +-
 drivers/net/ethernet/microchip/lan743x_main.c |   7 +-
 drivers/net/ethernet/nxp/lpc_eth.c            |   3 +-
 drivers/net/ethernet/rdc/r6040.c              |  12 +-
 drivers/net/ethernet/renesas/ravb_main.c      |   3 +-
 .../net/ethernet/samsung/sxgbe/sxgbe_main.c   |   4 +-
 drivers/net/ethernet/smsc/smsc911x.c          |   6 +-
 drivers/net/ethernet/smsc/smsc9420.c          |   6 +-
 drivers/net/ethernet/socionext/sni_ave.c      |  20 +--
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  15 ++-
 drivers/net/ethernet/toshiba/tc35815.c        |   2 +-
 drivers/net/ethernet/xilinx/xilinx_emaclite.c |   3 +-
 drivers/net/phy/Kconfig                       |   2 +-
 drivers/net/phy/et1011c.c                     |   2 +-
 drivers/net/phy/phy_device.c                  | 118 ++++++++++++++++++
 drivers/net/phy/ste10Xp.c                     |   4 +-
 drivers/net/usb/lan78xx.c                     |   2 +-
 drivers/staging/mt7621-eth/mdio.c             |   2 +-
 include/linux/phy.h                           |   8 ++
 47 files changed, 259 insertions(+), 344 deletions(-)

-- 
2.19.0.rc1

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 02/12] net: phy: et1011c: Remove incorrect missing 1000 Half Andrew Lunn
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

The PHY driver should not indicate that Pause is supported. It is upto
the MAC drive enable it, if it supports Pause frames. So remove it
from the ste10Xp driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v2: The datasheet indicates the PHY can negotiated pause, confirming
this is wrong.
---
 drivers/net/phy/ste10Xp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/ste10Xp.c b/drivers/net/phy/ste10Xp.c
index fbd548a1ad84..2fe9a87b55b5 100644
--- a/drivers/net/phy/ste10Xp.c
+++ b/drivers/net/phy/ste10Xp.c
@@ -86,7 +86,7 @@ static struct phy_driver ste10xp_pdriver[] = {
 	.phy_id = STE101P_PHY_ID,
 	.phy_id_mask = 0xfffffff0,
 	.name = "STe101p",
-	.features = PHY_BASIC_FEATURES | SUPPORTED_Pause,
+	.features = PHY_BASIC_FEATURES,
 	.flags = PHY_HAS_INTERRUPT,
 	.config_init = ste10Xp_config_init,
 	.ack_interrupt = ste10Xp_ack_interrupt,
@@ -97,7 +97,7 @@ static struct phy_driver ste10xp_pdriver[] = {
 	.phy_id = STE100P_PHY_ID,
 	.phy_id_mask = 0xffffffff,
 	.name = "STe100p",
-	.features = PHY_BASIC_FEATURES | SUPPORTED_Pause,
+	.features = PHY_BASIC_FEATURES,
 	.flags = PHY_HAS_INTERRUPT,
 	.config_init = ste10Xp_config_init,
 	.ack_interrupt = ste10Xp_ack_interrupt,
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 02/12] net: phy: et1011c: Remove incorrect missing 1000 Half
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 03/12] net: phy: bcm63xx: Allow to be built with COMPILE_TEST Andrew Lunn
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

The driver indicates it can do 10/100 full and half duplex, plus 1G
Full. The datasheet indicates 1G half is also supported. So make use
of the standard PHY_GBIT_FEATURES.

It could be, this was added because there is a MAC which does not
support 1G half. Bit this is the wrong place to enforce this.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/et1011c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/et1011c.c b/drivers/net/phy/et1011c.c
index a9a4edfa23c8..ab541c9c56fb 100644
--- a/drivers/net/phy/et1011c.c
+++ b/drivers/net/phy/et1011c.c
@@ -91,7 +91,7 @@ static struct phy_driver et1011c_driver[] = { {
 	.phy_id		= 0x0282f014,
 	.name		= "ET1011C",
 	.phy_id_mask	= 0xfffffff0,
-	.features	= (PHY_BASIC_FEATURES | SUPPORTED_1000baseT_Full),
+	.features	= PHY_GBIT_FEATURES,
 	.flags		= PHY_POLL,
 	.config_aneg	= et1011c_config_aneg,
 	.read_status	= et1011c_read_status,
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 03/12] net: phy: bcm63xx: Allow to be built with COMPILE_TEST
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 02/12] net: phy: et1011c: Remove incorrect missing 1000 Half Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed Andrew Lunn
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

There is nothing in this driver which prevents it to be compiled for
other architectures. Add COMPILE_TEST so we get better compile test
coverage.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 82070792edbb..3d187cd50eb0 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -240,7 +240,7 @@ config AT803X_PHY
 
 config BCM63XX_PHY
 	tristate "Broadcom 63xx SOCs internal PHY"
-	depends on BCM63XX
+	depends on BCM63XX || COMPILE_TEST
 	select BCM_NET_PHYLIB
 	---help---
 	  Currently supports the 6348 and 6358 PHYs.
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (2 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 03/12] net: phy: bcm63xx: Allow to be built with COMPILE_TEST Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-11-22 10:40   ` Anssi Hannula
  2018-09-11 23:53 ` [PATCH v3 net-next 05/12] net: bcmgenet: Fix speed selection for reverse MII Andrew Lunn
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Many Ethernet MAC drivers want to limit the PHY to only advertise a
maximum speed of 100Mbs or 1Gbps. Rather than using a mask, make use
of the helper function phy_set_max_speed().

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/8390/ax88796.c               |  4 +---
 drivers/net/ethernet/aeroflex/greth.c             |  4 ++--
 drivers/net/ethernet/agere/et131x.c               | 12 ++----------
 drivers/net/ethernet/allwinner/sun4i-emac.c       |  3 +--
 drivers/net/ethernet/altera/altera_tse_main.c     |  5 +----
 drivers/net/ethernet/amd/au1000_eth.c             | 12 +-----------
 drivers/net/ethernet/broadcom/bcm63xx_enet.c      | 10 ++--------
 drivers/net/ethernet/broadcom/genet/bcmmii.c      |  2 +-
 drivers/net/ethernet/broadcom/sb1250-mac.c        | 11 ++---------
 drivers/net/ethernet/broadcom/tg3.c               |  8 ++++----
 drivers/net/ethernet/cadence/macb_main.c          |  4 ++--
 drivers/net/ethernet/cortina/gemini.c             |  2 +-
 drivers/net/ethernet/dnet.c                       |  4 ++--
 drivers/net/ethernet/ethoc.c                      |  5 +----
 drivers/net/ethernet/freescale/fec_main.c         |  4 ++--
 drivers/net/ethernet/freescale/ucc_geth.c         |  7 +------
 drivers/net/ethernet/lantiq_etop.c                | 11 ++---------
 drivers/net/ethernet/mediatek/mtk_eth_soc.c       |  4 ++--
 drivers/net/ethernet/nxp/lpc_eth.c                |  3 +--
 drivers/net/ethernet/rdc/r6040.c                  | 12 ++----------
 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c   |  4 ++--
 drivers/net/ethernet/smsc/smsc911x.c              |  5 +++--
 drivers/net/ethernet/smsc/smsc9420.c              |  5 +++--
 drivers/net/ethernet/socionext/sni_ave.c          |  6 ++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  3 +--
 drivers/net/ethernet/toshiba/tc35815.c            |  2 +-
 drivers/net/ethernet/xilinx/xilinx_emaclite.c     |  3 +--
 drivers/staging/mt7621-eth/mdio.c                 |  2 +-
 28 files changed, 47 insertions(+), 110 deletions(-)

diff --git a/drivers/net/ethernet/8390/ax88796.c b/drivers/net/ethernet/8390/ax88796.c
index 2a0ddec1dd56..3dcc61821ed5 100644
--- a/drivers/net/ethernet/8390/ax88796.c
+++ b/drivers/net/ethernet/8390/ax88796.c
@@ -377,9 +377,7 @@ static int ax_mii_probe(struct net_device *dev)
 		return ret;
 	}
 
-	/* mask with MAC supported features */
-	phy_dev->supported &= PHY_BASIC_FEATURES;
-	phy_dev->advertising = phy_dev->supported;
+	phy_set_max_speed(phy_dev, SPEED_100);
 
 	netdev_info(dev, "PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
 		    phy_dev->drv->name, phydev_name(phy_dev), phy_dev->irq);
diff --git a/drivers/net/ethernet/aeroflex/greth.c b/drivers/net/ethernet/aeroflex/greth.c
index 4309be3724ad..7c9348a26cbb 100644
--- a/drivers/net/ethernet/aeroflex/greth.c
+++ b/drivers/net/ethernet/aeroflex/greth.c
@@ -1279,9 +1279,9 @@ static int greth_mdio_probe(struct net_device *dev)
 	}
 
 	if (greth->gbit_mac)
-		phy->supported &= PHY_GBIT_FEATURES;
+		phy_set_max_speed(phy, SPEED_1000);
 	else
-		phy->supported &= PHY_BASIC_FEATURES;
+		phy_set_max_speed(phy, SPEED_100);
 
 	phy->advertising = phy->supported;
 
diff --git a/drivers/net/ethernet/agere/et131x.c b/drivers/net/ethernet/agere/et131x.c
index 48220b6c600d..ea34bcb868b5 100644
--- a/drivers/net/ethernet/agere/et131x.c
+++ b/drivers/net/ethernet/agere/et131x.c
@@ -3258,19 +3258,11 @@ static int et131x_mii_probe(struct net_device *netdev)
 		return PTR_ERR(phydev);
 	}
 
-	phydev->supported &= (SUPPORTED_10baseT_Half |
-			      SUPPORTED_10baseT_Full |
-			      SUPPORTED_100baseT_Half |
-			      SUPPORTED_100baseT_Full |
-			      SUPPORTED_Autoneg |
-			      SUPPORTED_MII |
-			      SUPPORTED_TP);
+	phy_set_max_speed(phydev, SPEED_100);
 
 	if (adapter->pdev->device != ET131X_PCI_DEVICE_ID_FAST)
-		phydev->supported |= SUPPORTED_1000baseT_Half |
-				     SUPPORTED_1000baseT_Full;
+		phy_set_max_speed(phydev, SPEED_1000);
 
-	phydev->advertising = phydev->supported;
 	phydev->autoneg = AUTONEG_ENABLE;
 
 	phy_attached_info(phydev);
diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 3143de45baaa..e1acafa82214 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -172,8 +172,7 @@ static int emac_mdio_probe(struct net_device *dev)
 	}
 
 	/* mask with MAC supported features */
-	phydev->supported &= PHY_BASIC_FEATURES;
-	phydev->advertising = phydev->supported;
+	phy_set_max_speed(phydev, SPEED_100);
 
 	db->link = 0;
 	db->speed = 0;
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index baca8f704a45..02921d877c08 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -835,13 +835,10 @@ static int init_phy(struct net_device *dev)
 	}
 
 	/* Stop Advertising 1000BASE Capability if interface is not GMII
-	 * Note: Checkpatch throws CHECKs for the camel case defines below,
-	 * it's ok to ignore.
 	 */
 	if ((priv->phy_iface == PHY_INTERFACE_MODE_MII) ||
 	    (priv->phy_iface == PHY_INTERFACE_MODE_RMII))
-		phydev->advertising &= ~(SUPPORTED_1000baseT_Half |
-					 SUPPORTED_1000baseT_Full);
+		phy_set_max_speed(phydev, SPEED_100);
 
 	/* Broken HW is sometimes missing the pull-up resistor on the
 	 * MDIO line, which results in reads to non-existent devices returning
diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c
index 73ca8879ada7..7c1eb304c27e 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -564,17 +564,7 @@ static int au1000_mii_probe(struct net_device *dev)
 		return PTR_ERR(phydev);
 	}
 
-	/* mask with MAC supported features */
-	phydev->supported &= (SUPPORTED_10baseT_Half
-			      | SUPPORTED_10baseT_Full
-			      | SUPPORTED_100baseT_Half
-			      | SUPPORTED_100baseT_Full
-			      | SUPPORTED_Autoneg
-			      /* | SUPPORTED_Pause | SUPPORTED_Asym_Pause */
-			      | SUPPORTED_MII
-			      | SUPPORTED_TP);
-
-	phydev->advertising = phydev->supported;
+	phy_set_max_speed(phydev, SPEED_100);
 
 	aup->old_link = 0;
 	aup->old_speed = 0;
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 897302adc38e..2eee9459c2cf 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -890,14 +890,8 @@ static int bcm_enet_open(struct net_device *dev)
 		}
 
 		/* mask with MAC supported features */
-		phydev->supported &= (SUPPORTED_10baseT_Half |
-				      SUPPORTED_10baseT_Full |
-				      SUPPORTED_100baseT_Half |
-				      SUPPORTED_100baseT_Full |
-				      SUPPORTED_Autoneg |
-				      SUPPORTED_Pause |
-				      SUPPORTED_MII);
-		phydev->advertising = phydev->supported;
+		phydev->supported |= SUPPORTED_Pause;
+		phy_set_max_speed(phydev, SPEED_100);
 
 		if (priv->pause_auto && priv->pause_rx && priv->pause_tx)
 			phydev->advertising |= SUPPORTED_Pause;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4241ae928d4a..b11d58f1bf45 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -214,7 +214,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 
 	case PHY_INTERFACE_MODE_MII:
 		phy_name = "external MII";
-		phydev->supported &= PHY_BASIC_FEATURES;
+		phy_set_max_speed(phydev, SPEED_100);
 		bcmgenet_sys_writel(priv,
 				    PORT_MODE_EXT_EPHY, SYS_PORT_CTRL);
 		break;
diff --git a/drivers/net/ethernet/broadcom/sb1250-mac.c b/drivers/net/ethernet/broadcom/sb1250-mac.c
index ef4a0c326736..4ce4b097ec05 100644
--- a/drivers/net/ethernet/broadcom/sb1250-mac.c
+++ b/drivers/net/ethernet/broadcom/sb1250-mac.c
@@ -2357,15 +2357,8 @@ static int sbmac_mii_probe(struct net_device *dev)
 	}
 
 	/* Remove any features not supported by the controller */
-	phy_dev->supported &= SUPPORTED_10baseT_Half |
-			      SUPPORTED_10baseT_Full |
-			      SUPPORTED_100baseT_Half |
-			      SUPPORTED_100baseT_Full |
-			      SUPPORTED_1000baseT_Half |
-			      SUPPORTED_1000baseT_Full |
-			      SUPPORTED_Autoneg |
-			      SUPPORTED_MII |
-			      SUPPORTED_Pause |
+	phy_set_max_speed(phy_dev, SPEED_1000);
+	phy_dev->supported |= SUPPORTED_Pause |
 			      SUPPORTED_Asym_Pause;
 
 	phy_attached_info(phy_dev);
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index e6f28c7942ab..cdc32724c9d9 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -2122,15 +2122,15 @@ static int tg3_phy_init(struct tg3 *tp)
 	case PHY_INTERFACE_MODE_GMII:
 	case PHY_INTERFACE_MODE_RGMII:
 		if (!(tp->phy_flags & TG3_PHYFLG_10_100_ONLY)) {
-			phydev->supported &= (PHY_GBIT_FEATURES |
-					      SUPPORTED_Pause |
+			phy_set_max_speed(phydev, SPEED_1000);
+			phydev->supported &= (SUPPORTED_Pause |
 					      SUPPORTED_Asym_Pause);
 			break;
 		}
 		/* fallthru */
 	case PHY_INTERFACE_MODE_MII:
-		phydev->supported &= (PHY_BASIC_FEATURES |
-				      SUPPORTED_Pause |
+		phy_set_max_speed(phydev, SPEED_100);
+		phydev->supported &= (SUPPORTED_Pause |
 				      SUPPORTED_Asym_Pause);
 		break;
 	default:
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 16e4ef7d7185..bd4095c3a031 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -544,9 +544,9 @@ static int macb_mii_probe(struct net_device *dev)
 
 	/* mask with MAC supported features */
 	if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
-		phydev->supported &= PHY_GBIT_FEATURES;
+		phy_set_max_speed(phydev, SPEED_1000);
 	else
-		phydev->supported &= PHY_BASIC_FEATURES;
+		phy_set_max_speed(phydev, SPEED_100);
 
 	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
 		phydev->supported &= ~SUPPORTED_1000baseT_Half;
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 1c9ad3630c77..2b46c0de90d0 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -372,7 +372,7 @@ static int gmac_setup_phy(struct net_device *netdev)
 		return -ENODEV;
 	netdev->phydev = phy;
 
-	phy->supported &= PHY_GBIT_FEATURES;
+	phy_set_max_speed(phy, SPEED_1000);
 	phy->supported |= SUPPORTED_Asym_Pause | SUPPORTED_Pause;
 	phy->advertising = phy->supported;
 
diff --git a/drivers/net/ethernet/dnet.c b/drivers/net/ethernet/dnet.c
index 5a847941c46b..08b7ad1594ce 100644
--- a/drivers/net/ethernet/dnet.c
+++ b/drivers/net/ethernet/dnet.c
@@ -284,9 +284,9 @@ static int dnet_mii_probe(struct net_device *dev)
 
 	/* mask with MAC supported features */
 	if (bp->capabilities & DNET_HAS_GIGABIT)
-		phydev->supported &= PHY_GBIT_FEATURES;
+		phy_set_max_speed(phydev, SPEED_1000);
 	else
-		phydev->supported &= PHY_BASIC_FEATURES;
+		phy_set_max_speed(phydev, SPEED_100);
 
 	phydev->supported |= SUPPORTED_Asym_Pause | SUPPORTED_Pause;
 
diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
index 60da0499ad66..0f3e7f21c6fa 100644
--- a/drivers/net/ethernet/ethoc.c
+++ b/drivers/net/ethernet/ethoc.c
@@ -721,10 +721,7 @@ static int ethoc_mdio_probe(struct net_device *dev)
 		return err;
 	}
 
-	phy->advertising &= ~(ADVERTISED_1000baseT_Full |
-			      ADVERTISED_1000baseT_Half);
-	phy->supported &= ~(SUPPORTED_1000baseT_Full |
-			    SUPPORTED_1000baseT_Half);
+	phy_set_max_speed(phy, SPEED_100);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2708297e7795..5e849510c689 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1946,14 +1946,14 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 
 	/* mask with MAC supported features */
 	if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
-		phy_dev->supported &= PHY_GBIT_FEATURES;
+		phy_set_max_speed(phy_dev, 1000);
 		phy_dev->supported &= ~SUPPORTED_1000baseT_Half;
 #if !defined(CONFIG_M5272)
 		phy_dev->supported |= SUPPORTED_Pause;
 #endif
 	}
 	else
-		phy_dev->supported &= PHY_BASIC_FEATURES;
+		phy_set_max_speed(phy_dev, 100);
 
 	phy_dev->advertising = phy_dev->supported;
 
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 22a817da861e..9600837f21b8 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1742,12 +1742,7 @@ static int init_phy(struct net_device *dev)
 	if (priv->phy_interface == PHY_INTERFACE_MODE_SGMII)
 		uec_configure_serdes(dev);
 
-	phydev->supported &= (SUPPORTED_MII |
-			      SUPPORTED_Autoneg |
-			      ADVERTISED_10baseT_Half |
-			      ADVERTISED_10baseT_Full |
-			      ADVERTISED_100baseT_Half |
-			      ADVERTISED_100baseT_Full);
+	phy_set_max_speed(phydev, SPEED_100);
 
 	if (priv->max_speed == SPEED_1000)
 		phydev->supported |= ADVERTISED_1000baseT_Full;
diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 7a637b51c7d2..7b25ba957d3f 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -364,15 +364,8 @@ ltq_etop_mdio_probe(struct net_device *dev)
 		return PTR_ERR(phydev);
 	}
 
-	phydev->supported &= (SUPPORTED_10baseT_Half
-			      | SUPPORTED_10baseT_Full
-			      | SUPPORTED_100baseT_Half
-			      | SUPPORTED_100baseT_Full
-			      | SUPPORTED_Autoneg
-			      | SUPPORTED_MII
-			      | SUPPORTED_TP);
-
-	phydev->advertising = phydev->supported;
+	phy_set_max_speed(phydev, SPEED_100);
+
 	phy_attached_info(phydev);
 
 	return 0;
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b44bcfd85b05..e93b5375504b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -359,8 +359,8 @@ static int mtk_phy_connect(struct net_device *dev)
 		dev->phydev->supported |=
 		SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 
-	dev->phydev->supported &= PHY_GBIT_FEATURES | SUPPORTED_Pause |
-				   SUPPORTED_Asym_Pause;
+	phy_set_max_speed(dev->phydev, SPEED_1000);
+	dev->phydev->supported &= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 	dev->phydev->advertising = dev->phydev->supported |
 				    ADVERTISED_Autoneg;
 	phy_start_aneg(dev->phydev);
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
index 08381ef8bdb4..8b23d2848457 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -797,8 +797,7 @@ static int lpc_mii_probe(struct net_device *ndev)
 		return PTR_ERR(phydev);
 	}
 
-	/* mask with MAC supported features */
-	phydev->supported &= PHY_BASIC_FEATURES;
+	phy_set_max_speed(phydev, SPEED_100);
 
 	phydev->advertising = phydev->supported;
 
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index aa11b70b9ca4..04aa592f35c3 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -1024,16 +1024,8 @@ static int r6040_mii_probe(struct net_device *dev)
 		return PTR_ERR(phydev);
 	}
 
-	/* mask with MAC supported features */
-	phydev->supported &= (SUPPORTED_10baseT_Half
-				| SUPPORTED_10baseT_Full
-				| SUPPORTED_100baseT_Half
-				| SUPPORTED_100baseT_Full
-				| SUPPORTED_Autoneg
-				| SUPPORTED_MII
-				| SUPPORTED_TP);
-
-	phydev->advertising = phydev->supported;
+	phy_set_max_speed(phydev, SPEED_100);
+
 	lp->old_link = 0;
 	lp->old_duplex = -1;
 
diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
index a9da1ad4b4f2..690aee88f0eb 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
@@ -298,8 +298,8 @@ static int sxgbe_init_phy(struct net_device *ndev)
 	/* Stop Advertising 1000BASE Capability if interface is not GMII */
 	if ((phy_iface == PHY_INTERFACE_MODE_MII) ||
 	    (phy_iface == PHY_INTERFACE_MODE_RMII))
-		phydev->advertising &= ~(SUPPORTED_1000baseT_Half |
-					 SUPPORTED_1000baseT_Full);
+		phy_set_max_speed(phydev, SPEED_1000);
+
 	if (phydev->phy_id == 0) {
 		phy_disconnect(phydev);
 		return -ENODEV;
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index f0afb88d7bc2..f84dbd0beb8e 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -1048,9 +1048,10 @@ static int smsc911x_mii_probe(struct net_device *dev)
 
 	phy_attached_info(phydev);
 
+	phy_set_max_speed(phydev, SPEED_100);
+
 	/* mask with MAC supported features */
-	phydev->supported &= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
-			      SUPPORTED_Asym_Pause);
+	phydev->supported &= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
 	phydev->advertising = phydev->supported;
 
 	pdata->last_duplex = -1;
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index 2fa3c1d03abc..795f60d92611 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -1135,9 +1135,10 @@ static int smsc9420_mii_probe(struct net_device *dev)
 		return PTR_ERR(phydev);
 	}
 
+	phy_set_max_speed(phydev, SPEED_100);
+
 	/* mask with MAC supported features */
-	phydev->supported &= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
-			      SUPPORTED_Asym_Pause);
+	phydev->supported &= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
 	phydev->advertising = phydev->supported;
 
 	phy_attached_info(phydev);
diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index f7ecceeb1e28..76ff364c40e9 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1223,10 +1223,8 @@ static int ave_init(struct net_device *ndev)
 	phy_ethtool_get_wol(phydev, &wol);
 	device_set_wakeup_capable(&ndev->dev, !!wol.supported);
 
-	if (!phy_interface_is_rgmii(phydev)) {
-		phydev->supported &= ~PHY_GBIT_FEATURES;
-		phydev->supported |= PHY_BASIC_FEATURES;
-	}
+	if (!phy_interface_is_rgmii(phydev))
+		phy_set_max_speed(phydev, SPEED_100);
 	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 
 	phy_attached_info(phydev);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9f458bb16f2a..3d7aec7a050b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -987,8 +987,7 @@ static int stmmac_init_phy(struct net_device *dev)
 	if ((interface == PHY_INTERFACE_MODE_MII) ||
 	    (interface == PHY_INTERFACE_MODE_RMII) ||
 		(max_speed < 1000 && max_speed > 0))
-		phydev->advertising &= ~(SUPPORTED_1000baseT_Half |
-					 SUPPORTED_1000baseT_Full);
+		phy_set_max_speed(phydev, SPEED_100);
 
 	/*
 	 * Half-duplex mode not supported with multiqueue
diff --git a/drivers/net/ethernet/toshiba/tc35815.c b/drivers/net/ethernet/toshiba/tc35815.c
index cce9c9ed46aa..7163a8d10dba 100644
--- a/drivers/net/ethernet/toshiba/tc35815.c
+++ b/drivers/net/ethernet/toshiba/tc35815.c
@@ -628,7 +628,7 @@ static int tc_mii_probe(struct net_device *dev)
 	phy_attached_info(phydev);
 
 	/* mask with MAC supported features */
-	phydev->supported &= PHY_BASIC_FEATURES;
+	phy_set_max_speed(phydev, SPEED_100);
 	dropmask = 0;
 	if (options.speed == 10)
 		dropmask |= SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full;
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 42f1f518dad6..46d3092b8aad 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -941,8 +941,7 @@ static int xemaclite_open(struct net_device *dev)
 		}
 
 		/* EmacLite doesn't support giga-bit speeds */
-		lp->phy_dev->supported &= (PHY_BASIC_FEATURES);
-		lp->phy_dev->advertising = lp->phy_dev->supported;
+		phy_set_max_speed(lp->phy_dev, SPEED_100);
 
 		/* Don't advertise 1000BASE-T Full/Half duplex speeds */
 		phy_write(lp->phy_dev, MII_CTRL1000, 0);
diff --git a/drivers/staging/mt7621-eth/mdio.c b/drivers/staging/mt7621-eth/mdio.c
index 7ad0c4141205..2c6e1800a3fd 100644
--- a/drivers/staging/mt7621-eth/mdio.c
+++ b/drivers/staging/mt7621-eth/mdio.c
@@ -112,7 +112,7 @@ static void phy_init(struct mtk_eth *eth, struct mtk_mac *mac,
 	phy->autoneg = AUTONEG_ENABLE;
 	phy->speed = 0;
 	phy->duplex = 0;
-	phy->supported &= PHY_BASIC_FEATURES;
+	phy_set_max_speed(phy, SPEED_100);
 	phy->advertising = phy->supported | ADVERTISED_Autoneg;
 
 	phy_start_aneg(phy);
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 05/12] net: bcmgenet: Fix speed selection for reverse MII
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (3 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 06/12] net: ethernet: Fix up drivers masking pause support Andrew Lunn
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

The phy supported speed is being used to determine if the MAC should
be configured to 100 or 1G. The masking logic is broken. Instead, look
at 1G supported speeds to enable 1G MAC support.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
v3:
Add missing at in commit message
Change subject requested by Florian
---
 drivers/net/ethernet/broadcom/genet/bcmmii.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b11d58f1bf45..b756fc79424e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -226,11 +226,10 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		 * capabilities, use that knowledge to also configure the
 		 * Reverse MII interface correctly.
 		 */
-		if ((dev->phydev->supported & PHY_BASIC_FEATURES) ==
-				PHY_BASIC_FEATURES)
-			port_ctrl = PORT_MODE_EXT_RVMII_25;
-		else
+		if (dev->phydev->supported & PHY_1000BT_FEATURES)
 			port_ctrl = PORT_MODE_EXT_RVMII_50;
+		else
+			port_ctrl = PORT_MODE_EXT_RVMII_25;
 		bcmgenet_sys_writel(priv, port_ctrl, SYS_PORT_CTRL);
 		break;
 
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 06/12] net: ethernet: Fix up drivers masking pause support
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (4 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 05/12] net: bcmgenet: Fix speed selection for reverse MII Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode Andrew Lunn
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

PHY drivers don't indicate they support pause. They expect MAC drivers
to enable its support if the MAC has the needed hardware. Thus MAC
drivers should not mask Pause support, but enable it.

Change a few ANDs to ORs.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/tg3.c                     | 4 ++--
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c | 4 ++--
 drivers/net/ethernet/mediatek/mtk_eth_soc.c             | 2 +-
 drivers/net/ethernet/smsc/smsc911x.c                    | 2 +-
 drivers/net/ethernet/smsc/smsc9420.c                    | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index cdc32724c9d9..eab00239a47a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -2123,14 +2123,14 @@ static int tg3_phy_init(struct tg3 *tp)
 	case PHY_INTERFACE_MODE_RGMII:
 		if (!(tp->phy_flags & TG3_PHYFLG_10_100_ONLY)) {
 			phy_set_max_speed(phydev, SPEED_1000);
-			phydev->supported &= (SUPPORTED_Pause |
+			phydev->supported |= (SUPPORTED_Pause |
 					      SUPPORTED_Asym_Pause);
 			break;
 		}
 		/* fallthru */
 	case PHY_INTERFACE_MODE_MII:
 		phy_set_max_speed(phydev, SPEED_100);
-		phydev->supported &= (SUPPORTED_Pause |
+		phydev->supported |= (SUPPORTED_Pause |
 				      SUPPORTED_Asym_Pause);
 		break;
 	default:
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
index 398971a062f4..05b15d254e32 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -10,8 +10,6 @@
 
 #define HCLGE_PHY_SUPPORTED_FEATURES	(SUPPORTED_Autoneg | \
 					 SUPPORTED_TP | \
-					 SUPPORTED_Pause | \
-					 SUPPORTED_Asym_Pause | \
 					 PHY_10BT_FEATURES | \
 					 PHY_100BT_FEATURES | \
 					 PHY_1000BT_FEATURES)
@@ -213,6 +211,8 @@ int hclge_mac_connect_phy(struct hclge_dev *hdev)
 	}
 
 	phydev->supported &= HCLGE_PHY_SUPPORTED_FEATURES;
+	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
 	phydev->advertising = phydev->supported;
 
 	return 0;
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index e93b5375504b..db231bda7c2a 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -360,7 +360,7 @@ static int mtk_phy_connect(struct net_device *dev)
 		SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 
 	phy_set_max_speed(dev->phydev, SPEED_1000);
-	dev->phydev->supported &= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	dev->phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 	dev->phydev->advertising = dev->phydev->supported |
 				    ADVERTISED_Autoneg;
 	phy_start_aneg(dev->phydev);
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index f84dbd0beb8e..3e34bf53f055 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -1051,7 +1051,7 @@ static int smsc911x_mii_probe(struct net_device *dev)
 	phy_set_max_speed(phydev, SPEED_100);
 
 	/* mask with MAC supported features */
-	phydev->supported &= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+	phydev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
 	phydev->advertising = phydev->supported;
 
 	pdata->last_duplex = -1;
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index 795f60d92611..326177384544 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -1138,7 +1138,7 @@ static int smsc9420_mii_probe(struct net_device *dev)
 	phy_set_max_speed(phydev, SPEED_100);
 
 	/* mask with MAC supported features */
-	phydev->supported &= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+	phydev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
 	phydev->advertising = phydev->supported;
 
 	phy_attached_info(phydev);
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (5 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 06/12] net: ethernet: Fix up drivers masking pause support Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-17 15:13   ` Simon Horman
  2018-09-11 23:53 ` [PATCH v3 net-next 08/12] net: ethernet: Add helper for MACs which support asym pause Andrew Lunn
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Some MAC hardware cannot support a subset of link modes. e.g. often
1Gbps Full duplex is supported, but Half duplex is not. Add a helper
to remove such a link mode.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c |  6 +++---
 drivers/net/ethernet/cadence/macb_main.c       |  5 ++---
 drivers/net/ethernet/freescale/fec_main.c      |  3 ++-
 drivers/net/ethernet/microchip/lan743x_main.c  |  2 +-
 drivers/net/ethernet/renesas/ravb_main.c       |  3 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 12 ++++++++----
 drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
 drivers/net/usb/lan78xx.c                      |  2 +-
 include/linux/phy.h                            |  1 +
 9 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 078a04dc1182..4831f9de5945 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -895,9 +895,9 @@ int xgene_enet_phy_connect(struct net_device *ndev)
 	}
 
 	pdata->phy_speed = SPEED_UNKNOWN;
-	phy_dev->supported &= ~SUPPORTED_10baseT_Half &
-			      ~SUPPORTED_100baseT_Half &
-			      ~SUPPORTED_1000baseT_Half;
+	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 	phy_dev->supported |= SUPPORTED_Pause |
 			      SUPPORTED_Asym_Pause;
 	phy_dev->advertising = phy_dev->supported;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index bd4095c3a031..96ae8c992810 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -549,9 +549,8 @@ static int macb_mii_probe(struct net_device *dev)
 		phy_set_max_speed(phydev, SPEED_100);
 
 	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
-		phydev->supported &= ~SUPPORTED_1000baseT_Half;
-
-	phydev->advertising = phydev->supported;
+		phy_remove_link_mode(phydev,
+				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 
 	bp->link = 0;
 	bp->speed = 0;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5e849510c689..0c6fd77b6599 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1947,7 +1947,8 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 	/* mask with MAC supported features */
 	if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
 		phy_set_max_speed(phy_dev, 1000);
-		phy_dev->supported &= ~SUPPORTED_1000baseT_Half;
+		phy_remove_link_mode(phy_dev,
+				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 #if !defined(CONFIG_M5272)
 		phy_dev->supported |= SUPPORTED_Pause;
 #endif
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index e7dce79ff2c9..048307959c01 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1013,7 +1013,7 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
 		goto return_error;
 
 	/* MAC doesn't support 1000T Half */
-	phydev->supported &= ~SUPPORTED_1000baseT_Half;
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 
 	/* support both flow controls */
 	phy->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX);
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index aff5516b781e..fb2a1125780d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
 	}
 
 	/* 10BASE is not supported */
-	phydev->supported &= ~PHY_10BT_FEATURES;
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
 
 	phy_attached_info(phydev);
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3d7aec7a050b..3715a0a4af3c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -993,10 +993,14 @@ static int stmmac_init_phy(struct net_device *dev)
 	 * Half-duplex mode not supported with multiqueue
 	 * half-duplex can only works with single queue
 	 */
-	if (tx_cnt > 1)
-		phydev->supported &= ~(SUPPORTED_1000baseT_Half |
-				       SUPPORTED_100baseT_Half |
-				       SUPPORTED_10baseT_Half);
+	if (tx_cnt > 1) {
+		phy_remove_link_mode(phydev,
+				     ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+		phy_remove_link_mode(phydev,
+				     ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+		phy_remove_link_mode(phydev,
+				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+	}
 
 	/*
 	 * Broken HW is sometimes missing the pull-up resistor on the
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index db1172db1e7c..e9ca83a438b0 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
 }
 EXPORT_SYMBOL(phy_set_max_speed);
 
+/**
+ * phy_remove_link_mode - Remove a supported link mode
+ * @phydev: phy_device structure to remove link mode from
+ * @link_mode: Link mode to be removed
+ *
+ * Description: Some MACs don't support all link modes which the PHY
+ * does.  e.g. a 1G MAC often does not support 1000Half. Add a helper
+ * to remove a link mode.
+ */
+void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
+{
+	WARN_ON(link_mode > 31);
+
+	phydev->supported &= ~BIT(link_mode);
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_remove_link_mode);
+
 static void of_set_phy_supported(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 3ce3c66559e4..95a98a20b2e3 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2166,7 +2166,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
 	}
 
 	/* MAC doesn't support 1000T Half */
-	phydev->supported &= ~SUPPORTED_1000baseT_Half;
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 
 	/* support both flow controls */
 	dev->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index cd6f637cbbfb..9c4c3eca8cf2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1049,6 +1049,7 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd);
 int phy_start_interrupts(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
 int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
+void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 		       int (*run)(struct phy_device *));
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 08/12] net: ethernet: Add helper for MACs which support asym pause
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (6 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 09/12] net: ethernet: Add helper for MACs which support pause Andrew Lunn
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Rather than have the MAC drivers manipulate phydev members to indicate
they support Asym Pause, add a helper function.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v2: Fixup bad indentation in tg3.c
---
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c         |  4 ++--
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c      |  4 +---
 drivers/net/ethernet/broadcom/sb1250-mac.c          |  5 +----
 drivers/net/ethernet/broadcom/tg3.c                 |  8 ++------
 drivers/net/ethernet/cortina/gemini.c               |  3 +--
 drivers/net/ethernet/dnet.c                         |  4 +---
 drivers/net/ethernet/faraday/ftgmac100.c            |  3 +--
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c      |  3 +--
 drivers/net/ethernet/freescale/gianfar.c            |  4 ++--
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c |  4 +---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c         |  6 +-----
 drivers/net/ethernet/microchip/lan743x_main.c       |  5 +----
 drivers/net/ethernet/smsc/smsc911x.c                |  3 +--
 drivers/net/ethernet/smsc/smsc9420.c                |  3 +--
 drivers/net/ethernet/socionext/sni_ave.c            |  3 ++-
 drivers/net/phy/phy_device.c                        | 13 +++++++++++++
 include/linux/phy.h                                 |  1 +
 17 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index 3ceb4f95ca7c..289129011b9f 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -879,8 +879,8 @@ static bool xgbe_phy_finisar_phy_quirks(struct xgbe_prv_data *pdata)
 	phy_write(phy_data->phydev, 0x00, 0x9140);
 
 	phy_data->phydev->supported = PHY_GBIT_FEATURES;
-	phy_data->phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 	phy_data->phydev->advertising = phy_data->phydev->supported;
+	phy_support_asym_pause(phy_data->phydev);
 
 	netif_dbg(pdata, drv, pdata->netdev,
 		  "Finisar PHY quirk in place\n");
@@ -951,8 +951,8 @@ static bool xgbe_phy_belfuse_phy_quirks(struct xgbe_prv_data *pdata)
 	phy_write(phy_data->phydev, 0x00, reg & ~0x00800);
 
 	phy_data->phydev->supported = PHY_GBIT_FEATURES;
-	phy_data->phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
 	phy_data->phydev->advertising = phy_data->phydev->supported;
+	phy_support_asym_pause(phy_data->phydev);
 
 	netif_dbg(pdata, drv, pdata->netdev,
 		  "BelFuse PHY quirk in place\n");
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 4831f9de5945..e3560311711a 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -898,9 +898,7 @@ int xgene_enet_phy_connect(struct net_device *ndev)
 	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
 	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
 	phy_remove_link_mode(phy_dev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-	phy_dev->supported |= SUPPORTED_Pause |
-			      SUPPORTED_Asym_Pause;
-	phy_dev->advertising = phy_dev->supported;
+	phy_support_asym_pause(phy_dev);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/broadcom/sb1250-mac.c b/drivers/net/ethernet/broadcom/sb1250-mac.c
index 4ce4b097ec05..53acbbb36637 100644
--- a/drivers/net/ethernet/broadcom/sb1250-mac.c
+++ b/drivers/net/ethernet/broadcom/sb1250-mac.c
@@ -2358,13 +2358,10 @@ static int sbmac_mii_probe(struct net_device *dev)
 
 	/* Remove any features not supported by the controller */
 	phy_set_max_speed(phy_dev, SPEED_1000);
-	phy_dev->supported |= SUPPORTED_Pause |
-			      SUPPORTED_Asym_Pause;
+	phy_support_asym_pause(phy_dev);
 
 	phy_attached_info(phy_dev);
 
-	phy_dev->advertising = phy_dev->supported;
-
 	sc->phy_dev = phy_dev;
 
 	return 0;
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index eab00239a47a..193e990fac7a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -2123,15 +2123,13 @@ static int tg3_phy_init(struct tg3 *tp)
 	case PHY_INTERFACE_MODE_RGMII:
 		if (!(tp->phy_flags & TG3_PHYFLG_10_100_ONLY)) {
 			phy_set_max_speed(phydev, SPEED_1000);
-			phydev->supported |= (SUPPORTED_Pause |
-					      SUPPORTED_Asym_Pause);
+			phy_support_asym_pause(phydev);
 			break;
 		}
 		/* fallthru */
 	case PHY_INTERFACE_MODE_MII:
 		phy_set_max_speed(phydev, SPEED_100);
-		phydev->supported |= (SUPPORTED_Pause |
-				      SUPPORTED_Asym_Pause);
+		phy_support_asym_pause(phydev);
 		break;
 	default:
 		phy_disconnect(mdiobus_get_phy(tp->mdio_bus, tp->phy_addr));
@@ -2140,8 +2138,6 @@ static int tg3_phy_init(struct tg3 *tp)
 
 	tp->phy_flags |= TG3_PHYFLG_IS_CONNECTED;
 
-	phydev->advertising = phydev->supported;
-
 	phy_attached_info(phydev);
 
 	return 0;
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 2b46c0de90d0..ceec467f590d 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -373,8 +373,7 @@ static int gmac_setup_phy(struct net_device *netdev)
 	netdev->phydev = phy;
 
 	phy_set_max_speed(phy, SPEED_1000);
-	phy->supported |= SUPPORTED_Asym_Pause | SUPPORTED_Pause;
-	phy->advertising = phy->supported;
+	phy_support_asym_pause(phy);
 
 	/* set PHY interface type */
 	switch (phy->interface) {
diff --git a/drivers/net/ethernet/dnet.c b/drivers/net/ethernet/dnet.c
index 08b7ad1594ce..79521e27f0d1 100644
--- a/drivers/net/ethernet/dnet.c
+++ b/drivers/net/ethernet/dnet.c
@@ -288,9 +288,7 @@ static int dnet_mii_probe(struct net_device *dev)
 	else
 		phy_set_max_speed(phydev, SPEED_100);
 
-	phydev->supported |= SUPPORTED_Asym_Pause | SUPPORTED_Pause;
-
-	phydev->advertising = phydev->supported;
+	phy_support_asym_pause(phydev);
 
 	bp->link = 0;
 	bp->speed = 0;
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index ed6c76d20b45..3f319ee66ab4 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1079,8 +1079,7 @@ static int ftgmac100_mii_probe(struct ftgmac100 *priv, phy_interface_t intf)
 	/* Indicate that we support PAUSE frames (see comment in
 	 * Documentation/networking/phy.txt)
 	 */
-	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-	phydev->advertising = phydev->supported;
+	phy_support_asym_pause(phydev);
 
 	/* Display what we found */
 	phy_attached_info(phydev);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 783134f1b779..a5131a510e8b 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2491,8 +2491,7 @@ static int dpaa_phy_init(struct net_device *net_dev)
 
 	/* Remove any features not supported by the controller */
 	phy_dev->supported &= mac_dev->if_support;
-	phy_dev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
-	phy_dev->advertising = phy_dev->supported;
+	phy_support_asym_pause(phy_dev);
 
 	mac_dev->phy_dev = phy_dev;
 	net_dev->phydev = phy_dev;
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index f27f9bae1a4a..40a1a87cd338 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1814,8 +1814,8 @@ static int init_phy(struct net_device *dev)
 	phydev->supported &= (GFAR_SUPPORTED | gigabit_support);
 	phydev->advertising = phydev->supported;
 
-	/* Add support for flow control, but don't advertise it by default */
-	phydev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+	/* Add support for flow control */
+	phy_support_asym_pause(phydev);
 
 	/* disable EEE autoneg, EEE not supported by eTSEC */
 	memset(&edata, 0, sizeof(struct ethtool_eee));
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
index 05b15d254e32..24b1f2a0c32a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -211,9 +211,7 @@ int hclge_mac_connect_phy(struct hclge_dev *hdev)
 	}
 
 	phydev->supported &= HCLGE_PHY_SUPPORTED_FEATURES;
-	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-
-	phydev->advertising = phydev->supported;
+	phy_support_asym_pause(phydev);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index db231bda7c2a..cc1e9a96a43b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -355,12 +355,8 @@ static int mtk_phy_connect(struct net_device *dev)
 	dev->phydev->speed = 0;
 	dev->phydev->duplex = 0;
 
-	if (of_phy_is_fixed_link(mac->of_node))
-		dev->phydev->supported |=
-		SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-
 	phy_set_max_speed(dev->phydev, SPEED_1000);
-	dev->phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	phy_support_asym_pause(dev->phydev);
 	dev->phydev->advertising = dev->phydev->supported |
 				    ADVERTISED_Autoneg;
 	phy_start_aneg(dev->phydev);
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 048307959c01..b1a0e657febf 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -999,7 +999,6 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
 	struct phy_device *phydev;
 	struct net_device *netdev;
 	int ret = -EIO;
-	u32 mii_adv;
 
 	netdev = adapter->netdev;
 	phydev = phy_find_first(adapter->mdiobus);
@@ -1016,10 +1015,8 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter)
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 
 	/* support both flow controls */
+	phy_support_asym_pause(phydev);
 	phy->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX);
-	phydev->advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-	mii_adv = (u32)mii_advertise_flowctrl(phy->fc_request_control);
-	phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
 	phy->fc_autoneg = phydev->autoneg;
 
 	phy_start(phydev);
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 3e34bf53f055..c009407618d9 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -1051,8 +1051,7 @@ static int smsc911x_mii_probe(struct net_device *dev)
 	phy_set_max_speed(phydev, SPEED_100);
 
 	/* mask with MAC supported features */
-	phydev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
-	phydev->advertising = phydev->supported;
+	phy_support_asym_pause(phydev);
 
 	pdata->last_duplex = -1;
 	pdata->last_carrier = -1;
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index 326177384544..9b6366b20110 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -1138,8 +1138,7 @@ static int smsc9420_mii_probe(struct net_device *dev)
 	phy_set_max_speed(phydev, SPEED_100);
 
 	/* mask with MAC supported features */
-	phydev->supported |= (SUPPORTED_Pause | SUPPORTED_Asym_Pause);
-	phydev->advertising = phydev->supported;
+	phy_support_asym_pause(phydev);
 
 	phy_attached_info(phydev);
 
diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index 76ff364c40e9..a50720ec109c 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1225,7 +1225,8 @@ static int ave_init(struct net_device *ndev)
 
 	if (!phy_interface_is_rgmii(phydev))
 		phy_set_max_speed(phydev, SPEED_100);
-	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
+	phy_support_asym_pause(phydev);
 
 	phy_attached_info(phydev);
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e9ca83a438b0..a0646a66f005 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1783,6 +1783,19 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
 }
 EXPORT_SYMBOL(phy_remove_link_mode);
 
+/**
+ * phy_support_asym_pause - Enable support of asym pause
+ * @phydev: target phy_device struct
+ *
+ * Description: Called by the MAC to indicate is supports Asym Pause.
+ */
+void phy_support_asym_pause(struct phy_device *phydev)
+{
+	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_support_asym_pause);
+
 static void of_set_phy_supported(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9c4c3eca8cf2..e2db819807c1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1050,6 +1050,7 @@ int phy_start_interrupts(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
 int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
 void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
+void phy_support_asym_pause(struct phy_device *phydev);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 		       int (*run)(struct phy_device *));
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 09/12] net: ethernet: Add helper for MACs which support pause
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (7 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 08/12] net: ethernet: Add helper for MACs which support asym pause Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause Andrew Lunn
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Rather than have the MAC drivers manipulate phydev members, add a
helper function for MACs supporting Pause, but not Asym Pause.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v2: rename phy_support_pause() to phy_support_sym_pause()
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c |  2 +-
 drivers/net/ethernet/freescale/fec_main.c    |  4 +---
 drivers/net/phy/phy_device.c                 | 14 ++++++++++++++
 include/linux/phy.h                          |  1 +
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 2eee9459c2cf..9f25667c38e6 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -890,7 +890,7 @@ static int bcm_enet_open(struct net_device *dev)
 		}
 
 		/* mask with MAC supported features */
-		phydev->supported |= SUPPORTED_Pause;
+		phy_support_sym_pause(phydev);
 		phy_set_max_speed(phydev, SPEED_100);
 
 		if (priv->pause_auto && priv->pause_rx && priv->pause_tx)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 0c6fd77b6599..05ce0903391a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1950,14 +1950,12 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 		phy_remove_link_mode(phy_dev,
 				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
 #if !defined(CONFIG_M5272)
-		phy_dev->supported |= SUPPORTED_Pause;
+		phy_support_sym_pause(phy_dev);
 #endif
 	}
 	else
 		phy_set_max_speed(phy_dev, 100);
 
-	phy_dev->advertising = phy_dev->supported;
-
 	fep->link = 0;
 	fep->full_duplex = 0;
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a0646a66f005..e657d5ae2ab8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1783,6 +1783,20 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
 }
 EXPORT_SYMBOL(phy_remove_link_mode);
 
+/**
+ * phy_support_sym_pause - Enable support of symmetrical pause
+ * @phydev: target phy_device struct
+ *
+ * Description: Called by the MAC to indicate is supports symmetrical
+ * Pause, but not asym pause.
+ */
+void phy_support_sym_pause(struct phy_device *phydev)
+{
+	phydev->supported |= SUPPORTED_Pause;
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_support_sym_pause);
+
 /**
  * phy_support_asym_pause - Enable support of asym pause
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e2db819807c1..bc5d6c3f1388 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1050,6 +1050,7 @@ int phy_start_interrupts(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
 int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
 void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
+void phy_support_sym_pause(struct phy_device *phydev);
 void phy_support_asym_pause(struct phy_device *phydev);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (8 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 09/12] net: ethernet: Add helper for MACs which support pause Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 11/12] net: ethernet: Add helper for set_pauseparam for Pause Andrew Lunn
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

ethtool can be used to enable/disable pause. Add a helper to configure
the PHY when asym pause is supported.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v3: Fix return from phy_set_asym_pause found by 0-day.
v2: Also trigger autoneg if the advertising settings have changed.
---
 .../ethernet/apm/xgene/xgene_enet_ethtool.c   | 26 ++--------
 drivers/net/ethernet/aurora/nb8800.c          |  9 +---
 drivers/net/ethernet/broadcom/tg3.c           | 43 +++++-----------
 drivers/net/ethernet/faraday/ftgmac100.c      | 17 ++-----
 .../ethernet/freescale/dpaa/dpaa_ethtool.c    | 23 +--------
 .../net/ethernet/freescale/gianfar_ethtool.c  | 49 ++++++-------------
 .../hisilicon/hns3/hns3pf/hclge_main.c        |  8 +--
 drivers/net/ethernet/socionext/sni_ave.c      | 11 +----
 drivers/net/phy/phy_device.c                  | 30 ++++++++++++
 include/linux/phy.h                           |  1 +
 10 files changed, 69 insertions(+), 148 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index 4f50f11718f4..dfe03afd00b0 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -306,7 +306,6 @@ static int xgene_set_pauseparam(struct net_device *ndev,
 {
 	struct xgene_enet_pdata *pdata = netdev_priv(ndev);
 	struct phy_device *phydev = ndev->phydev;
-	u32 oldadv, newadv;
 
 	if (phy_interface_mode_is_rgmii(pdata->phy_mode) ||
 	    pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) {
@@ -322,29 +321,12 @@ static int xgene_set_pauseparam(struct net_device *ndev,
 		pdata->tx_pause = pp->tx_pause;
 		pdata->rx_pause = pp->rx_pause;
 
-		oldadv = phydev->advertising;
-		newadv = oldadv & ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
+		phy_set_asym_pause(phydev, pp->rx_pause,  pp->tx_pause);
 
-		if (pp->rx_pause)
-			newadv |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
-
-		if (pp->tx_pause)
-			newadv ^= ADVERTISED_Asym_Pause;
-
-		if (oldadv ^ newadv) {
-			phydev->advertising = newadv;
-
-			if (phydev->autoneg)
-				return phy_start_aneg(phydev);
-
-			if (!pp->autoneg) {
-				pdata->mac_ops->flowctl_tx(pdata,
-							   pdata->tx_pause);
-				pdata->mac_ops->flowctl_rx(pdata,
-							   pdata->rx_pause);
-			}
+		if (!pp->autoneg) {
+			pdata->mac_ops->flowctl_tx(pdata, pdata->tx_pause);
+			pdata->mac_ops->flowctl_rx(pdata, pdata->rx_pause);
 		}
-
 	} else {
 		if (pp->autoneg)
 			return -EINVAL;
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index c8d1f8fa4713..6f56276015a4 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -935,18 +935,11 @@ static void nb8800_pause_adv(struct net_device *dev)
 {
 	struct nb8800_priv *priv = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
-	u32 adv = 0;
 
 	if (!phydev)
 		return;
 
-	if (priv->pause_rx)
-		adv |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
-	if (priv->pause_tx)
-		adv ^= ADVERTISED_Asym_Pause;
-
-	phydev->supported |= adv;
-	phydev->advertising |= adv;
+	phy_set_asym_pause(phydev, priv->pause_rx, priv->pause_tx);
 }
 
 static int nb8800_open(struct net_device *dev)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 193e990fac7a..b2a3d008e1df 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12492,7 +12492,6 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
 		tg3_warn_mgmt_link_flap(tp);
 
 	if (tg3_flag(tp, USE_PHYLIB)) {
-		u32 newadv;
 		struct phy_device *phydev;
 
 		phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
@@ -12503,20 +12502,16 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
 			return -EINVAL;
 
 		tp->link_config.flowctrl = 0;
+		phy_set_asym_pause(phydev, epause->rx_pause, epause->tx_pause);
 		if (epause->rx_pause) {
 			tp->link_config.flowctrl |= FLOW_CTRL_RX;
 
 			if (epause->tx_pause) {
 				tp->link_config.flowctrl |= FLOW_CTRL_TX;
-				newadv = ADVERTISED_Pause;
-			} else
-				newadv = ADVERTISED_Pause |
-					 ADVERTISED_Asym_Pause;
+			}
 		} else if (epause->tx_pause) {
 			tp->link_config.flowctrl |= FLOW_CTRL_TX;
-			newadv = ADVERTISED_Asym_Pause;
-		} else
-			newadv = 0;
+		}
 
 		if (epause->autoneg)
 			tg3_flag_set(tp, PAUSE_AUTONEG);
@@ -12524,33 +12519,19 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
 			tg3_flag_clear(tp, PAUSE_AUTONEG);
 
 		if (tp->phy_flags & TG3_PHYFLG_IS_CONNECTED) {
-			u32 oldadv = phydev->advertising &
-				     (ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-			if (oldadv != newadv) {
-				phydev->advertising &=
-					~(ADVERTISED_Pause |
-					  ADVERTISED_Asym_Pause);
-				phydev->advertising |= newadv;
-				if (phydev->autoneg) {
-					/*
-					 * Always renegotiate the link to
-					 * inform our link partner of our
-					 * flow control settings, even if the
-					 * flow control is forced.  Let
-					 * tg3_adjust_link() do the final
-					 * flow control setup.
-					 */
-					return phy_start_aneg(phydev);
-				}
+			if (phydev->autoneg) {
+				/* phy_set_asym_pause() will
+				 * renegotiate the link to inform our
+				 * link partner of our flow control
+				 * settings, even if the flow control
+				 * is forced.  Let tg3_adjust_link()
+				 * do the final flow control setup.
+				 */
+				return 0;
 			}
 
 			if (!epause->autoneg)
 				tg3_setup_flow_control(tp, 0, 0);
-		} else {
-			tp->link_config.advertising &=
-					~(ADVERTISED_Pause |
-					  ADVERTISED_Asym_Pause);
-			tp->link_config.advertising |= newadv;
 		}
 	} else {
 		int irq_sync = 0;
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 3f319ee66ab4..d8ead7e4177e 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1219,22 +1219,11 @@ static int ftgmac100_set_pauseparam(struct net_device *netdev,
 	priv->tx_pause = pause->tx_pause;
 	priv->rx_pause = pause->rx_pause;
 
-	if (phydev) {
-		phydev->advertising &= ~ADVERTISED_Pause;
-		phydev->advertising &= ~ADVERTISED_Asym_Pause;
+	if (phydev)
+		phy_set_asym_pause(phydev, pause->rx_pause, pause->tx_pause);
 
-		if (pause->rx_pause) {
-			phydev->advertising |= ADVERTISED_Pause;
-			phydev->advertising |= ADVERTISED_Asym_Pause;
-		}
-
-		if (pause->tx_pause)
-			phydev->advertising ^= ADVERTISED_Asym_Pause;
-	}
 	if (netif_running(netdev)) {
-		if (phydev && priv->aneg_pause)
-			phy_start_aneg(phydev);
-		else
+		if (!(phydev && priv->aneg_pause))
 			ftgmac100_config_pause(priv);
 	}
 
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 3184c8f7cdd0..1f8cdbc4378c 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -210,29 +210,8 @@ static int dpaa_set_pauseparam(struct net_device *net_dev,
 	/* Determine the sym/asym advertised PAUSE capabilities from the desired
 	 * rx/tx pause settings.
 	 */
-	newadv = 0;
-	if (epause->rx_pause)
-		newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause;
-	if (epause->tx_pause)
-		newadv ^= ADVERTISED_Asym_Pause;
 
-	oldadv = phydev->advertising &
-			(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-
-	/* If there are differences between the old and the new advertised
-	 * values, restart PHY autonegotiation and advertise the new values.
-	 */
-	if (oldadv != newadv) {
-		phydev->advertising &= ~(ADVERTISED_Pause
-				| ADVERTISED_Asym_Pause);
-		phydev->advertising |= newadv;
-		if (phydev->autoneg) {
-			err = phy_start_aneg(phydev);
-			if (err < 0)
-				netdev_err(net_dev, "phy_start_aneg() = %d\n",
-					   err);
-		}
-	}
+	phy_set_asym_pause(phydev, epause->rx_pause, epause->tx_pause);
 
 	fman_get_pause_cfg(mac_dev, &rx_pause, &tx_pause);
 	err = fman_set_mac_active_pause(mac_dev, rx_pause, tx_pause);
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 395a5266ea30..3545e8f715f2 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -503,7 +503,6 @@ static int gfar_spauseparam(struct net_device *dev,
 	struct gfar_private *priv = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
 	struct gfar __iomem *regs = priv->gfargrp[0].regs;
-	u32 oldadv, newadv;
 
 	if (!phydev)
 		return -ENODEV;
@@ -514,54 +513,36 @@ static int gfar_spauseparam(struct net_device *dev,
 		return -EINVAL;
 
 	priv->rx_pause_en = priv->tx_pause_en = 0;
+	phy_set_asym_pause(phydev, epause->rx_pause, epause->tx_pause);
 	if (epause->rx_pause) {
 		priv->rx_pause_en = 1;
 
 		if (epause->tx_pause) {
 			priv->tx_pause_en = 1;
-			/* FLOW_CTRL_RX & TX */
-			newadv = ADVERTISED_Pause;
-		} else  /* FLOW_CTLR_RX */
-			newadv = ADVERTISED_Pause | ADVERTISED_Asym_Pause;
+		}
 	} else if (epause->tx_pause) {
 		priv->tx_pause_en = 1;
-		/* FLOW_CTLR_TX */
-		newadv = ADVERTISED_Asym_Pause;
-	} else
-		newadv = 0;
+	}
 
 	if (epause->autoneg)
 		priv->pause_aneg_en = 1;
 	else
 		priv->pause_aneg_en = 0;
 
-	oldadv = phydev->advertising &
-		(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-	if (oldadv != newadv) {
-		phydev->advertising &=
-			~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-		phydev->advertising |= newadv;
-		if (phydev->autoneg)
-			/* inform link partner of our
-			 * new flow ctrl settings
-			 */
-			return phy_start_aneg(phydev);
-
-		if (!epause->autoneg) {
-			u32 tempval;
-			tempval = gfar_read(&regs->maccfg1);
-			tempval &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
-
-			priv->tx_actual_en = 0;
-			if (priv->tx_pause_en) {
-				priv->tx_actual_en = 1;
-				tempval |= MACCFG1_TX_FLOW;
-			}
+	if (!epause->autoneg) {
+		u32 tempval = gfar_read(&regs->maccfg1);
 
-			if (priv->rx_pause_en)
-				tempval |= MACCFG1_RX_FLOW;
-			gfar_write(&regs->maccfg1, tempval);
+		tempval &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
+
+		priv->tx_actual_en = 0;
+		if (priv->tx_pause_en) {
+			priv->tx_actual_en = 1;
+			tempval |= MACCFG1_TX_FLOW;
 		}
+
+		if (priv->rx_pause_en)
+			tempval |= MACCFG1_RX_FLOW;
+		gfar_write(&regs->maccfg1, tempval);
 	}
 
 	return 0;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index c56db06b63e0..cf18608669f5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5228,13 +5228,7 @@ static void hclge_set_flowctrl_adv(struct hclge_dev *hdev, u32 rx_en, u32 tx_en)
 	if (!phydev)
 		return;
 
-	phydev->advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-
-	if (rx_en)
-		phydev->advertising |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
-
-	if (tx_en)
-		phydev->advertising ^= ADVERTISED_Asym_Pause;
+	phy_set_asym_pause(phydev, rx_en, tx_en);
 }
 
 static int hclge_cfg_pauseparam(struct hclge_dev *hdev, u32 rx_en, u32 tx_en)
diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index a50720ec109c..61e6abb966ac 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -461,16 +461,7 @@ static int ave_ethtool_set_pauseparam(struct net_device *ndev,
 	priv->pause_rx   = pause->rx_pause;
 	priv->pause_tx   = pause->tx_pause;
 
-	phydev->advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
-	if (pause->rx_pause)
-		phydev->advertising |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
-	if (pause->tx_pause)
-		phydev->advertising ^= ADVERTISED_Asym_Pause;
-
-	if (pause->autoneg) {
-		if (netif_running(ndev))
-			phy_start_aneg(phydev);
-	}
+	phy_set_asym_pause(phydev, pause->rx_pause, pause->tx_pause);
 
 	return 0;
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e657d5ae2ab8..5732d89c8e37 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1810,6 +1810,36 @@ void phy_support_asym_pause(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_support_asym_pause);
 
+/**
+ * phy_set_asym_pause - Configure Pause and Asym Pause
+ * @phydev: target phy_device struct
+ * @rx: Receiver Pause is supported
+ * @tx: Transmit Pause is supported
+ *
+ * Description: Configure advertised Pause support depending on if
+ * transmit and receiver pause is supported. If there has been a
+ * change in adverting, trigger a new autoneg. Generally called from
+ * the set_pauseparam .ndo.
+ */
+void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
+{
+	u16 oldadv = phydev->advertising;
+	u16 newadv = oldadv &= ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+
+	if (rx)
+		newadv |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	if (tx)
+		newadv ^= SUPPORTED_Asym_Pause;
+
+	if (oldadv != newadv) {
+		phydev->advertising = newadv;
+
+		if (phydev->autoneg)
+			phy_start_aneg(phydev);
+	}
+}
+EXPORT_SYMBOL(phy_set_asym_pause);
+
 static void of_set_phy_supported(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bc5d6c3f1388..e4062ba7472f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1052,6 +1052,7 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
 void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
 void phy_support_sym_pause(struct phy_device *phydev);
 void phy_support_asym_pause(struct phy_device *phydev);
+void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 		       int (*run)(struct phy_device *));
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 11/12] net: ethernet: Add helper for set_pauseparam for Pause
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (9 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-11 23:53 ` [PATCH v3 net-next 12/12] net: ethernet: Add helper to determine if pause configuration is supported Andrew Lunn
  2018-09-13  3:24 ` [PATCH v3 net-next 00/12] Preparing for phylib limkmodes David Miller
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

ethtool can be used to enable/disable pause. Add a helper to configure
the PHY when Pause is supported.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v3:
  Fix kerndoc, missing parameter.
v2:
  Rename phy_set_pause() to phy_set_sym_pause()
  Use the bcm63xx_enet.c logic, not fec_main.c
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c |  7 ++----
 drivers/net/ethernet/freescale/fec_main.c    |  9 ++------
 drivers/net/phy/phy_device.c                 | 23 ++++++++++++++++++++
 include/linux/phy.h                          |  2 ++
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 9f25667c38e6..02e7dfc1a2ef 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -892,11 +892,8 @@ static int bcm_enet_open(struct net_device *dev)
 		/* mask with MAC supported features */
 		phy_support_sym_pause(phydev);
 		phy_set_max_speed(phydev, SPEED_100);
-
-		if (priv->pause_auto && priv->pause_rx && priv->pause_tx)
-			phydev->advertising |= SUPPORTED_Pause;
-		else
-			phydev->advertising &= ~SUPPORTED_Pause;
+		phy_set_sym_pause(phydev, priv->pause_rx, priv->pause_rx,
+				  priv->pause_auto);
 
 		phy_attached_info(phydev);
 
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 05ce0903391a..2e0bb90131b6 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2229,13 +2229,8 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 	fep->pause_flag |= pause->rx_pause ? FEC_PAUSE_FLAG_ENABLE : 0;
 	fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
 
-	if (pause->rx_pause || pause->autoneg) {
-		ndev->phydev->supported |= ADVERTISED_Pause;
-		ndev->phydev->advertising |= ADVERTISED_Pause;
-	} else {
-		ndev->phydev->supported &= ~ADVERTISED_Pause;
-		ndev->phydev->advertising &= ~ADVERTISED_Pause;
-	}
+	phy_set_sym_pause(ndev->phydev, pause->rx_pause, pause->tx_pause,
+			  pause->autoneg);
 
 	if (pause->autoneg) {
 		if (netif_running(ndev))
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5732d89c8e37..de95f1e072e9 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1810,6 +1810,29 @@ void phy_support_asym_pause(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_support_asym_pause);
 
+/**
+ * phy_set_sym_pause - Configure symmetric Pause
+ * @phydev: target phy_device struct
+ * @rx: Receiver Pause is supported
+ * @tx: Transmit Pause is supported
+ * @autoneg: Auto neg should be used
+ *
+ * Description: Configure advertised Pause support depending on if
+ * receiver pause and pause auto neg is supported. Generally called
+ * from the set_pauseparam .ndo.
+ */
+void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
+		       bool autoneg)
+{
+	phydev->supported &= ~SUPPORTED_Pause;
+
+	if (rx && tx && autoneg)
+		phydev->supported |= SUPPORTED_Pause;
+
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_set_sym_pause);
+
 /**
  * phy_set_asym_pause - Configure Pause and Asym Pause
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e4062ba7472f..8521391ebb20 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1052,6 +1052,8 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
 void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
 void phy_support_sym_pause(struct phy_device *phydev);
 void phy_support_asym_pause(struct phy_device *phydev);
+void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
+		       bool autoneg);
 void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 net-next 12/12] net: ethernet: Add helper to determine if pause configuration is supported
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (10 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 11/12] net: ethernet: Add helper for set_pauseparam for Pause Andrew Lunn
@ 2018-09-11 23:53 ` Andrew Lunn
  2018-09-13  3:24 ` [PATCH v3 net-next 00/12] Preparing for phylib limkmodes David Miller
  12 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-11 23:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Rather than have MAC drivers open code the test, add a helper in
phylib. This will help when we change the type of phydev->supported.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../ethernet/apm/xgene/xgene_enet_ethtool.c   |  4 +---
 drivers/net/ethernet/broadcom/tg3.c           |  4 +---
 .../ethernet/freescale/dpaa/dpaa_ethtool.c    |  4 +---
 .../net/ethernet/freescale/gianfar_ethtool.c  |  4 +---
 drivers/net/phy/phy_device.c                  | 20 +++++++++++++++++++
 include/linux/phy.h                           |  2 ++
 6 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index dfe03afd00b0..78dd09b5beeb 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -312,9 +312,7 @@ static int xgene_set_pauseparam(struct net_device *ndev,
 		if (!phydev)
 			return -EINVAL;
 
-		if (!(phydev->supported & SUPPORTED_Pause) ||
-		    (!(phydev->supported & SUPPORTED_Asym_Pause) &&
-		     pp->rx_pause != pp->tx_pause))
+		if (!phy_validate_pause(phydev, pp))
 			return -EINVAL;
 
 		pdata->pause_autoneg = pp->autoneg;
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index b2a3d008e1df..fb0e458e25b7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12496,9 +12496,7 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
 
 		phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
 
-		if (!(phydev->supported & SUPPORTED_Pause) ||
-		    (!(phydev->supported & SUPPORTED_Asym_Pause) &&
-		     (epause->rx_pause != epause->tx_pause)))
+		if (!phy_validate_pause(phydev, epause))
 			return -EINVAL;
 
 		tp->link_config.flowctrl = 0;
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 1f8cdbc4378c..5d0fdf667b82 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -194,9 +194,7 @@ static int dpaa_set_pauseparam(struct net_device *net_dev,
 		return -ENODEV;
 	}
 
-	if (!(phydev->supported & SUPPORTED_Pause) ||
-	    (!(phydev->supported & SUPPORTED_Asym_Pause) &&
-	    (epause->rx_pause != epause->tx_pause)))
+	if (!phy_validate_pause(phydev, epause))
 		return -EINVAL;
 
 	/* The MAC should know how to handle PAUSE frame autonegotiation before
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 3545e8f715f2..d3662965f59d 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -507,9 +507,7 @@ static int gfar_spauseparam(struct net_device *dev,
 	if (!phydev)
 		return -ENODEV;
 
-	if (!(phydev->supported & SUPPORTED_Pause) ||
-	    (!(phydev->supported & SUPPORTED_Asym_Pause) &&
-	     (epause->rx_pause != epause->tx_pause)))
+	if (!phy_validate_pause(phydev, epause))
 		return -EINVAL;
 
 	priv->rx_pause_en = priv->tx_pause_en = 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index de95f1e072e9..af64a9320fb0 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1863,6 +1863,26 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
 }
 EXPORT_SYMBOL(phy_set_asym_pause);
 
+/**
+ * phy_validate_pause - Test if the PHY/MAC support the pause configuration
+ * @phydev: phy_device struct
+ * @pp: requested pause configuration
+ *
+ * Description: Test if the PHY/MAC combination supports the Pause
+ * configuration the user is requesting. Returns True if it is
+ * supported, false otherwise.
+ */
+bool phy_validate_pause(struct phy_device *phydev,
+			struct ethtool_pauseparam *pp)
+{
+	if (!(phydev->supported & SUPPORTED_Pause) ||
+	    (!(phydev->supported & SUPPORTED_Asym_Pause) &&
+	     pp->rx_pause != pp->tx_pause))
+		return false;
+	return true;
+}
+EXPORT_SYMBOL(phy_validate_pause);
+
 static void of_set_phy_supported(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8521391ebb20..192a1fa0c73b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1055,6 +1055,8 @@ void phy_support_asym_pause(struct phy_device *phydev);
 void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx,
 		       bool autoneg);
 void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
+bool phy_validate_pause(struct phy_device *phydev,
+			struct ethtool_pauseparam *pp);
 
 int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
 		       int (*run)(struct phy_device *));
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 00/12] Preparing for phylib limkmodes
  2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
                   ` (11 preceding siblings ...)
  2018-09-11 23:53 ` [PATCH v3 net-next 12/12] net: ethernet: Add helper to determine if pause configuration is supported Andrew Lunn
@ 2018-09-13  3:24 ` David Miller
  12 siblings, 0 replies; 34+ messages in thread
From: David Miller @ 2018-09-13  3:24 UTC (permalink / raw)
  To: andrew; +Cc: netdev, f.fainelli

From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 12 Sep 2018 01:53:07 +0200

> phylib currently makes us of a u32 bitmap for advertising, supported,
> and link partner capabilities. For a long time, this has been
> sufficient, for devices up to 1Gbps. With more MAC/PHY combinations
> now supporting speeds greater than 1Gbps, we have run out of
> bits. There is the need to replace this u32 with an
> __ETHTOOL_DECLARE_LINK_MODE_MASK, which makes use of linux's generic
> bitmaps.
> 
> This patchset does some of the work preparing for this change. A few
> cleanups are applied to PHY drivers. Some MAC drivers directly access
> members of phydev which are going to change type. These patches adds
> some helpers and swaps MAC drivers to use them, mostly dealing with
> Pause configuration.
> 
> v3:
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Add missing at in commit message
> Change Subject of patch 5
> Fix return in from phy_set_asym_pause
> Fix kerneldoc in phy_set_pause
> 
> v2:
> Fixup bad indentation in tg3.c
> Rename phy_support_pause() to phy_support_sym_pause()
> Also trigger autoneg if the advertising settings have changed.
> Rename phy_set_pause() to phy_set_sym_pause()
> Use the bcm63xx_enet.c logic, not fec_main.c for validating pause

Series applied, thanks Andrew.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-11 23:53 ` [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode Andrew Lunn
@ 2018-09-17 15:13   ` Simon Horman
  2018-09-17 15:38     ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-17 15:13 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> Some MAC hardware cannot support a subset of link modes. e.g. often
> 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> to remove such a link mode.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c |  6 +++---
>  drivers/net/ethernet/cadence/macb_main.c       |  5 ++---
>  drivers/net/ethernet/freescale/fec_main.c      |  3 ++-
>  drivers/net/ethernet/microchip/lan743x_main.c  |  2 +-
>  drivers/net/ethernet/renesas/ravb_main.c       |  3 ++-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 12 ++++++++----
>  drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
>  drivers/net/usb/lan78xx.c                      |  2 +-
>  include/linux/phy.h                            |  1 +
>  9 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> index 078a04dc1182..4831f9de5945 100644

...

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index aff5516b781e..fb2a1125780d 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
>  	}
>  
>  	/* 10BASE is not supported */
> -	phydev->supported &= ~PHY_10BT_FEATURES;
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
>  
>  	phy_attached_info(phydev);
>  

...

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index db1172db1e7c..e9ca83a438b0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
>  }
>  EXPORT_SYMBOL(phy_set_max_speed);
>  
> +/**
> + * phy_remove_link_mode - Remove a supported link mode
> + * @phydev: phy_device structure to remove link mode from
> + * @link_mode: Link mode to be removed
> + *
> + * Description: Some MACs don't support all link modes which the PHY
> + * does.  e.g. a 1G MAC often does not support 1000Half. Add a helper
> + * to remove a link mode.
> + */
> +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> +{
> +	WARN_ON(link_mode > 31);
> +
> +	phydev->supported &= ~BIT(link_mode);
> +	phydev->advertising = phydev->supported;
> +}
> +EXPORT_SYMBOL(phy_remove_link_mode);
> +
>  static void of_set_phy_supported(struct phy_device *phydev)
>  {
>  	struct device_node *node = phydev->mdio.dev.of_node;

Hi Andrew,

I believe that for the RAVB the overall effect of this change is that
10-BaseT modes are no longer advertised (although both with and without
this patch they are not supported).

Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
I have observed that this results in the link no longer being negotiated
on one switch (the one I usually use) while it seemed fine on another.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-17 15:13   ` Simon Horman
@ 2018-09-17 15:38     ` Andrew Lunn
  2018-09-18 10:58       ` Simon Horman
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-17 15:38 UTC (permalink / raw)
  To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli

On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > Some MAC hardware cannot support a subset of link modes. e.g. often
> > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > to remove such a link mode.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> >  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c |  6 +++---
> >  drivers/net/ethernet/cadence/macb_main.c       |  5 ++---
> >  drivers/net/ethernet/freescale/fec_main.c      |  3 ++-
> >  drivers/net/ethernet/microchip/lan743x_main.c  |  2 +-
> >  drivers/net/ethernet/renesas/ravb_main.c       |  3 ++-
> >  .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 12 ++++++++----
> >  drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
> >  drivers/net/usb/lan78xx.c                      |  2 +-
> >  include/linux/phy.h                            |  1 +
> >  9 files changed, 38 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > index 078a04dc1182..4831f9de5945 100644
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > index aff5516b781e..fb2a1125780d 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> >  	}
> >  
> >  	/* 10BASE is not supported */
> > -	phydev->supported &= ~PHY_10BT_FEATURES;
> > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> >  
> >  	phy_attached_info(phydev);
> >  
> 
> ...
> 
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index db1172db1e7c..e9ca83a438b0 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> >  }
> >  EXPORT_SYMBOL(phy_set_max_speed);
> >  
> > +/**
> > + * phy_remove_link_mode - Remove a supported link mode
> > + * @phydev: phy_device structure to remove link mode from
> > + * @link_mode: Link mode to be removed
> > + *
> > + * Description: Some MACs don't support all link modes which the PHY
> > + * does.  e.g. a 1G MAC often does not support 1000Half. Add a helper
> > + * to remove a link mode.
> > + */
> > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > +{
> > +	WARN_ON(link_mode > 31);
> > +
> > +	phydev->supported &= ~BIT(link_mode);
> > +	phydev->advertising = phydev->supported;
> > +}
> > +EXPORT_SYMBOL(phy_remove_link_mode);
> > +
> >  static void of_set_phy_supported(struct phy_device *phydev)
> >  {
> >  	struct device_node *node = phydev->mdio.dev.of_node;
> 
> Hi Andrew,
> 
> I believe that for the RAVB the overall effect of this change is that
> 10-BaseT modes are no longer advertised (although both with and without
> this patch they are not supported).
> 
> Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> I have observed that this results in the link no longer being negotiated
> on one switch (the one I usually use) while it seemed fine on another.

Hi Simon

Thanks for testing this.

Could you dump the PHY registers with and without this patch:

$ mii-tool -vv eth0

Once difference is that phy_remove_link_mode() does
phydev->advertising = phydev->supported where as the old code does
not. I though phylib would do this anyway, it does at some point in
time, but i didn't check when. It could be you are actually
advertising 10, even if you don't support it.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-17 15:38     ` Andrew Lunn
@ 2018-09-18 10:58       ` Simon Horman
  2018-09-18 13:02         ` Andrew Lunn
  2018-09-20 13:25         ` Andrew Lunn
  0 siblings, 2 replies; 34+ messages in thread
From: Simon Horman @ 2018-09-18 10:58 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

On Mon, Sep 17, 2018 at 05:38:11PM +0200, Andrew Lunn wrote:
> On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> > On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > > Some MAC hardware cannot support a subset of link modes. e.g. often
> > > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > > to remove such a link mode.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > >  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c |  6 +++---
> > >  drivers/net/ethernet/cadence/macb_main.c       |  5 ++---
> > >  drivers/net/ethernet/freescale/fec_main.c      |  3 ++-
> > >  drivers/net/ethernet/microchip/lan743x_main.c  |  2 +-
> > >  drivers/net/ethernet/renesas/ravb_main.c       |  3 ++-
> > >  .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 12 ++++++++----
> > >  drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
> > >  drivers/net/usb/lan78xx.c                      |  2 +-
> > >  include/linux/phy.h                            |  1 +
> > >  9 files changed, 38 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > > index 078a04dc1182..4831f9de5945 100644
> > 
> > ...
> > 
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > > index aff5516b781e..fb2a1125780d 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> > >  	}
> > >  
> > >  	/* 10BASE is not supported */
> > > -	phydev->supported &= ~PHY_10BT_FEATURES;
> > > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> > >  
> > >  	phy_attached_info(phydev);
> > >  
> > 
> > ...
> > 
> > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > > index db1172db1e7c..e9ca83a438b0 100644
> > > --- a/drivers/net/phy/phy_device.c
> > > +++ b/drivers/net/phy/phy_device.c
> > > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> > >  }
> > >  EXPORT_SYMBOL(phy_set_max_speed);
> > >  
> > > +/**
> > > + * phy_remove_link_mode - Remove a supported link mode
> > > + * @phydev: phy_device structure to remove link mode from
> > > + * @link_mode: Link mode to be removed
> > > + *
> > > + * Description: Some MACs don't support all link modes which the PHY
> > > + * does.  e.g. a 1G MAC often does not support 1000Half. Add a helper
> > > + * to remove a link mode.
> > > + */
> > > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > > +{
> > > +	WARN_ON(link_mode > 31);
> > > +
> > > +	phydev->supported &= ~BIT(link_mode);
> > > +	phydev->advertising = phydev->supported;
> > > +}
> > > +EXPORT_SYMBOL(phy_remove_link_mode);
> > > +
> > >  static void of_set_phy_supported(struct phy_device *phydev)
> > >  {
> > >  	struct device_node *node = phydev->mdio.dev.of_node;
> > 
> > Hi Andrew,
> > 
> > I believe that for the RAVB the overall effect of this change is that
> > 10-BaseT modes are no longer advertised (although both with and without
> > this patch they are not supported).
> > 
> > Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> > I have observed that this results in the link no longer being negotiated
> > on one switch (the one I usually use) while it seemed fine on another.
> 
> Hi Simon
> 
> Thanks for testing this.
> 
> Could you dump the PHY registers with and without this patch:
> 
> $ mii-tool -vv eth0
> 
> Once difference is that phy_remove_link_mode() does
> phydev->advertising = phydev->supported where as the old code does
> not. I though phylib would do this anyway, it does at some point in
> time, but i didn't check when. It could be you are actually
> advertising 10, even if you don't support it.

Hi Andrew,

here are the results. I ran them with the device connected to the switch
which doesn't allow successful link negotiation when this patch is present.

1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")

# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: no link
  registers for MII PHY 0: 
    1140 7949 0022 1622 0d81 c1e1 000f 0000
    0000 0300 0000 0000 0000 0000 0000 3000
    0000 0000 0000 0078 7002 0000 0000 0200
    0000 0000 0000 0528 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: no link
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD flow-control
  link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

2. net-next with this patch reverted

# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
  registers for MII PHY 0: 
    1140 796d 0022 1622 0181 c1e1 000f 0000
    0000 0300 3800 0000 0000 0000 0000 3000
    0000 0000 0000 0c7e 54fe 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

3. net-next with the following modification:

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320fb0..f531b615d80b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1779,7 +1779,6 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
 	WARN_ON(link_mode > 31);
 
 	phydev->supported &= ~BIT(link_mode);
-	phydev->advertising = phydev->supported;
 }
 EXPORT_SYMBOL(phy_remove_link_mode);


# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
  registers for MII PHY 0: 
    1140 796d 0022 1622 0181 c1e1 000f 0000
    0000 0300 3800 0000 0000 0000 0000 3000
    0000 0000 0000 087e 44fe 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-18 10:58       ` Simon Horman
@ 2018-09-18 13:02         ` Andrew Lunn
  2018-09-19  7:45           ` Simon Horman
  2018-09-20 13:25         ` Andrew Lunn
  1 sibling, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-18 13:02 UTC (permalink / raw)
  To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli

> Hi Andrew,

Hi Simon

Thanks for the dumps
 
> 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
> 
>   basic status: no link
>   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD 100baseTx-HD flow-control
>   link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> 
> 2. net-next with this patch reverted
> 
>   basic status: autonegotiation complete, link ok
>   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD 100baseTx-HD
>   link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

So flow-control is not present here.

>   basic status: autonegotiation complete, link ok
>   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD 100baseTx-HD
>   link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

And here also.

Looking at the code, i see:

/* E-MAC init function */
static void ravb_emac_init(struct net_device *ndev)
{
        struct ravb_private *priv = netdev_priv(ndev);

        /* Receive frame limit set register */
        ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);

        /* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
        ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
                   (ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
                   ECMR_TE | ECMR_RE, ECMR);

Does this mean Pause is not supported in the hardware?

     Thanks
	Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-18 13:02         ` Andrew Lunn
@ 2018-09-19  7:45           ` Simon Horman
  2018-09-19 12:32             ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-19  7:45 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

[CC Sergei, linux-renesas-soc]

On Tue, Sep 18, 2018 at 03:02:36PM +0200, Andrew Lunn wrote:
> > Hi Andrew,
> 
> Hi Simon
> 
> Thanks for the dumps
>  
> > 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
> > 
> >   basic status: no link
> >   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> >   advertising:  100baseTx-FD 100baseTx-HD flow-control
> >   link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> > 
> > 2. net-next with this patch reverted
> > 
> >   basic status: autonegotiation complete, link ok
> >   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> >   advertising:  100baseTx-FD 100baseTx-HD
> >   link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> 
> So flow-control is not present here.
> 
> >   basic status: autonegotiation complete, link ok
> >   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> >   advertising:  100baseTx-FD 100baseTx-HD
> >   link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> 
> And here also.

Thanks for raising this, I noticed it too.

> Looking at the code, i see:
> 
> /* E-MAC init function */
> static void ravb_emac_init(struct net_device *ndev)
> {
>         struct ravb_private *priv = netdev_priv(ndev);
> 
>         /* Receive frame limit set register */
>         ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
> 
>         /* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
>         ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
>                    (ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
>                    ECMR_TE | ECMR_RE, ECMR);
> 
> Does this mean Pause is not supported in the hardware?

According to my reading of the documentation Pause is supported by the
hardware and the above code seems to conflict with the comment (possibly
both the code and comment predate the current documentation). My reading of
the documentation is that the above unconditionally _enables_ receiving and
sending Pause frames with time parameter value 0.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-19  7:45           ` Simon Horman
@ 2018-09-19 12:32             ` Andrew Lunn
  2018-09-20  8:05               ` Simon Horman
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-19 12:32 UTC (permalink / raw)
  To: Simon Horman
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

> > And here also.
> 
> Thanks for raising this, I noticed it too.
> 
> > Looking at the code, i see:
> > 
> > /* E-MAC init function */
> > static void ravb_emac_init(struct net_device *ndev)
> > {
> >         struct ravb_private *priv = netdev_priv(ndev);
> > 
> >         /* Receive frame limit set register */
> >         ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
> > 
> >         /* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
> >         ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
> >                    (ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
> >                    ECMR_TE | ECMR_RE, ECMR);
> > 
> > Does this mean Pause is not supported in the hardware?
> 
> According to my reading of the documentation Pause is supported by the
> hardware and the above code seems to conflict with the comment (possibly
> both the code and comment predate the current documentation). My reading of
> the documentation is that the above unconditionally _enables_ receiving and
> sending Pause frames with time parameter value 0.

Hi Simon

We should first prove that this additional Pause is causing the
issue. After that, we can decide if we want to add Pause support to
the driver. Please could you test this patch.

Thanks
	Andrew

>From 0f69f4991454d48f34b05d5dc006c04a180c7842 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 18 Sep 2018 18:12:54 -0500
Subject: [PATCH] ravb: Disable Pause Advertisement

The previous commit to ravb had the side effect of making the PHY
advertise Pause. This previously did not happen, and it appears the
MAC does not support Pause. By default, phydev->supported has Pause
enabled, but phydev->advertising does not. Rather than rely on this,
be explicit, and remove the Pause link mode.

Reported-by: Simon Horman <horms@verge.net.au>
Fixes: 41124fa64d4b ("net: ethernet: Add helper to remove a supported link mode")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fb2a1125780d..d7630c0fdb0a 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1073,9 +1073,10 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE is not supported */
+	/* 10BASE and Pause is not supported */
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
 
 	phy_attached_info(phydev);
 
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-19 12:32             ` Andrew Lunn
@ 2018-09-20  8:05               ` Simon Horman
  2018-09-20 12:51                 ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-20  8:05 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

On Wed, Sep 19, 2018 at 02:32:00PM +0200, Andrew Lunn wrote:
> > > And here also.
> > 
> > Thanks for raising this, I noticed it too.
> > 
> > > Looking at the code, i see:
> > > 
> > > /* E-MAC init function */
> > > static void ravb_emac_init(struct net_device *ndev)
> > > {
> > >         struct ravb_private *priv = netdev_priv(ndev);
> > > 
> > >         /* Receive frame limit set register */
> > >         ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
> > > 
> > >         /* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
> > >         ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
> > >                    (ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
> > >                    ECMR_TE | ECMR_RE, ECMR);
> > > 
> > > Does this mean Pause is not supported in the hardware?
> > 
> > According to my reading of the documentation Pause is supported by the
> > hardware and the above code seems to conflict with the comment (possibly
> > both the code and comment predate the current documentation). My reading of
> > the documentation is that the above unconditionally _enables_ receiving and
> > sending Pause frames with time parameter value 0.
> 
> Hi Simon
> 
> We should first prove that this additional Pause is causing the
> issue. After that, we can decide if we want to add Pause support to
> the driver. Please could you test this patch.

Hi Andrew,

thanks for your patch. I agree with the approach you have taken here,
however, unfortunately this patch does not seem to resolve the problem that
I have observed.

With this patch on top of net-next ([1] & [2]) I see:

[1] net-next as of two days ago, the version used when reporting
    results earlier in this thread
    cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")

# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: no link
  registers for MII PHY 0: 
    1140 7949 0022 1622 0981 c1e1 000d 0000
    0000 0300 0000 0000 0000 0000 0000 3000
    0000 0000 0000 0000 7002 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: no link
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

[2] net-next as of this morning
    faa08325b429 ("isdn/hisax: Remove unnecessary parenthesis")

# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: no link
  registers for MII PHY 0: 
    1140 7949 0022 1622 0981 c1e1 000d 0000
    0000 0300 0000 0000 0000 0000 0000 3000
    0000 0000 0000 0000 6002 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: no link
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

I note a difference in the 3rd line of hex output: 7002 vs 6002
but I am unsure if that is relevant.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-20  8:05               ` Simon Horman
@ 2018-09-20 12:51                 ` Andrew Lunn
  2018-09-21  8:17                   ` Simon Horman
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-20 12:51 UTC (permalink / raw)
  To: Simon Horman
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

> eth0: no link
>   registers for MII PHY 0: 
>     1140 7949 0022 1622 0981 c1e1 000d 0000

Hi Simon

The ID registers 0022 1622 indicate this is a Micrel KSZ9031.
Are you using the micrel PHY driver?

> I note a difference in the 3rd line of hex output: 7002 vs 6002
> but I am unsure if that is relevant.

Register 20, or 0x14. The datasheet says "Reserved" and there is no
description given :-(

I will decode the other registers and see if i can find anything.

    Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-18 10:58       ` Simon Horman
  2018-09-18 13:02         ` Andrew Lunn
@ 2018-09-20 13:25         ` Andrew Lunn
  2018-09-21  8:13           ` Simon Horman
  1 sibling, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-20 13:25 UTC (permalink / raw)
  To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli

> 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
> 
> # mii-tool -vv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: no link
>   registers for MII PHY 0: 
>     1140 7949 0022 1622 0d81 c1e1 000f 0000
>     0000 0300 0000 0000 0000 0000 0000 3000
>     0000 0000 0000 0078 7002 0000 0000 0200
>     0000 0000 0000 0528 0000 0000 0000 0000
>   product info: vendor 00:08:85, model 34 rev 2
>   basic mode:   autonegotiation enabled
>   basic status: no link
>   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD 100baseTx-HD flow-control
>   link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> 
> 2. net-next with this patch reverted
> 
> # mii-tool -vv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 100baseTx-FD, link ok
>   registers for MII PHY 0: 
>     1140 796d 0022 1622 0181 c1e1 000f 0000

Hi Simon

Word 5 is what we are advertising. Bits 10 and 11 are Pause and Asym
Pause. In the good case here, neither are set. In this bad case above,
both bits are set.

The patch i asked you to try only cleared the Pause bit, not the
Asymmetric Pause bit. mii-tool only saying 'flow-control' did not
help.

Word 6 is what the partner is advertising. c1e1 indicates the partner
does not support flow control, both bits are 0. I don't see why this
is preventing auto-net though. But in the bad case, the status
register indicates auto-neg has not completed.

Anyway, please can you try this patch, which also removes Aysm Pause.

Thanks
	Andrew

>From 00a061304af51831ca1dc86bf6ce23d01f724229 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 18 Sep 2018 18:12:54 -0500
Subject: [PATCH] ravb: Disable Pause Advertisement

The previous commit to ravb had the side effect of making the PHY
advertise Pause. This previously did not happen, and it appears the
MAC does not support Pause. By default, phydev->supported has Pause
enabled, but phydev->advertising does not. Rather than rely on this,
be explicit, and remove the Pause link mode.

Reported-by: Simon Horman <horms@verge.net.au>
Fixes: 41124fa64d4b ("net: ethernet: Add helper to remove a supported link mode")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fb2a1125780d..b0f2612ad226 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1073,9 +1073,11 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE is not supported */
+	/* 10BASE, Pause and Asym Pause is not supported */
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
 
 	phy_attached_info(phydev);
 
-- 
2.19.0.rc1

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-20 13:25         ` Andrew Lunn
@ 2018-09-21  8:13           ` Simon Horman
  2018-09-21 13:01             ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-21  8:13 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

On Thu, Sep 20, 2018 at 03:25:30PM +0200, Andrew Lunn wrote:
> > 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
> > 
> > # mii-tool -vv eth0
> > Using SIOCGMIIPHY=0x8947
> > eth0: no link
> >   registers for MII PHY 0: 
> >     1140 7949 0022 1622 0d81 c1e1 000f 0000
> >     0000 0300 0000 0000 0000 0000 0000 3000
> >     0000 0000 0000 0078 7002 0000 0000 0200
> >     0000 0000 0000 0528 0000 0000 0000 0000
> >   product info: vendor 00:08:85, model 34 rev 2
> >   basic mode:   autonegotiation enabled
> >   basic status: no link
> >   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> >   advertising:  100baseTx-FD 100baseTx-HD flow-control
> >   link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> > 
> > 2. net-next with this patch reverted
> > 
> > # mii-tool -vv eth0
> > Using SIOCGMIIPHY=0x8947
> > eth0: negotiated 100baseTx-FD, link ok
> >   registers for MII PHY 0: 
> >     1140 796d 0022 1622 0181 c1e1 000f 0000
> 
> Hi Simon
> 
> Word 5 is what we are advertising. Bits 10 and 11 are Pause and Asym
> Pause. In the good case here, neither are set. In this bad case above,
> both bits are set.
> 
> The patch i asked you to try only cleared the Pause bit, not the
> Asymmetric Pause bit. mii-tool only saying 'flow-control' did not
> help.
> 
> Word 6 is what the partner is advertising. c1e1 indicates the partner
> does not support flow control, both bits are 0. I don't see why this
> is preventing auto-net though. But in the bad case, the status
> register indicates auto-neg has not completed.
> 
> Anyway, please can you try this patch, which also removes Aysm Pause.

Thanks Andrew,

it seems that removing Aysm Pause does the trick.


* net-next [5678cb3c96ee ("net-next: mscc: remove unused ocelot_dev_gmii.h")]
+ Your patch to disable Pause and Asym_Pause (and 10baseT)
  => Success!

# ip link set up dev eth0; sleep 10; mii-tool -vv eth0
[   13.418522] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
[   16.399410] ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
  registers for MII PHY 0: 
    1140 796d 0022 1622 0181 c1e1 000f 0000
    0000 0300 3800 0000 0000 0000 0000 3000
    0000 0000 0000 0c7e 54fe 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

* net-next [5678cb3c96ee ("net-next: mscc: remove unused ocelot_dev_gmii.h")]
+ Your patch modified to only disable Asym_Pause (and 10baseT)
  => Success!

# ip link set up dev eth0; sleep 10; mii-tool -vv eth0
[   86.414460] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
[   89.414651] ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
  registers for MII PHY 0: 
    1140 796d 0022 1622 0581 c1e1 000f 0000
    0000 0300 3800 0000 0000 0000 0000 3000
    0000 0000 0000 087e 44fe 0000 0000 0200
    0000 0000 0000 0500 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD flow-control
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

* net-next [5678cb3c96ee ("net-next: mscc: remove unused ocelot_dev_gmii.h")]
  + Your patch modified to only disable Pause (and 10baseT)
  => Fail (same test and result as yesterday)

# ip link set up dev eth0; sleep 10; mii-tool -vv eth0
[   52.518742] Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
Using SIOCGMIIPHY=0x8947
eth0: no link
  registers for MII PHY 0: 
    1140 7949 0022 1622 0981 c1e1 000f 0000
    0000 0300 0000 0000 0000 0000 0000 3000
    0000 0000 0000 0878 7002 0000 0000 0200
    0000 0000 0000 0528 0000 0000 0000 0000
  product info: vendor 00:08:85, model 34 rev 2
  basic mode:   autonegotiation enabled
  basic status: no link
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD
  link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD

> 
> Thanks
> 	Andrew
> 
> >From 00a061304af51831ca1dc86bf6ce23d01f724229 Mon Sep 17 00:00:00 2001
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Tue, 18 Sep 2018 18:12:54 -0500
> Subject: [PATCH] ravb: Disable Pause Advertisement
> 
> The previous commit to ravb had the side effect of making the PHY
> advertise Pause. This previously did not happen, and it appears the
> MAC does not support Pause. By default, phydev->supported has Pause
> enabled, but phydev->advertising does not. Rather than rely on this,
> be explicit, and remove the Pause link mode.
> 
> Reported-by: Simon Horman <horms@verge.net.au>
> Fixes: 41124fa64d4b ("net: ethernet: Add helper to remove a supported link mode")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index fb2a1125780d..b0f2612ad226 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1073,9 +1073,11 @@ static int ravb_phy_init(struct net_device *ndev)
>  		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
>  	}
>  
> -	/* 10BASE is not supported */
> +	/* 10BASE, Pause and Asym Pause is not supported */
>  	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
>  	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
>  
>  	phy_attached_info(phydev);
>  
> -- 
> 2.19.0.rc1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-20 12:51                 ` Andrew Lunn
@ 2018-09-21  8:17                   ` Simon Horman
  2018-09-24 15:36                     ` Simon Horman
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-21  8:17 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

On Thu, Sep 20, 2018 at 02:51:06PM +0200, Andrew Lunn wrote:
> > eth0: no link
> >   registers for MII PHY 0: 
> >     1140 7949 0022 1622 0981 c1e1 000d 0000
> 
> Hi Simon
> 
> The ID registers 0022 1622 indicate this is a Micrel KSZ9031.
> Are you using the micrel PHY driver?


Yes, when the Link is successfully negotiated I see:

Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)

> > I note a difference in the 3rd line of hex output: 7002 vs 6002
> > but I am unsure if that is relevant.
> 
> Register 20, or 0x14. The datasheet says "Reserved" and there is no
> description given :-(
> 
> I will decode the other registers and see if i can find anything.

Thanks, very much appreciated.

I believe your patch to disable Asym_Pause solves the immediate
problem I have observed.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-21  8:13           ` Simon Horman
@ 2018-09-21 13:01             ` Andrew Lunn
  2018-09-25  8:40               ` Simon Horman
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Lunn @ 2018-09-21 13:01 UTC (permalink / raw)
  To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli

> Thanks Andrew,
> 
> it seems that removing Aysm Pause does the trick.

Great.

I will submit the patch today.

I see two possible followups:

1) Figure out why auto-neg does not complete when Pause is
advertised. Is this a problem with the local PHY or the link partner?
The Micrel we have some control over, but the link partner in the
switch we have to treat as a black box.

2) If we can get negotiation to work correctly, then implement Pause
in the MAC driver. When phylib calls the adjust_link callback
phydev->pause and phydev->asym_pause tells you want the partner can
do. You can then decide how to program the MAC. There is also a
get/set for ethtool.

It really requires somebody with the hardware to do this.

   Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-21  8:17                   ` Simon Horman
@ 2018-09-24 15:36                     ` Simon Horman
  2018-09-24 15:50                       ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Horman @ 2018-09-24 15:36 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

On Fri, Sep 21, 2018 at 10:17:35AM +0200, Simon Horman wrote:
> On Thu, Sep 20, 2018 at 02:51:06PM +0200, Andrew Lunn wrote:
> > > eth0: no link
> > >   registers for MII PHY 0: 
> > >     1140 7949 0022 1622 0981 c1e1 000d 0000
> > 
> > Hi Simon
> > 
> > The ID registers 0022 1622 indicate this is a Micrel KSZ9031.
> > Are you using the micrel PHY driver?
> 
> 
> Yes, when the Link is successfully negotiated I see:
> 
> Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
> 
> > > I note a difference in the 3rd line of hex output: 7002 vs 6002
> > > but I am unsure if that is relevant.
> > 
> > Register 20, or 0x14. The datasheet says "Reserved" and there is no
> > description given :-(
> > 
> > I will decode the other registers and see if i can find anything.
> 
> Thanks, very much appreciated.
> 
> I believe your patch to disable Asym_Pause solves the immediate
> problem I have observed.

Andrew, how would you like to resolve this?
Let me know how I can help.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-24 15:36                     ` Simon Horman
@ 2018-09-24 15:50                       ` Andrew Lunn
  2018-09-25  7:38                         ` Simon Horman
  2018-09-27  3:08                         ` David Miller
  0 siblings, 2 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-09-24 15:50 UTC (permalink / raw)
  To: Simon Horman
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

On Mon, Sep 24, 2018 at 05:36:00PM +0200, Simon Horman wrote:
> On Fri, Sep 21, 2018 at 10:17:35AM +0200, Simon Horman wrote:
> > On Thu, Sep 20, 2018 at 02:51:06PM +0200, Andrew Lunn wrote:
> > > > eth0: no link
> > > >   registers for MII PHY 0: 
> > > >     1140 7949 0022 1622 0981 c1e1 000d 0000
> > > 
> > > Hi Simon
> > > 
> > > The ID registers 0022 1622 indicate this is a Micrel KSZ9031.
> > > Are you using the micrel PHY driver?
> > 
> > 
> > Yes, when the Link is successfully negotiated I see:
> > 
> > Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
> > 
> > > > I note a difference in the 3rd line of hex output: 7002 vs 6002
> > > > but I am unsure if that is relevant.
> > > 
> > > Register 20, or 0x14. The datasheet says "Reserved" and there is no
> > > description given :-(
> > > 
> > > I will decode the other registers and see if i can find anything.
> > 
> > Thanks, very much appreciated.
> > 
> > I believe your patch to disable Asym_Pause solves the immediate
> > problem I have observed.
> 
> Andrew, how would you like to resolve this?
> Let me know how I can help.

Hi Simon

I submitted it to netdev in the usual way. I hope DaveM will accept
and merge it.

    Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-24 15:50                       ` Andrew Lunn
@ 2018-09-25  7:38                         ` Simon Horman
  2018-09-27  3:08                         ` David Miller
  1 sibling, 0 replies; 34+ messages in thread
From: Simon Horman @ 2018-09-25  7:38 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, netdev, Florian Fainelli, Sergei Shtylyov,
	linux-renesas-soc

On Mon, Sep 24, 2018 at 05:50:23PM +0200, Andrew Lunn wrote:
> On Mon, Sep 24, 2018 at 05:36:00PM +0200, Simon Horman wrote:
> > On Fri, Sep 21, 2018 at 10:17:35AM +0200, Simon Horman wrote:
> > > On Thu, Sep 20, 2018 at 02:51:06PM +0200, Andrew Lunn wrote:
> > > > > eth0: no link
> > > > >   registers for MII PHY 0: 
> > > > >     1140 7949 0022 1622 0981 c1e1 000d 0000
> > > > 
> > > > Hi Simon
> > > > 
> > > > The ID registers 0022 1622 indicate this is a Micrel KSZ9031.
> > > > Are you using the micrel PHY driver?
> > > 
> > > 
> > > Yes, when the Link is successfully negotiated I see:
> > > 
> > > Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=204)
> > > 
> > > > > I note a difference in the 3rd line of hex output: 7002 vs 6002
> > > > > but I am unsure if that is relevant.
> > > > 
> > > > Register 20, or 0x14. The datasheet says "Reserved" and there is no
> > > > description given :-(
> > > > 
> > > > I will decode the other registers and see if i can find anything.
> > > 
> > > Thanks, very much appreciated.
> > > 
> > > I believe your patch to disable Asym_Pause solves the immediate
> > > problem I have observed.
> > 
> > Andrew, how would you like to resolve this?
> > Let me know how I can help.
> 
> Hi Simon
> 
> I submitted it to netdev in the usual way. I hope DaveM will accept
> and merge it.

Thanks, I see that now. Sorry for not noticing it before pinging you.
And thanks a lot for your patience in resolving this problem.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-21 13:01             ` Andrew Lunn
@ 2018-09-25  8:40               ` Simon Horman
  0 siblings, 0 replies; 34+ messages in thread
From: Simon Horman @ 2018-09-25  8:40 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

On Fri, Sep 21, 2018 at 03:01:44PM +0200, Andrew Lunn wrote:
> > Thanks Andrew,
> > 
> > it seems that removing Aysm Pause does the trick.
> 
> Great.
> 
> I will submit the patch today.
> 
> I see two possible followups:
> 
> 1) Figure out why auto-neg does not complete when Pause is
> advertised. Is this a problem with the local PHY or the link partner?
> The Micrel we have some control over, but the link partner in the
> switch we have to treat as a black box.

I may have limited scope to configure the link partner,
but it will be limited at best.

> 2) If we can get negotiation to work correctly, then implement Pause
> in the MAC driver. When phylib calls the adjust_link callback
> phydev->pause and phydev->asym_pause tells you want the partner can
> do. You can then decide how to program the MAC. There is also a
> get/set for ethtool.
> 
> It really requires somebody with the hardware to do this.

I have the hardware but I'm not sure how easily I can get
it to others.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-24 15:50                       ` Andrew Lunn
  2018-09-25  7:38                         ` Simon Horman
@ 2018-09-27  3:08                         ` David Miller
  2018-10-01 12:43                           ` Simon Horman
  1 sibling, 1 reply; 34+ messages in thread
From: David Miller @ 2018-09-27  3:08 UTC (permalink / raw)
  To: andrew; +Cc: horms, netdev, f.fainelli, sergei.shtylyov, linux-renesas-soc

From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 24 Sep 2018 17:50:23 +0200

> I submitted it to netdev in the usual way. I hope DaveM will accept
> and merge it.

Andrew did I miss your patch somehow?

If so, sorry, please resend.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
  2018-09-27  3:08                         ` David Miller
@ 2018-10-01 12:43                           ` Simon Horman
  0 siblings, 0 replies; 34+ messages in thread
From: Simon Horman @ 2018-10-01 12:43 UTC (permalink / raw)
  To: David Miller
  Cc: andrew, netdev, f.fainelli, sergei.shtylyov, linux-renesas-soc

On Wed, Sep 26, 2018 at 08:08:46PM -0700, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Mon, 24 Sep 2018 17:50:23 +0200
> 
> > I submitted it to netdev in the usual way. I hope DaveM will accept
> > and merge it.
> 
> Andrew did I miss your patch somehow?
> 
> If so, sorry, please resend.

Hi Dave,

I believe you have the patch in net-next as:

65c5877f6462 ("ravb: Disable Pause Advertisement")

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
  2018-09-11 23:53 ` [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed Andrew Lunn
@ 2018-11-22 10:40   ` Anssi Hannula
  2018-11-22 18:33     ` Andrew Lunn
  0 siblings, 1 reply; 34+ messages in thread
From: Anssi Hannula @ 2018-11-22 10:40 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli

Hi,

On 12.9.2018 2:53, Andrew Lunn wrote:
> Many Ethernet MAC drivers want to limit the PHY to only advertise a
> maximum speed of 100Mbs or 1Gbps. Rather than using a mask, make use
> of the helper function phy_set_max_speed().

But what if the PHY does not support 1Gbps in the first place?

This now adds 1Gbps to ->supported and ->advertising even on PHYs that
do not support 1Gbps or have the max speed limited to e.g. 100M via the
"max-speed" device tree property, breaking things.

Unless I'm missing something on how this is supposed to work now :)

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/8390/ax88796.c               |  4 +---
>  drivers/net/ethernet/aeroflex/greth.c             |  4 ++--
>  drivers/net/ethernet/agere/et131x.c               | 12 ++----------
>  drivers/net/ethernet/allwinner/sun4i-emac.c       |  3 +--
>  drivers/net/ethernet/altera/altera_tse_main.c     |  5 +----
>  drivers/net/ethernet/amd/au1000_eth.c             | 12 +-----------
>  drivers/net/ethernet/broadcom/bcm63xx_enet.c      | 10 ++--------
>  drivers/net/ethernet/broadcom/genet/bcmmii.c      |  2 +-
>  drivers/net/ethernet/broadcom/sb1250-mac.c        | 11 ++---------
>  drivers/net/ethernet/broadcom/tg3.c               |  8 ++++----
>  drivers/net/ethernet/cadence/macb_main.c          |  4 ++--
>  drivers/net/ethernet/cortina/gemini.c             |  2 +-
>  drivers/net/ethernet/dnet.c                       |  4 ++--
>  drivers/net/ethernet/ethoc.c                      |  5 +----
>  drivers/net/ethernet/freescale/fec_main.c         |  4 ++--
>  drivers/net/ethernet/freescale/ucc_geth.c         |  7 +------
>  drivers/net/ethernet/lantiq_etop.c                | 11 ++---------
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c       |  4 ++--
>  drivers/net/ethernet/nxp/lpc_eth.c                |  3 +--
>  drivers/net/ethernet/rdc/r6040.c                  | 12 ++----------
>  drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c   |  4 ++--
>  drivers/net/ethernet/smsc/smsc911x.c              |  5 +++--
>  drivers/net/ethernet/smsc/smsc9420.c              |  5 +++--
>  drivers/net/ethernet/socionext/sni_ave.c          |  6 ++----
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  3 +--
>  drivers/net/ethernet/toshiba/tc35815.c            |  2 +-
>  drivers/net/ethernet/xilinx/xilinx_emaclite.c     |  3 +--
>  drivers/staging/mt7621-eth/mdio.c                 |  2 +-
>  28 files changed, 47 insertions(+), 110 deletions(-)
[...]
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 16e4ef7d7185..bd4095c3a031 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -544,9 +544,9 @@ static int macb_mii_probe(struct net_device *dev)
>  
>  	/* mask with MAC supported features */
>  	if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
> -		phydev->supported &= PHY_GBIT_FEATURES;
> +		phy_set_max_speed(phydev, SPEED_1000);
>  	else
> -		phydev->supported &= PHY_BASIC_FEATURES;
> +		phy_set_max_speed(phydev, SPEED_100);
>  
>  	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>  		phydev->supported &= ~SUPPORTED_1000baseT_Half;

[...]

-- 
Anssi Hannula / Bitwise Oy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
  2018-11-22 10:40   ` Anssi Hannula
@ 2018-11-22 18:33     ` Andrew Lunn
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2018-11-22 18:33 UTC (permalink / raw)
  To: Anssi Hannula; +Cc: David Miller, netdev, Florian Fainelli

On Thu, Nov 22, 2018 at 12:40:25PM +0200, Anssi Hannula wrote:
> Hi,
> 
> On 12.9.2018 2:53, Andrew Lunn wrote:
> > Many Ethernet MAC drivers want to limit the PHY to only advertise a
> > maximum speed of 100Mbs or 1Gbps. Rather than using a mask, make use
> > of the helper function phy_set_max_speed().
> 
> But what if the PHY does not support 1Gbps in the first place?

Yes, you are correct. __set_phy_supported() needs modifying to take
into account what the PHY can do.

Thanks for pointing this out. I will take a look.

       Andrew

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2018-11-23  5:14 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-11 23:53 [PATCH v3 net-next 00/12] Preparing for phylib limkmodes Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 02/12] net: phy: et1011c: Remove incorrect missing 1000 Half Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 03/12] net: phy: bcm63xx: Allow to be built with COMPILE_TEST Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed Andrew Lunn
2018-11-22 10:40   ` Anssi Hannula
2018-11-22 18:33     ` Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 05/12] net: bcmgenet: Fix speed selection for reverse MII Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 06/12] net: ethernet: Fix up drivers masking pause support Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode Andrew Lunn
2018-09-17 15:13   ` Simon Horman
2018-09-17 15:38     ` Andrew Lunn
2018-09-18 10:58       ` Simon Horman
2018-09-18 13:02         ` Andrew Lunn
2018-09-19  7:45           ` Simon Horman
2018-09-19 12:32             ` Andrew Lunn
2018-09-20  8:05               ` Simon Horman
2018-09-20 12:51                 ` Andrew Lunn
2018-09-21  8:17                   ` Simon Horman
2018-09-24 15:36                     ` Simon Horman
2018-09-24 15:50                       ` Andrew Lunn
2018-09-25  7:38                         ` Simon Horman
2018-09-27  3:08                         ` David Miller
2018-10-01 12:43                           ` Simon Horman
2018-09-20 13:25         ` Andrew Lunn
2018-09-21  8:13           ` Simon Horman
2018-09-21 13:01             ` Andrew Lunn
2018-09-25  8:40               ` Simon Horman
2018-09-11 23:53 ` [PATCH v3 net-next 08/12] net: ethernet: Add helper for MACs which support asym pause Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 09/12] net: ethernet: Add helper for MACs which support pause Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 11/12] net: ethernet: Add helper for set_pauseparam for Pause Andrew Lunn
2018-09-11 23:53 ` [PATCH v3 net-next 12/12] net: ethernet: Add helper to determine if pause configuration is supported Andrew Lunn
2018-09-13  3:24 ` [PATCH v3 net-next 00/12] Preparing for phylib limkmodes David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.