Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 01/12] net: phy: ste10Xp: Remove wrong SUPPORTED_Pause
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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

* [PATCH net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 .../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      | 13 +----
 .../ethernet/freescale/dpaa/dpaa_ethtool.c    | 26 ++-------
 .../net/ethernet/freescale/gianfar_ethtool.c  | 53 +++++++------------
 .../hisilicon/hns3/hns3pf/hclge_main.c        |  8 +--
 drivers/net/ethernet/socionext/sni_ave.c      |  6 +--
 drivers/net/phy/phy_device.c                  | 23 ++++++++
 include/linux/phy.h                           |  1 +
 10 files changed, 69 insertions(+), 139 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 9aa7955d5d31..b406b6e34a8c 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) {
+				/* 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 (!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..032d59ffae8b 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1219,18 +1219,9 @@ 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);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 3184c8f7cdd0..80a9f2758030 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -210,28 +210,12 @@ 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);
+	if (phydev->autoneg) {
+		err = phy_start_aneg(phydev);
+		if (err < 0)
+			netdev_err(net_dev, "phy_start_aneg() = %d\n", err);
 	}
 
 	fman_get_pause_cfg(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..589a99718694 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,40 @@ 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 (phydev->autoneg)
+		/* inform link partner of our new flow ctrl settings */
+		return phy_start_aneg(phydev);
 
-			if (priv->rx_pause_en)
-				tempval |= MACCFG1_RX_FLOW;
-			gfar_write(&regs->maccfg1, tempval);
+	if (!epause->autoneg) {
+		u32 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 (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 8577dfc799ad..aa7babff2fc2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5208,13 +5208,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..fedda3fa585d 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -461,11 +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;
+	phy_set_asym_pause(phydev, pause->rx_pause, pause->tx_pause);
 
 	if (pause->autoneg) {
 		if (netif_running(ndev))
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5273091450a8..bcdcd4a42d82 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_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. Generally called from the
+ * set_pauseparam .ndo.
+ */
+void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
+{
+	phydev->supported &= ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
+
+	if (rx)
+		phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	if (tx)
+		phydev->supported ^= SUPPORTED_Asym_Pause;
+
+	phydev->advertising = phydev->supported;
+}
+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 083d51ff6b5a..f767a8f22856 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_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

* [PATCH net-next 09/12] net: ethernet: Add helper for MACs which support pause
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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..c55b49b82690 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_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..043b8c76821e 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_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..5273091450a8 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_pause - Enable support of pause
+ * @phydev: target phy_device struct
+ *
+ * Description: Called by the MAC to indicate is supports Pause, but not
+ * asym pause.
+ */
+void phy_support_pause(struct phy_device *phydev)
+{
+	phydev->supported |= SUPPORTED_Pause;
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_support_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..083d51ff6b5a 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_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

* [PATCH net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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 5333274a283c..881e566730f3 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -208,7 +208,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 c6707ea2d751..2ece43cd9c9b 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 ff1ffb46198a..3206728ed49f 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

* [PATCH net-next 08/12] net: ethernet: Add helper for MACs which support asym pause
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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..9aa7955d5d31 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 65a22cd9aef2..fc841e17fec0 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2493,8 +2493,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

* [PATCH net-next 05/12] net: ethernet: genet: Fix speed selection
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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
1G supported speeds to enable 1G MAC support.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 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 881e566730f3..69587a61e8d6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -220,11 +220,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

* [PATCH net-next 12/12] net: ethernet: Add helper to determine if pause configuration is supported
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 .../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 b406b6e34a8c..c6b5a9932d79 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 80a9f2758030..0f0ebdb791cf 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 589a99718694..f8e08ee0be19 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 3078792b900f..c1fdb01d56d2 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1854,6 +1854,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 205fb966d686..a8df64841e31 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1054,6 +1054,8 @@ void phy_support_pause(struct phy_device *phydev);
 void phy_support_asym_pause(struct phy_device *phydev);
 void phy_set_pause(struct phy_device *phydev, bool rx, 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

* [PATCH net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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 2ece43cd9c9b..4ececc20eeb2 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 3206728ed49f..f77e051b557d 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 a9991c5f4736..cc91207eef89 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2178,7 +2178,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

* [PATCH net-next 02/12] net: phy: et1011c: Remove incorrect missing 1000 Half
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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

* [PATCH net-next 11/12] net: ethernet: Add helper for set_pauseparam for Pause
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 drivers/net/ethernet/broadcom/bcm63xx_enet.c |  6 +-----
 drivers/net/ethernet/freescale/fec_main.c    |  8 +-------
 drivers/net/phy/phy_device.c                 | 21 ++++++++++++++++++++
 include/linux/phy.h                          |  1 +
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index c55b49b82690..e297972c63cf 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -892,11 +892,7 @@ static int bcm_enet_open(struct net_device *dev)
 		/* mask with MAC supported features */
 		phy_support_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_pause(phydev, 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 043b8c76821e..239eeac3d7ce 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2229,13 +2229,7 @@ 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_pause(ndev->phydev, pause->rx_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 bcdcd4a42d82..3078792b900f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1810,6 +1810,27 @@ void phy_support_asym_pause(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_support_asym_pause);
 
+/**
+ * phy_set_pause - Configure Pause
+ * @phydev: target phy_device struct
+ * @rx: Receiver 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_pause(struct phy_device *phydev, bool rx, bool autoneg)
+{
+	phydev->supported &= ~SUPPORTED_Pause;
+
+	if (rx || autoneg)
+		phydev->supported |= SUPPORTED_Pause;
+
+	phydev->advertising = phydev->supported;
+}
+EXPORT_SYMBOL(phy_set_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 f767a8f22856..205fb966d686 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_pause(struct phy_device *phydev);
 void phy_support_asym_pause(struct phy_device *phydev);
+void phy_set_pause(struct phy_device *phydev, bool rx, 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

* [PATCH net-next 06/12] net: ethernet: Fix up drivers masking pause support
From: Andrew Lunn @ 2018-09-02 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, maxime.chevallier, Andrew Lunn
In-Reply-To: <1535908001-18593-1-git-send-email-andrew@lunn.ch>

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>
---
 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

* Re: [PATCH net-next v2 0/3] net: mvneta: some small improvements
From: David Miller @ 2018-09-02 21:14 UTC (permalink / raw)
  To: Jisheng.Zhang
  Cc: thomas.petazzoni, andrew, gregory.clement, netdev,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20180831160810.2539ef4c@xhacker.debian>

From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Date: Fri, 31 Aug 2018 16:08:10 +0800

> patch1 removes the NETIF_F_GRO check ourself, because the net subsystem
> will handle it for us.
> 
> patch2 enables NETIF_F_RXCSUM by default, since the driver and HW
> supports the feature.
> 
> patch3 is a small optimization, to reduce smp_processor_id() calling
> in mvneta_tx_done_gbe.
> 
> since v1:
>  - based on net-next tree
>  - remove the fix patches, since they should be based on net branch.
>  - Add Gregory's Reviewed-by tag

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH] isdn: mISDN: tei: Fix a sleep-in-atomic-context bug in create_teimgr()
From: isdn @ 2018-09-02 16:31 UTC (permalink / raw)
  To: Jia-Ju Bai; +Cc: netdev, linux-kernel
In-Reply-To: <20180901120019.31664-1-baijiaju1990@gmail.com>

Hi,

I do not understand the analysis and do not see that the spinlock is a
problem here.
I think your DSAC analyzer assumes that the FUNC_PTR mgr_ctrl call calls
the  mgr_ctrl in tei.c, but in real it calls l2->ch.ctrl() which is the
function in layer2.c, not tei.c. And the function in layer2.c should not
do any GFP_KERNEL allocation.

Same for your 2. reported issue.

Am 01.09.2018 um 14:00 schrieb Jia-Ju Bai:
> The kernel module may sleep with holding a spinlock.
> 
> The function call paths (from bottom to top) in Linux-4.16 are:
> 
> [FUNC] kzalloc(GFP_KERNEL)
> drivers/isdn/mISDN/tei.c, 1058: kzalloc in create_teimgr
> drivers/isdn/mISDN/tei.c, 1278: create_teimgr in mgr_ctrl
> drivers/isdn/mISDN/tei.c, 1048: [FUNC_PTR]mgr_ctrl in create_teimgr
> drivers/isdn/mISDN/tei.c, 1045: _raw_read_lock_irqsave in create_teimgr
> 
> Note that [FUNC_PTR] means a function pointer call is used.
> 
> To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
> 
> This bug is found by my static analysis tool DSAC.
> 
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
>  drivers/isdn/mISDN/tei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
> index 12d9e5f4beb1..6d95ee639fdb 100644
> --- a/drivers/isdn/mISDN/tei.c
> +++ b/drivers/isdn/mISDN/tei.c
> @@ -1055,7 +1055,7 @@ create_teimgr(struct manager *mgr, struct channel_req *crq)
>  		       crq->adr.tei, crq->adr.sapi);
>  	if (!l2)
>  		return -ENOMEM;
> -	l2->tm = kzalloc(sizeof(struct teimgr), GFP_KERNEL);
> +	l2->tm = kzalloc(sizeof(struct teimgr), GFP_ATOMIC);
>  	if (!l2->tm) {
>  		kfree(l2);
>  		printk(KERN_ERR "kmalloc teimgr failed\n");
> 

^ permalink raw reply

* [PATCH net-next 2/2] net: dsa: mv88e6xxx: Add SERDES phydev_link_change for 6352
From: Andrew Lunn @ 2018-09-02 16:13 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Fainelli, Vivien Didelot, Chris Healy, netdev,
	Andrew Lunn
In-Reply-To: <1535904795-17405-1-git-send-email-andrew@lunn.ch>

The 6352 family has one SERDES interface, which can be used by either
port 4 or port 5. Add interrupt support for the SERDES interface, and
report when the link status changes.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c   |   6 ++
 drivers/net/dsa/mv88e6xxx/serdes.c | 105 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/serdes.h |  16 +++++
 3 files changed, 127 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8da3d39e3218..614dcc3e6a8b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3160,6 +3160,8 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_power = mv88e6352_serdes_power,
+	.serdes_irq_setup = mv88e6352_serdes_irq_setup,
+	.serdes_irq_free = mv88e6352_serdes_irq_free,
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.phylink_validate = mv88e6352_phylink_validate,
 };
@@ -3366,6 +3368,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_power = mv88e6352_serdes_power,
+	.serdes_irq_setup = mv88e6352_serdes_irq_setup,
+	.serdes_irq_free = mv88e6352_serdes_irq_free,
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.avb_ops = &mv88e6352_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
@@ -3664,6 +3668,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_power = mv88e6352_serdes_power,
+	.serdes_irq_setup = mv88e6352_serdes_irq_setup,
+	.serdes_irq_free = mv88e6352_serdes_irq_free,
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.avb_ops = &mv88e6352_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index e82983975754..bb69650ff772 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -185,6 +185,111 @@ int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
 	return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
 }
 
+static void mv88e6352_serdes_irq_link(struct mv88e6xxx_chip *chip, int port)
+{
+	struct dsa_switch *ds = chip->ds;
+	u16 status;
+	bool up;
+
+	mv88e6352_serdes_read(chip, MII_BMSR, &status);
+
+	/* Status must be read twice in order to give the current link
+	 * status. Otherwise the change in link status since the last
+	 * read of the register is returned.
+	 */
+	mv88e6352_serdes_read(chip, MII_BMSR, &status);
+
+	up = status & BMSR_LSTATUS;
+
+	dsa_port_phylink_mac_change(ds, port, up);
+}
+
+static irqreturn_t mv88e6352_serdes_thread_fn(int irq, void *dev_id)
+{
+	struct mv88e6xxx_port *port = dev_id;
+	struct mv88e6xxx_chip *chip = port->chip;
+	irqreturn_t ret = IRQ_NONE;
+	u16 status;
+	int err;
+
+	mutex_lock(&chip->reg_lock);
+
+	err = mv88e6352_serdes_read(chip, MV88E6352_SERDES_INT_STATUS, &status);
+	if (err)
+		goto out;
+
+	if (status & MV88E6352_SERDES_INT_LINK_CHANGE) {
+		ret = IRQ_HANDLED;
+		mv88e6352_serdes_irq_link(chip, port->port);
+	}
+out:
+	mutex_unlock(&chip->reg_lock);
+
+	return ret;
+}
+
+static int mv88e6352_serdes_irq_enable(struct mv88e6xxx_chip *chip)
+{
+	return mv88e6352_serdes_write(chip, MV88E6352_SERDES_INT_ENABLE,
+				      MV88E6352_SERDES_INT_LINK_CHANGE);
+}
+
+static int mv88e6352_serdes_irq_disable(struct mv88e6xxx_chip *chip)
+{
+	return mv88e6352_serdes_write(chip, MV88E6352_SERDES_INT_ENABLE, 0);
+}
+
+int mv88e6352_serdes_irq_setup(struct mv88e6xxx_chip *chip, int port)
+{
+	int err;
+
+	if (!mv88e6352_port_has_serdes(chip, port))
+		return 0;
+
+	chip->ports[port].serdes_irq = irq_find_mapping(chip->g2_irq.domain,
+							MV88E6352_SERDES_IRQ);
+	if (chip->ports[port].serdes_irq < 0) {
+		dev_err(chip->dev, "Unable to map SERDES irq: %d\n",
+			chip->ports[port].serdes_irq);
+		return chip->ports[port].serdes_irq;
+	}
+
+	/* Requesting the IRQ will trigger irq callbacks. So we cannot
+	 * hold the reg_lock.
+	 */
+	mutex_unlock(&chip->reg_lock);
+	err = request_threaded_irq(chip->ports[port].serdes_irq, NULL,
+				   mv88e6352_serdes_thread_fn,
+				   IRQF_ONESHOT, "mv88e6xxx-serdes",
+				   &chip->ports[port]);
+	mutex_lock(&chip->reg_lock);
+
+	if (err) {
+		dev_err(chip->dev, "Unable to request SERDES interrupt: %d\n",
+			err);
+		return err;
+	}
+
+	return mv88e6352_serdes_irq_enable(chip);
+}
+
+void mv88e6352_serdes_irq_free(struct mv88e6xxx_chip *chip, int port)
+{
+	if (!mv88e6352_port_has_serdes(chip, port))
+		return;
+
+	mv88e6352_serdes_irq_disable(chip);
+
+	/* Freeing the IRQ will trigger irq callbacks. So we cannot
+	 * hold the reg_lock.
+	 */
+	mutex_unlock(&chip->reg_lock);
+	free_irq(chip->ports[port].serdes_irq, &chip->ports[port]);
+	mutex_lock(&chip->reg_lock);
+
+	chip->ports[port].serdes_irq = 0;
+}
+
 /* Return the SERDES lane address a port is using. Only Ports 9 and 10
  * have SERDES lanes. Returns -ENODEV if a port does not have a lane.
  */
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index b1496de9c6fe..7870c5a9ef12 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -18,6 +18,19 @@
 
 #define MV88E6352_ADDR_SERDES		0x0f
 #define MV88E6352_SERDES_PAGE_FIBER	0x01
+#define MV88E6352_SERDES_IRQ		0x0b
+#define MV88E6352_SERDES_INT_ENABLE	0x12
+#define MV88E6352_SERDES_INT_SPEED_CHANGE	BIT(14)
+#define MV88E6352_SERDES_INT_DUPLEX_CHANGE	BIT(13)
+#define MV88E6352_SERDES_INT_PAGE_RX		BIT(12)
+#define MV88E6352_SERDES_INT_AN_COMPLETE	BIT(11)
+#define MV88E6352_SERDES_INT_LINK_CHANGE	BIT(10)
+#define MV88E6352_SERDES_INT_SYMBOL_ERROR	BIT(9)
+#define MV88E6352_SERDES_INT_FALSE_CARRIER	BIT(8)
+#define MV88E6352_SERDES_INT_FIFO_OVER_UNDER	BIT(7)
+#define MV88E6352_SERDES_INT_FIBRE_ENERGY	BIT(4)
+#define MV88E6352_SERDES_INT_STATUS	0x13
+
 
 #define MV88E6341_ADDR_SERDES		0x15
 
@@ -73,5 +86,8 @@ int mv88e6390_serdes_irq_enable(struct mv88e6xxx_chip *chip, int port,
 				int lane);
 int mv88e6390_serdes_irq_disable(struct mv88e6xxx_chip *chip, int port,
 				 int lane);
+int mv88e6352_serdes_irq_setup(struct mv88e6xxx_chip *chip, int port);
+void mv88e6352_serdes_irq_free(struct mv88e6xxx_chip *chip, int port);
+
 
 #endif
-- 
2.19.0.rc1

^ permalink raw reply related

* [PATCH net-next 1/2] net: dsa: mv88e6xxx: Fix writing to a PHY page.
From: Andrew Lunn @ 2018-09-02 16:13 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Fainelli, Vivien Didelot, Chris Healy, netdev,
	Andrew Lunn
In-Reply-To: <1535904795-17405-1-git-send-email-andrew@lunn.ch>

After changing to the needed page, actually write the value to the
register!

Fixes: 09cb7dfd3f14 ("net: dsa: mv88e6xxx: describe PHY page and SerDes")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---

It has been broken like this for over 2 years. So i don't think it has
been important until now. So i've decided not to submit it to net,
only net-next, were i do actually need this to work.
---
 drivers/net/dsa/mv88e6xxx/phy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/phy.c b/drivers/net/dsa/mv88e6xxx/phy.c
index 46af8052e535..152a65d46e0b 100644
--- a/drivers/net/dsa/mv88e6xxx/phy.c
+++ b/drivers/net/dsa/mv88e6xxx/phy.c
@@ -110,6 +110,9 @@ int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
 	err = mv88e6xxx_phy_page_get(chip, phy, page);
 	if (!err) {
 		err = mv88e6xxx_phy_write(chip, phy, MV88E6XXX_PHY_PAGE, page);
+		if (!err)
+			err = mv88e6xxx_phy_write(chip, phy, reg, val);
+
 		mv88e6xxx_phy_page_put(chip, phy);
 	}
 
-- 
2.19.0.rc1

^ permalink raw reply related

* [PATCH net-next 0/2] Full phylink support for mv88e6352
From: Andrew Lunn @ 2018-09-02 16:13 UTC (permalink / raw)
  To: David Miller
  Cc: Florian Fainelli, Vivien Didelot, Chris Healy, netdev,
	Andrew Lunn

These two patches implement full phylink support for the mv88e6352
family, when using an SFP connected to its SERDES interface. This adds
interrupt support to the SERDES, so that we get interrupts on link
up/down, and then make calls phydev_link_change().

The first patch is a minor bug fix, which does not seem to affect any
current features, so i'm not submitting it for stable. It is however
required for configuring SERDES interrupts.

Andrew Lunn (2):
  net: dsa: mv88e6xxx: Fix writing to a PHY page.
  net: dsa: mv88e6xxx: Add SERDES phydev_link_change for 6352

 drivers/net/dsa/mv88e6xxx/chip.c   |   6 ++
 drivers/net/dsa/mv88e6xxx/phy.c    |   3 +
 drivers/net/dsa/mv88e6xxx/serdes.c | 105 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/serdes.h |  16 +++++
 4 files changed, 130 insertions(+)

-- 
2.19.0.rc1

^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: mvneta: enable NETIF_F_RXCSUM by default
From: Andrew Lunn @ 2018-09-02 18:21 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: netdev, Gregory CLEMENT, linux-kernel, thomas.petazzoni,
	David S. Miller, linux-arm-kernel
In-Reply-To: <20180831161003.3f0dc351@xhacker.debian>

On Fri, Aug 31, 2018 at 04:10:03PM +0800, Jisheng Zhang wrote:
> The code and HW supports NETIF_F_RXCSUM, so let's enable it by default.
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Hi Jisheng

I ran some quick tests on a 370RD devel board, which uses a Marvell
switch connected to mvneta. I did no see any obvious regressions.

Tested-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* eBPF: verifier: back-edge
From: Sameeh Jubran @ 2018-09-02 13:52 UTC (permalink / raw)
  To: netdev

Hi all,

I am trying to insert instructions into the bpf using the bof syscall,
the instructions were generated using the following command line:

clang -I ~/Builds/bpf_rss/iproute2/include -Wall -target bpf -O2
-emit-llvm -c upstream/qemu/hw/net/rss_tap_bpf_program.c -o - | llc
-march=bpf -filetype=obj -o tap_bpf_program.o

and then were translated to bpf instructions using the BPFCparser tool

Every time I try to insert the array of instructions the verfier fails
with the following error:

back-edge from insn 363 to 364

I am not sure how to debug this error since the instructions are in
binary and the precompiled source code doesn't seem to contain any
weird loops or jump to instructions...

Is there a way to identify which line of source code is causing these errors?

Thanks!

^ permalink raw reply

* Re: [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Lukas Wunner @ 2018-09-02 13:21 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: Linus Walleij, Jonathan Corbet, Miguel Ojeda Sandonis,
	Peter Korsgaard, Peter Rosin, Ulf Hansson, Andrew Lunn,
	Florian Fainelli, David S. Miller, Dominik Brodowski,
	Greg Kroah-Hartman, Kishon Vijay Abraham I, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Jiri Slaby, Willy Tarreau
In-Reply-To: <20180902120144.6855-2-jmkrzyszt@gmail.com>

On Sun, Sep 02, 2018 at 02:01:41PM +0200, Janusz Krzysztofik wrote:
> @@ -461,7 +461,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
>  
>  		/* Clamp all values to [0,1] */
>  		for (i = 0; i < lh->numdescs; i++)
> -			vals[i] = !!ghd.values[i];
> +			__assign_bit(i, vals, !!ghd.values[i]);

The "!!" becomes unnecessary and can be removed, same for the code
comment above.


>  /**
>   * gpiod_get_array_value() - read values from an array of GPIOs
> - * @array_size: number of elements in the descriptor / value arrays
> + * @array_size: number of elements in the descriptor array / value bitmap
>   * @desc_array: array of GPIO descriptors whose values will be read
> - * @value_array: array to store the read values
> + * @value_bitnap: bitmap to store the read values

Typo, s/bitnap/bitmap/

Otherwise LGTM.

^ permalink raw reply

* [PATCH v7 2/4] gpiolib: Identify arrays matching GPIO hardware
From: Janusz Krzysztofik @ 2018-09-02 12:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
	Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
	David S. Miller, Dominik Brodowski, Greg Kroah-Hartman,
	Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180902120144.6855-1-jmkrzyszt@gmail.com>

Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order.  If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.

While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip.  Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion.  Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 Documentation/driver-api/gpio/consumer.rst |  4 +-
 drivers/gpio/gpiolib.c                     | 72 +++++++++++++++++++++++++++++-
 drivers/gpio/gpiolib.h                     |  9 ++++
 include/linux/gpio/consumer.h              |  9 ++++
 4 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be obtained with one call::
 					   enum gpiod_flags flags)
 
 This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors.  It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
 
 	struct gpio_descs {
+		struct gpio_array *info;
 		unsigned int ndescs;
 		struct gpio_desc *desc[];
 	}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 434d09779a1f..141f2f290538 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 {
 	struct gpio_desc *desc;
 	struct gpio_descs *descs;
-	int count;
+	struct gpio_array *array_info = NULL;
+	struct gpio_chip *chip;
+	int count, bitmap_size;
 
 	count = gpiod_count(dev, con_id);
 	if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			gpiod_put_array(descs);
 			return ERR_CAST(desc);
 		}
+
 		descs->desc[descs->ndescs] = desc;
+
+		chip = gpiod_to_chip(desc);
+		/*
+		 * Select a chip of first array member
+		 * whose index matches its pin hardware number
+		 * as a candidate for fast bitmap processing.
+		 */
+		if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+			struct gpio_descs *array;
+
+			bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+						    chip->ngpio : count);
+
+			array = kzalloc(struct_size(descs, desc, count) +
+					struct_size(array_info, invert_mask,
+					3 * bitmap_size), GFP_KERNEL);
+			if (!array) {
+				gpiod_put_array(descs);
+				return ERR_PTR(-ENOMEM);
+			}
+
+			memcpy(array, descs,
+			       struct_size(descs, desc, descs->ndescs + 1));
+			kfree(descs);
+
+			descs = array;
+			array_info = (void *)(descs->desc + count);
+			array_info->get_mask = array_info->invert_mask +
+						  bitmap_size;
+			array_info->set_mask = array_info->get_mask +
+						  bitmap_size;
+
+			array_info->desc = descs->desc;
+			array_info->size = count;
+			array_info->chip = chip;
+			bitmap_set(array_info->get_mask, descs->ndescs,
+				   count - descs->ndescs);
+			bitmap_set(array_info->set_mask, descs->ndescs,
+				   count - descs->ndescs);
+			descs->info = array_info;
+		}
+		/*
+		 * Unmark members which don't qualify for fast bitmap
+		 * processing (different chip, not in hardware order)
+		 */
+		if (array_info && (chip != array_info->chip ||
+		    gpio_chip_hwgpio(desc) != descs->ndescs)) {
+			__clear_bit(descs->ndescs, array_info->get_mask);
+			__clear_bit(descs->ndescs, array_info->set_mask);
+		} else if (array_info) {
+			/* Exclude open drain or open source from fast output */
+			if (gpiochip_line_is_open_drain(chip, descs->ndescs) ||
+			    gpiochip_line_is_open_source(chip, descs->ndescs))
+				__clear_bit(descs->ndescs,
+					    array_info->set_mask);
+			/* Identify 'fast' pins which require invertion */
+			if (gpiod_is_active_low(desc))
+				__set_bit(descs->ndescs,
+					  array_info->invert_mask);
+		}
+
 		descs->ndescs++;
 	}
+	if (array_info)
+		dev_dbg(dev,
+			"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
+			array_info->chip->label, array_info->size,
+			*array_info->get_mask, *array_info->set_mask,
+			*array_info->invert_mask);
 	return descs;
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 11e83d2eef89..b60905d558b1 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -183,6 +183,15 @@ static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev,
 }
 #endif
 
+struct gpio_array {
+	struct gpio_desc	**desc;
+	unsigned int		size;
+	struct gpio_chip	*chip;
+	unsigned long		*get_mask;
+	unsigned long		*set_mask;
+	unsigned long		invert_mask[];
+};
+
 struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 1b21dc7b0fad..8dede3e886af 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -17,11 +17,20 @@ struct device;
  */
 struct gpio_desc;
 
+/**
+ * Opaque descriptor for a structure of GPIO array attributes.  This structure
+ * is attached to struct gpiod_descs obtained from gpiod_get_array() and can be
+ * passed back to get/set array functions in order to activate fast processing
+ * path if applicable.
+ */
+struct gpio_array;
+
 /**
  * Struct containing an array of descriptors that can be obtained using
  * gpiod_get_array().
  */
 struct gpio_descs {
+	struct gpio_array *info;
 	unsigned int ndescs;
 	struct gpio_desc *desc[];
 };
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH net-next v2 1/2] netlink: ipv4 igmp join notifications
From: Patrick Ruddy @ 2018-09-02 11:18 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: netdev, Jiří Pírko, Stephen Hemminger
In-Reply-To: <CAJieiUizzPcnfxw-t4fciZ_rHYCFaFfBfGOY0YLyKGxm8jDAFQ@mail.gmail.com>

Hi Roopa

inline

thx

-pr

On Fri, 2018-08-31 at 09:29 -0700, Roopa Prabhu wrote:
> On Fri, Aug 31, 2018 at 4:20 AM, Patrick Ruddy
> <pruddy@vyatta.att-mail.com> wrote:
> > Some userspace applications need to know about IGMP joins from the kernel
> > for 2 reasons
> > 1. To allow the programming of multicast MAC filters in hardware
> > 2. To form a multicast FORUS list for non link-local multicast
> >    groups to be sent to the kernel and from there to the interested
> >    party.
> > (1) can be fulfilled but simply sending the hardware multicast MAC
> > address to be programmed but (2) requires the L3 address to be sent
> > since this cannot be constructed from the MAC address whereas the
> > reverse translation is a standard library function.
> > 
> > This commit provides addition and deletion of multicast addresses
> > using the RTM_NEWADDR and RTM_DELADDR messages. It also provides
> > the RTM_GETADDR extension to allow multicast join state to be read
> > from the kernel.
> > 
> > Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com>
> > ---
> > v2: fix kbuild warnings.
> 
> I am still going through the series, but AFAICT, user-space caches listening to
> RTNLGRP_IPV4_IFADDR will now also get multicast addresses by default ?
> 

Yes that's the crux of this change. It's unfortunate that I could not
use IFA_MULTICAST to distinguish the SAFI. I suppose the other option
would be to create a set of new NEW/DEL/GETMULTICAST messages but the
partial code for RTM_GETMULTICAST in ipv6/mcast.c complicates that
slightly. Happy to look at it if you think that would be be better.

> 
> > 
> >  include/linux/igmp.h |  4 ++
> >  net/ipv4/devinet.c   | 39 +++++++++++++------
> >  net/ipv4/igmp.c      | 90 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 122 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/linux/igmp.h b/include/linux/igmp.h
> > index 119f53941c12..644a548024ed 100644
> > --- a/include/linux/igmp.h
> > +++ b/include/linux/igmp.h
> > @@ -19,6 +19,8 @@
> >  #include <linux/timer.h>
> >  #include <linux/in.h>
> >  #include <linux/refcount.h>
> > +#include <linux/netlink.h>
> > +#include <linux/netdevice.h>
> >  #include <uapi/linux/igmp.h>
> > 
> >  static inline struct igmphdr *igmp_hdr(const struct sk_buff *skb)
> > @@ -130,6 +132,8 @@ extern void ip_mc_unmap(struct in_device *);
> >  extern void ip_mc_remap(struct in_device *);
> >  extern void ip_mc_dec_group(struct in_device *in_dev, __be32 addr);
> >  extern void ip_mc_inc_group(struct in_device *in_dev, __be32 addr);
> > +extern int ip_mc_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb,
> > +                            struct net_device *dev);
> >  int ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed);
> > 
> >  #endif
> > diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> > index ea4bd8a52422..42f7dcc4fb5e 100644
> > --- a/net/ipv4/devinet.c
> > +++ b/net/ipv4/devinet.c
> > @@ -57,6 +57,7 @@
> >  #endif
> >  #include <linux/kmod.h>
> >  #include <linux/netconf.h>
> > +#include <linux/igmp.h>
> > 
> >  #include <net/arp.h>
> >  #include <net/ip.h>
> > @@ -1651,6 +1652,7 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
> >         int h, s_h;
> >         int idx, s_idx;
> >         int ip_idx, s_ip_idx;
> > +       int multicast, mcast_idx;
> >         struct net_device *dev;
> >         struct in_device *in_dev;
> >         struct in_ifaddr *ifa;
> > @@ -1659,6 +1661,8 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
> >         s_h = cb->args[0];
> >         s_idx = idx = cb->args[1];
> >         s_ip_idx = ip_idx = cb->args[2];
> > +       multicast = cb->args[3];
> > +       mcast_idx = cb->args[4];
> > 
> >         for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
> >                 idx = 0;
> > @@ -1675,18 +1679,29 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
> >                         if (!in_dev)
> >                                 goto cont;
> > 
> > -                       for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
> > -                            ifa = ifa->ifa_next, ip_idx++) {
> > -                               if (ip_idx < s_ip_idx)
> > -                                       continue;
> > -                               if (inet_fill_ifaddr(skb, ifa,
> > -                                            NETLINK_CB(cb->skb).portid,
> > -                                            cb->nlh->nlmsg_seq,
> > -                                            RTM_NEWADDR, NLM_F_MULTI) < 0) {
> > -                                       rcu_read_unlock();
> > -                                       goto done;
> > +                       if (!multicast) {
> > +                               for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
> > +                                    ifa = ifa->ifa_next, ip_idx++) {
> > +                                       if (ip_idx < s_ip_idx)
> > +                                               continue;
> > +                                       if (inet_fill_ifaddr(skb, ifa,
> > +                                                            NETLINK_CB(cb->skb).portid,
> > +                                                            cb->nlh->nlmsg_seq,
> > +                                                            RTM_NEWADDR,
> > +                                                            NLM_F_MULTI) < 0) {
> > +                                               rcu_read_unlock();
> > +                                               goto done;
> > +                                       }
> > +                                       nl_dump_check_consistent(cb,
> > +                                                                nlmsg_hdr(skb));
> >                                 }
> > -                               nl_dump_check_consistent(cb, nlmsg_hdr(skb));
> > +                               /* set for multicast loop */
> > +                               multicast++;
> > +                       }
> > +                       /* loop over multicast addresses */
> > +                       if (ip_mc_dump_ifaddr(skb, cb, dev) < 0) {
> > +                               rcu_read_unlock();
> > +                               goto done;
> >                         }
> >  cont:
> >                         idx++;
> > @@ -1698,6 +1713,8 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
> >         cb->args[0] = h;
> >         cb->args[1] = idx;
> >         cb->args[2] = ip_idx;
> > +       cb->args[3] = multicast;
> > +       cb->args[4] = mcast_idx;
> > 
> >         return skb->len;
> >  }
> > diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> > index cf75f8944b05..c9bbd1d27124 100644
> > --- a/net/ipv4/igmp.c
> > +++ b/net/ipv4/igmp.c
> > @@ -86,6 +86,7 @@
> >  #include <linux/inetdevice.h>
> >  #include <linux/igmp.h>
> >  #include <linux/if_arp.h>
> > +#include <net/netlink.h>
> >  #include <linux/rtnetlink.h>
> >  #include <linux/times.h>
> >  #include <linux/pkt_sched.h>
> > @@ -1384,6 +1385,91 @@ static void ip_mc_hash_remove(struct in_device *in_dev,
> >  }
> > 
> > 
> > +static int fill_addr(struct sk_buff *skb, struct net_device *dev, __be32 addr,
> > +                    int type, unsigned int flags)
> > +{
> > +       struct nlmsghdr *nlh;
> > +       struct ifaddrmsg *ifm;
> > +
> > +       nlh = nlmsg_put(skb, 0, 0, type, sizeof(*ifm), flags);
> > +       if (!nlh)
> > +               return -EMSGSIZE;
> > +
> > +       ifm = nlmsg_data(nlh);
> > +       ifm->ifa_family = AF_INET;
> > +       ifm->ifa_prefixlen = 32;
> > +       ifm->ifa_flags = IFA_F_PERMANENT;
> > +       ifm->ifa_scope = RT_SCOPE_LINK;
> > +       ifm->ifa_index = dev->ifindex;
> > +
> > +       if (nla_put_in_addr(skb, IFA_ADDRESS, addr))
> > +               goto nla_put_failure;
> > +       nlmsg_end(skb, nlh);
> > +       return 0;
> > +
> > +nla_put_failure:
> > +       nlmsg_cancel(skb, nlh);
> > +       return -EMSGSIZE;
> > +}
> > +
> > +static inline size_t addr_nlmsg_size(void)
> > +{
> > +       return NLMSG_ALIGN(sizeof(struct ifaddrmsg))
> > +               + nla_total_size(sizeof(__be32));
> > +}
> > +
> > +static void ip_mc_addr_notify(struct net_device *dev, __be32 addr, int type)
> > +{
> > +       struct net *net = dev_net(dev);
> > +       struct sk_buff *skb;
> > +       int err = -ENOBUFS;
> > +
> > +       skb = nlmsg_new(addr_nlmsg_size(), GFP_ATOMIC);
> > +       if (!skb)
> > +               goto errout;
> > +
> > +       err = fill_addr(skb, dev, addr, type, 0);
> > +       if (err < 0) {
> > +               WARN_ON(err == -EMSGSIZE);
> > +               kfree_skb(skb);
> > +               goto errout;
> > +       }
> > +       rtnl_notify(skb, net, 0, RTNLGRP_IPV4_IFADDR, NULL, GFP_ATOMIC);
> > +       return;
> > +errout:
> > +       if (err < 0)
> > +               rtnl_set_sk_err(net, RTNLGRP_LINK, err);
> 
> 
> s/RTNLGRP_LINK/RTNLGRP_IPV4_IFADDR/
> 
> 
> 
> 
> > +}
> > +
> > +int ip_mc_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb,
> > +                     struct net_device *dev)
> > +{
> > +       int s_idx;
> > +       int idx = 0;
> > +       struct ip_mc_list *im;
> > +       struct in_device *in_dev;
> > +
> > +       ASSERT_RTNL();
> > +
> > +       s_idx = cb->args[4];
> > +       in_dev = __in_dev_get_rtnl(dev);
> > +
> > +       for_each_pmc_rtnl(in_dev, im) {
> > +               if (idx < s_idx)
> > +                       continue;
> > +               if (fill_addr(skb, dev, im->multiaddr, RTM_NEWADDR,
> > +                             NLM_F_MULTI) < 0)
> > +                       goto done;
> > +               nl_dump_check_consistent(cb, nlmsg_hdr(skb));
> > +               idx++;
> > +       }
> > +
> > + done:
> > +       cb->args[4] = idx;
> > +
> > +       return skb->len;
> > +}
> > +
> >  /*
> >   *     A socket has joined a multicast group on device dev.
> >   */
> > @@ -1433,6 +1519,8 @@ static void __ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
> >         igmpv3_del_delrec(in_dev, im);
> >  #endif
> >         igmp_group_added(im);
> > +
> > +       ip_mc_addr_notify(in_dev->dev, addr, RTM_NEWADDR);
> >         if (!in_dev->dead)
> >                 ip_rt_multicast_event(in_dev);
> >  out:
> > @@ -1664,6 +1752,8 @@ void ip_mc_dec_group(struct in_device *in_dev, __be32 addr)
> >                                 in_dev->mc_count--;
> >                                 igmp_group_dropped(i);
> >                                 ip_mc_clear_src(i);
> > +                               ip_mc_addr_notify(in_dev->dev, addr,
> > +                                                 RTM_DELADDR);
> > 
> >                                 if (!in_dev->dead)
> >                                         ip_rt_multicast_event(in_dev);
> > --
> > 2.17.1
> > 

^ permalink raw reply

* Re: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-09-02 10:19 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Linus Walleij, Jonathan Corbet, Peter Korsgaard, Peter Rosin,
	Ulf Hansson, Andrew Lunn, Florian Fainelli, David S. Miller,
	Dominik Brodowski, Greg Kroah-Hartman, Kishon Vijay Abraham I,
	Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Jiri Slaby, Willy Tarreau,
	Geert Uytterhoeven, Linux 
In-Reply-To: <CANiq72kJT15ELkYr2U5r4-pvGunQFLmZ5dhdZLnjmM7NWBnkNQ@mail.gmail.com>

Hi Miguel,

On Thursday, August 30, 2018 1:10:59 PM CEST Miguel Ojeda wrote:
> Hi Janusz,
> 
> On Wed, Aug 29, 2018 at 10:48 PM, Janusz Krzysztofik
> <jmkrzyszt@gmail.com> wrote:
> > ...
> >         /* High nibble + RS, RW */
> > -       for (i = 4; i < 8; i++)
> > -               values[PIN_DATA0 + i] = !!(val & BIT(i));
> > -       values[PIN_CTRL_RS] = rs;
> > +       value_bitmap[0] = val;
> > +       __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> >         n = 5;
> >         if (hd->pins[PIN_CTRL_RW]) {
> > -               values[PIN_CTRL_RW] = 0;
> > +               __clear_bit(PIN_CTRL_RW, value_bitmap);
> >                 n++;
> >         }
> > +       value_bitmap[0] >>= PIN_DATA4;
> >
> >         /* Present the data to the port */
> > -       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> > -                                      &values[PIN_DATA4]);
> > +       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], 
value_bitmap);
> >
> >         hd44780_strobe_gpio(hd);
> >
> >         /* Low nibble */
> > -       for (i = 0; i < 4; i++)
> > -               values[PIN_DATA4 + i] = !!(val & BIT(i));
> > +       value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
> > +       value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
> 
> This is still wrong! What I originally meant in my v4 review is that
> there is an extra ~ in the second line.

Indeed, that's wrong, I missed your original point, sorry.

> Also, a couple of general comments:
> 
>  - Please review the list of CCs (I was not CC'd originally, so maybe
> there are other maintainers that aren't, either)

That's probably because early versions of the series, prior to v4, were not 
touching existing GPIO API so there were no changes to users of gpiod_get/
set_array_value() and their variants. From v4 on, you are in the loop so don't 
worry, you haven't missed anything.
But anyway, thanks for your suggestion to review the Cc; list, I've done that 
for v7 and added still a few people who contributed most to the code being 
changed.

>  - In general, the new code seems harder to read than the original one
> (but that is my impression).

I hope we are slowly approaching acceptable readability in recent iterations.

Thanks,
Janusz

^ permalink raw reply

* Re: [PATCH net-next 0/5] rtnetlink: add IFA_IF_NETNSID for RTM_GETADDR
From: Jiri Benc @ 2018-09-02  9:58 UTC (permalink / raw)
  To: Christian Brauner
  Cc: David Miller, nicolas.dichtel, ktkhai, netdev, linux-kernel,
	kuznet, yoshfuji, pombredanne, kstewart, gregkh, dsahern, fw,
	lucien.xin, jakub.kicinski
In-Reply-To: <20180901184704.lzynbw5zkyiqc6wb@gmail.com>

On Sat, 1 Sep 2018 20:47:05 +0200, Christian Brauner wrote:
> Sorry for the late reply. Yup, sounds good to me.
> But maybe IFA_TARGET_NETNSID to indicate that we're talking network
> namespaces here? Seems to me that NSID might give the wrong impression.
> I'll send v1 soon. I expect tomorrow or sometime next week.

On the other hand, we currently have IFLA_IF_NETNSID for the link
operations. IFA_IF_NETNSID is more consistent with the existing
attribute. It may be confusing to authors of user space programs to
have attribute names doing the same thing constructed differently for
different calls (IFLA_IF_NETNSID and IFA_TARGET_NETNSID).

As for the patch set itself, it makes sense to me, whatever the
final attribute name is.

Thanks,

 Jiri

^ permalink raw reply

* Re: [PATCH RFC] net/mlx5_en: switch to Toeplitz RSS hash by default
From: Konstantin Khlebnikov @ 2018-09-02  9:55 UTC (permalink / raw)
  To: Tariq Toukan, netdev, Saeed Mahameed, Gal Pressman, Or Gerlitz,
	David S. Miller
In-Reply-To: <f9c1ee4c-41a3-c9e4-01f8-7337dfc90077@mellanox.com>

On 02.09.2018 12:29, Tariq Toukan wrote:
> 
> 
> On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote:
>> XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits.
>> It seems not enough for RFS. All other drivers use toeplitz.
>>
>> Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used
>> together with NETIF_F_RXHASH (enabled by default too): "Enabling both
>> XOR Hash function and RX Hashing can limit RPS functionality".
>>
>> XOR is default in mlx5_en since commit 2be6967cdbc9
>> ("net/mlx5e: Support ETH_RSS_HASH_XOR").
>>
>> Hash function could be set via ethtool. But it would be nice to have
>> single standard for drivers or proper description why this one is special.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> ---
> 
> Hi Konstantin,
> 
> Thanks for the patch.
> 
> I understand the motivation.
> 
> This change affects the default out-of-the-box behavior and requires a full performance cycle. We'll run performance regression tomorrow, 
> results should be ready by EOW.
>  > I'll update.

Ok, thank you.

The only mention I've found in your documentation
http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Linux_User_Manual_v4_4.pdf

is
---
1.1.10 RSS Support
1.1.10.1 RSS Hash Function
The device has the ability to use XOR as the RSS distribution function, instead of the default
Toplitz function.
The XOR function can be better distributed among driver's receive queues in small number of
streams, where it distributes each TCP/UDP stream to a different queue.
---

So Toeplitz is supposed to be default hash function for all versions of drivers and hardware.

Also XOR8 seems vulnerable for ddos - hash is predictable, no random\secret vector, only 8 bits.
So, it's easy to route all flows into one point. As we got it by accident.

Moreover, in kernel 4.4.y hash switch via ethtool is broken and does not work =)

> 
> Regards,
> Tariq

^ permalink raw reply

* Re: [PATCH RFC] net/mlx5_en: switch to Toeplitz RSS hash by default
From: Tariq Toukan @ 2018-09-02  9:29 UTC (permalink / raw)
  To: Konstantin Khlebnikov, netdev, Saeed Mahameed, Gal Pressman,
	Or Gerlitz, David S. Miller, Tariq Toukan
In-Reply-To: <153571495680.372632.9464993927924201878.stgit@buzz>



On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote:
> XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits.
> It seems not enough for RFS. All other drivers use toeplitz.
> 
> Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used
> together with NETIF_F_RXHASH (enabled by default too): "Enabling both
> XOR Hash function and RX Hashing can limit RPS functionality".
> 
> XOR is default in mlx5_en since commit 2be6967cdbc9
> ("net/mlx5e: Support ETH_RSS_HASH_XOR").
> 
> Hash function could be set via ethtool. But it would be nice to have
> single standard for drivers or proper description why this one is special.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---

Hi Konstantin,

Thanks for the patch.

I understand the motivation.

This change affects the default out-of-the-box behavior and requires a 
full performance cycle. We'll run performance regression tomorrow, 
results should be ready by EOW.

I'll update.

Regards,
Tariq

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox