[CFT 0/8] rework phylink interface for split MAC/PCS support

[CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

Hi,

The following series changes the phylink interface to allow us to
better support split MAC / MAC PCS setups.  The fundamental change
required for this turns out to be quite simple.

Today, mac_config() is used for everything to do with setting the
parameters for the MAC, and mac_link_up() is used to inform the
MAC driver that the link is now up (and so to allow packet flow.)
mac_config() also has had a few implementation issues, with folk
who believe that members such as "speed" and "duplex" are always
valid, where "link" gets used inappropriately, etc.

With the proposed patches, all this changes subtly - but in a
backwards compatible way at this stage.

We pass the the full resolved link state (speed, duplex, pause) to
mac_link_up(), and it is now guaranteed that these parameters to
this function will always be valid (no more SPEED_UNKNOWN or
DUPLEX_UNKNOWN here - unless phylink is fed with such things.)

Drivers should convert over to using the state in mac_link_up()
rather than configuring the speed, duplex and pause in the
mac_config() method. The patch series includes a number of MAC
drivers which I've thought have been easy targets - I've left the
remainder as I think they need maintainer input. However, *all*
drivers will need conversion for future phylink development.

 Documentation/networking/sfp-phylink.rst          |  17 +++-
 drivers/net/dsa/b53/b53_common.c                  |   4 +-
 drivers/net/dsa/b53/b53_priv.h                    |   4 +-
 drivers/net/dsa/bcm_sf2.c                         |   4 +-
 drivers/net/dsa/lantiq_gswip.c                    |   4 +-
 drivers/net/dsa/mt7530.c                          |   4 +-
 drivers/net/dsa/mv88e6xxx/chip.c                  |  79 +++++++++++++----
 drivers/net/dsa/sja1105/sja1105_main.c            |   4 +-
 drivers/net/ethernet/cadence/macb.h               |   1 -
 drivers/net/ethernet/cadence/macb_main.c          |  53 ++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  61 ++++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
 drivers/net/ethernet/marvell/mvneta.c             |  63 ++++++++-----
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c   | 102 +++++++++++++---------
 drivers/net/ethernet/mediatek/mtk_eth_soc.c       |   7 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   4 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |  37 ++++----
 drivers/net/phy/phylink.c                         |   9 +-
 include/linux/phylink.h                           |  57 ++++++++----
 include/net/dsa.h                                 |   4 +-
 net/dsa/port.c                                    |   7 +-
 21 files changed, 350 insertions(+), 176 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

Hi,

The following series changes the phylink interface to allow us to
better support split MAC / MAC PCS setups.  The fundamental change
required for this turns out to be quite simple.

Today, mac_config() is used for everything to do with setting the
parameters for the MAC, and mac_link_up() is used to inform the
MAC driver that the link is now up (and so to allow packet flow.)
mac_config() also has had a few implementation issues, with folk
who believe that members such as "speed" and "duplex" are always
valid, where "link" gets used inappropriately, etc.

With the proposed patches, all this changes subtly - but in a
backwards compatible way at this stage.

We pass the the full resolved link state (speed, duplex, pause) to
mac_link_up(), and it is now guaranteed that these parameters to
this function will always be valid (no more SPEED_UNKNOWN or
DUPLEX_UNKNOWN here - unless phylink is fed with such things.)

Drivers should convert over to using the state in mac_link_up()
rather than configuring the speed, duplex and pause in the
mac_config() method. The patch series includes a number of MAC
drivers which I've thought have been easy targets - I've left the
remainder as I think they need maintainer input. However, *all*
drivers will need conversion for future phylink development.

 Documentation/networking/sfp-phylink.rst          |  17 +++-
 drivers/net/dsa/b53/b53_common.c                  |   4 +-
 drivers/net/dsa/b53/b53_priv.h                    |   4 +-
 drivers/net/dsa/bcm_sf2.c                         |   4 +-
 drivers/net/dsa/lantiq_gswip.c                    |   4 +-
 drivers/net/dsa/mt7530.c                          |   4 +-
 drivers/net/dsa/mv88e6xxx/chip.c                  |  79 +++++++++++++----
 drivers/net/dsa/sja1105/sja1105_main.c            |   4 +-
 drivers/net/ethernet/cadence/macb.h               |   1 -
 drivers/net/ethernet/cadence/macb_main.c          |  53 ++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  61 ++++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
 drivers/net/ethernet/marvell/mvneta.c             |  63 ++++++++-----
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c   | 102 +++++++++++++---------
 drivers/net/ethernet/mediatek/mtk_eth_soc.c       |   7 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   4 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |  37 ++++----
 drivers/net/phy/phylink.c                         |   9 +-
 include/linux/phylink.h                           |  57 ++++++++----
 include/net/dsa.h                                 |   4 +-
 net/dsa/port.c                                    |   7 +-
 21 files changed, 350 insertions(+), 176 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

Hi,

The following series changes the phylink interface to allow us to
better support split MAC / MAC PCS setups.  The fundamental change
required for this turns out to be quite simple.

Today, mac_config() is used for everything to do with setting the
parameters for the MAC, and mac_link_up() is used to inform the
MAC driver that the link is now up (and so to allow packet flow.)
mac_config() also has had a few implementation issues, with folk
who believe that members such as "speed" and "duplex" are always
valid, where "link" gets used inappropriately, etc.

With the proposed patches, all this changes subtly - but in a
backwards compatible way at this stage.

We pass the the full resolved link state (speed, duplex, pause) to
mac_link_up(), and it is now guaranteed that these parameters to
this function will always be valid (no more SPEED_UNKNOWN or
DUPLEX_UNKNOWN here - unless phylink is fed with such things.)

Drivers should convert over to using the state in mac_link_up()
rather than configuring the speed, duplex and pause in the
mac_config() method. The patch series includes a number of MAC
drivers which I've thought have been easy targets - I've left the
remainder as I think they need maintainer input. However, *all*
drivers will need conversion for future phylink development.

 Documentation/networking/sfp-phylink.rst          |  17 +++-
 drivers/net/dsa/b53/b53_common.c                  |   4 +-
 drivers/net/dsa/b53/b53_priv.h                    |   4 +-
 drivers/net/dsa/bcm_sf2.c                         |   4 +-
 drivers/net/dsa/lantiq_gswip.c                    |   4 +-
 drivers/net/dsa/mt7530.c                          |   4 +-
 drivers/net/dsa/mv88e6xxx/chip.c                  |  79 +++++++++++++----
 drivers/net/dsa/sja1105/sja1105_main.c            |   4 +-
 drivers/net/ethernet/cadence/macb.h               |   1 -
 drivers/net/ethernet/cadence/macb_main.c          |  53 ++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  61 ++++++++-----
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
 drivers/net/ethernet/marvell/mvneta.c             |  63 ++++++++-----
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c   | 102 +++++++++++++---------
 drivers/net/ethernet/mediatek/mtk_eth_soc.c       |   7 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   4 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |  37 ++++----
 drivers/net/phy/phylink.c                         |   9 +-
 include/linux/phylink.h                           |  57 ++++++++----
 include/net/dsa.h                                 |   4 +-
 net/dsa/port.c                                    |   7 +-
 21 files changed, 350 insertions(+), 176 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King]

Propagate the resolved link parameters via the mac_link_up() call for
MACs that do not automatically track their PCS state.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/networking/sfp-phylink.rst      | 17 +++++-
 drivers/net/ethernet/cadence/macb_main.c      |  7 ++-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  7 ++-
 drivers/net/ethernet/marvell/mvneta.c         |  8 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 19 +++++--
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |  7 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  4 +-
 .../net/ethernet/xilinx/xilinx_axienet_main.c |  7 ++-
 drivers/net/phy/phylink.c                     |  9 ++-
 include/linux/phylink.h                       | 57 ++++++++++++++-----
 net/dsa/port.c                                |  4 +-
 11 files changed, 105 insertions(+), 41 deletions(-)

diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
index d753a309f9d1..8d7af28cd835 100644
--- a/Documentation/networking/sfp-phylink.rst
+++ b/Documentation/networking/sfp-phylink.rst
@@ -74,10 +74,13 @@ phylib to the sfp/phylink support.  Please send patches to improve
 this documentation.
 
 1. Optionally split the network driver's phylib update function into
-   three parts dealing with link-down, link-up and reconfiguring the
-   MAC settings. This can be done as a separate preparation commit.
+   two parts dealing with link-down and link-up. This can be done as
+   a separate preparation commit.
 
-   An example of this preparation can be found in git commit fc548b991fb0.
+   An older example of this preparation can be found in git commit
+   fc548b991fb0, although this was splitting into three parts; the
+   link-up part now includes configuring the MAC for the link settings.
+   Please see :c:func:`mac_link_up` for more information on this.
 
 2. Replace::
 
@@ -207,6 +210,14 @@ this documentation.
    using. This is particularly important for in-band negotiation
    methods such as 1000base-X and SGMII.
 
+   The :c:func:`mac_link_up` method is used to inform the MAC that the
+   link has come up. The call includes the negotiation mode and interface
+   for reference only. The finalised link parameters are also supplied
+   (speed, duplex and flow control/pause enablement settings) which
+   should be used to configure the MAC when the MAC and PCS are not
+   tightly integrated, or when the settings are not coming from in-band
+   negotiation.
+
    The :c:func:`mac_config` method is used to update the MAC with the
    requested state, and must avoid unnecessarily taking the link down
    when making changes to the MAC configuration.  This means the
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4508f0d150da..92e0c1035db8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -619,8 +619,11 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
 	netif_tx_stop_all_queues(ndev);
 }
 
-static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
-			     phy_interface_t interface, struct phy_device *phy)
+static void macb_mac_link_up(struct phylink_config *config,
+			     struct phy_device *phy,
+			     unsigned int mode, phy_interface_t interface,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause)
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 84233e467ed1..3a75c5b58f95 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -154,8 +154,11 @@ static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
 		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
 }
 
-static void dpaa2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy)
+static void dpaa2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause)
 {
 	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
 	struct dpmac_link_state *dpmac_state = &mac->state;
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index b7045b6a15c2..8eb5f7fd3bb2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3952,9 +3952,11 @@ static void mvneta_mac_link_down(struct phylink_config *config,
 	mvneta_set_eee(pp, false);
 }
 
-static void mvneta_mac_link_up(struct phylink_config *config, unsigned int mode,
-			       phy_interface_t interface,
-			       struct phy_device *phy)
+static void mvneta_mac_link_up(struct phylink_config *config,
+			       struct phy_device *phy,
+			       unsigned int mode, phy_interface_t interface,
+			       int speed, int duplex,
+			       bool tx_pause, bool rx_pause)
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 72133cbe55d4..ed8042d97e29 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -58,8 +58,11 @@ static struct {
  */
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 			     const struct phylink_link_state *state);
-static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy);
+static void mvpp2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause);
 
 /* Queue modes */
 #define MVPP2_QDIST_SINGLE_MODE	0
@@ -3473,8 +3476,9 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
 			.interface = port->phy_interface,
 		};
 		mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
-		mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
-				  port->phy_interface, NULL);
+		mvpp2_mac_link_up(&port->phylink_config, NULL,
+				  MLO_AN_INBAND, port->phy_interface,
+				  SPEED_UNKNOWN, DUPLEX_UNKNOWN, false, false);
 	}
 
 	netif_tx_start_all_queues(port->dev);
@@ -5141,8 +5145,11 @@ static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 	mvpp2_port_enable(port);
 }
 
-static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy)
+static void mvpp2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause)
 {
 	struct net_device *dev = to_net_dev(config->dev);
 	struct mvpp2_port *port = netdev_priv(dev);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8c6cfd15481c..8d28f90acfe7 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -412,9 +412,10 @@ static void mtk_mac_link_down(struct phylink_config *config, unsigned int mode,
 	mtk_w32(mac->hw, mcr, MTK_MAC_MCR(mac->id));
 }
 
-static void mtk_mac_link_up(struct phylink_config *config, unsigned int mode,
-			    phy_interface_t interface,
-			    struct phy_device *phy)
+static void mtk_mac_link_up(struct phylink_config *config,
+			    struct phy_device *phy,
+			    unsigned int mode, phy_interface_t interface,
+			    int speed, int duplex, bool tx_pause, bool rx_pause)
 {
 	struct mtk_mac *mac = container_of(config, struct mtk_mac,
 					   phylink_config);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5836b21edd7e..c1fa77ecdbe1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -950,8 +950,10 @@ static void stmmac_mac_link_down(struct phylink_config *config,
 }
 
 static void stmmac_mac_link_up(struct phylink_config *config,
+			       struct phy_device *phy,
 			       unsigned int mode, phy_interface_t interface,
-			       struct phy_device *phy)
+			       int speed, int duplex,
+			       bool tx_pause, bool rx_pause)
 {
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
 
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 20746b801959..197740781157 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1486,9 +1486,10 @@ static void axienet_mac_link_down(struct phylink_config *config,
 }
 
 static void axienet_mac_link_up(struct phylink_config *config,
-				unsigned int mode,
-				phy_interface_t interface,
-				struct phy_device *phy)
+				struct phy_device *phy,
+				unsigned int mode, phy_interface_t interface,
+				int speed, int duplex,
+				bool tx_pause, bool rx_pause)
 {
 	/* nothing meaningful to do */
 }
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 2899fbe699ab..b4367fab7899 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -480,8 +480,11 @@ static void phylink_mac_link_up(struct phylink *pl,
 	struct net_device *ndev = pl->netdev;
 
 	pl->cur_interface = link_state.interface;
-	pl->ops->mac_link_up(pl->config, pl->cur_link_an_mode,
-			     pl->cur_interface, pl->phydev);
+	pl->ops->mac_link_up(pl->config, pl->phydev,
+			     pl->cur_link_an_mode, pl->cur_interface,
+			     link_state.speed, link_state.duplex,
+			     !!(link_state.pause & MLO_PAUSE_TX),
+			     !!(link_state.pause & MLO_PAUSE_RX));
 
 	if (ndev)
 		netif_carrier_on(ndev);
@@ -547,6 +550,8 @@ static void phylink_resolve(struct work_struct *w)
 				link_state.pause = pl->phy_state.pause;
 				phylink_apply_manual_flow(pl, &link_state);
 				phylink_mac_config(pl, &link_state);
+			} else {
+				phylink_apply_manual_flow(pl, &link_state);
 			}
 			break;
 		}
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 812357c03df4..2180eb1aa254 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -91,9 +91,10 @@ struct phylink_mac_ops {
 	void (*mac_an_restart)(struct phylink_config *config);
 	void (*mac_link_down)(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface);
-	void (*mac_link_up)(struct phylink_config *config, unsigned int mode,
-			    phy_interface_t interface,
-			    struct phy_device *phy);
+	void (*mac_link_up)(struct phylink_config *config,
+			    struct phy_device *phy, unsigned int mode,
+			    phy_interface_t interface, int speed, int duplex,
+			    bool tx_pause, bool rx_pause);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -152,6 +153,9 @@ void mac_pcs_get_state(struct phylink_config *config,
  * guaranteed to be correct, and so any mac_config() implementation must
  * never reference these fields.
  *
+ * (this requires a rewrite - please refer to mac_link_up() for situations
+ *  where the PCS and MAC are not tightly integrated.)
+ *
  * In all negotiation modes, as defined by @mode, @state->pause indicates the
  * pause settings which should be applied as follows. If %MLO_PAUSE_AN is not
  * set, %MLO_PAUSE_TX and %MLO_PAUSE_RX indicate whether the MAC should send
@@ -162,12 +166,20 @@ void mac_pcs_get_state(struct phylink_config *config,
  * The action performed depends on the currently selected mode:
  *
  * %MLO_AN_FIXED, %MLO_AN_PHY:
- *   Configure the specified @state->speed and @state->duplex over a link
- *   specified by @state->interface. @state->advertising may be used, but
- *   is not required. Pause modes as above. Other members of @state must
- *   be ignored.
+ *   Configure for non-inband negotiation mode, where the link settings
+ *   are completely communicated via mac_link_up().  The physical link
+ *   protocol from the MAC is specified by @state->interface.
+ *
+ *   @state->advertising may be used, but is not required.
+ *
+ *   Older drivers (prior to the mac_link_up() change) may use @state->speed,
+ *   @state->duplex and @state->pause to configure the MAC, but this is
+ *   deprecated; such drivers should be converted to use mac_link_up().
  *
- *   Valid state members: interface, speed, duplex, pause, advertising.
+ *   Other members of @state must be ignored.
+ *
+ *   Valid state members: interface, advertising.
+ *   Deprecated state members: speed, duplex, pause.
  *
  * %MLO_AN_INBAND:
  *   place the link in an inband negotiation mode (such as 802.3z
@@ -228,19 +240,34 @@ void mac_link_down(struct phylink_config *config, unsigned int mode,
 /**
  * mac_link_up() - allow the link to come up
  * @config: a pointer to a &struct phylink_config.
+ * @phy: any attached phy
  * @mode: link autonegotiation mode
  * @interface: link &typedef phy_interface_t mode
- * @phy: any attached phy
+ * @speed: link speed
+ * @duplex: link duplex
+ * @tx_pause: link transmit pause enablement status
+ * @rx_pause: link receive pause enablement status
  *
- * If @mode is not an in-band negotiation mode (as defined by
- * phylink_autoneg_inband()), allow the link to come up. If @phy
- * is non-%NULL, configure Energy Efficient Ethernet by calling
+ * Configure the MAC for an established link.
+ *
+ * @speed, @duplex, @tx_pause and @rx_pause indicate the finalised link
+ * settings, and should be used to configure the MAC block appropriately
+ * where these settings are not automatically conveyed from the PCS block,
+ * or if in-band negotiation (as defined by phylink_autoneg_inband(@mode))
+ * is disabled.
+ *
+ * Note that when 802.3z in-band negotiation is in use, it is possible
+ * that the user wishes to override the pause settings, and this should
+ * be allowed when considering the implementation of this method.
+ *
+ * If in-band negotiation mode is disabled, allow the link to come up. If
+ * @phy is non-%NULL, configure Energy Efficient Ethernet by calling
  * phy_init_eee() and perform appropriate MAC configuration for EEE.
  * Interface type selection must be done in mac_config().
  */
-void mac_link_up(struct phylink_config *config, unsigned int mode,
-		 phy_interface_t interface,
-		 struct phy_device *phy);
+void mac_link_up(struct phylink_config *config, struct phy_device *phy,
+		 unsigned int mode, phy_interface_t interface,
+		 int speed, int duplex, bool tx_pause, bool rx_pause);
 #endif
 
 struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 774facb8d547..b2f5262b35cf 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -489,9 +489,11 @@ static void dsa_port_phylink_mac_link_down(struct phylink_config *config,
 }
 
 static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
+					 struct phy_device *phydev,
 					 unsigned int mode,
 					 phy_interface_t interface,
-					 struct phy_device *phydev)
+					 int speed, int duplex,
+					 bool tx_pause, bool rx_pause)
 {
 	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
-- 
2.20.1

[CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King]

Propagate the resolved link parameters via the mac_link_up() call for
MACs that do not automatically track their PCS state.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/networking/sfp-phylink.rst      | 17 +++++-
 drivers/net/ethernet/cadence/macb_main.c      |  7 ++-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  7 ++-
 drivers/net/ethernet/marvell/mvneta.c         |  8 ++-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 19 +++++--
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |  7 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  4 +-
 .../net/ethernet/xilinx/xilinx_axienet_main.c |  7 ++-
 drivers/net/phy/phylink.c                     |  9 ++-
 include/linux/phylink.h                       | 57 ++++++++++++++-----
 net/dsa/port.c                                |  4 +-
 11 files changed, 105 insertions(+), 41 deletions(-)

diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
index d753a309f9d1..8d7af28cd835 100644
--- a/Documentation/networking/sfp-phylink.rst
+++ b/Documentation/networking/sfp-phylink.rst
@@ -74,10 +74,13 @@ phylib to the sfp/phylink support.  Please send patches to improve
 this documentation.
 
 1. Optionally split the network driver's phylib update function into
-   three parts dealing with link-down, link-up and reconfiguring the
-   MAC settings. This can be done as a separate preparation commit.
+   two parts dealing with link-down and link-up. This can be done as
+   a separate preparation commit.
 
-   An example of this preparation can be found in git commit fc548b991fb0.
+   An older example of this preparation can be found in git commit
+   fc548b991fb0, although this was splitting into three parts; the
+   link-up part now includes configuring the MAC for the link settings.
+   Please see :c:func:`mac_link_up` for more information on this.
 
 2. Replace::
 
@@ -207,6 +210,14 @@ this documentation.
    using. This is particularly important for in-band negotiation
    methods such as 1000base-X and SGMII.
 
+   The :c:func:`mac_link_up` method is used to inform the MAC that the
+   link has come up. The call includes the negotiation mode and interface
+   for reference only. The finalised link parameters are also supplied
+   (speed, duplex and flow control/pause enablement settings) which
+   should be used to configure the MAC when the MAC and PCS are not
+   tightly integrated, or when the settings are not coming from in-band
+   negotiation.
+
    The :c:func:`mac_config` method is used to update the MAC with the
    requested state, and must avoid unnecessarily taking the link down
    when making changes to the MAC configuration.  This means the
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4508f0d150da..92e0c1035db8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -619,8 +619,11 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
 	netif_tx_stop_all_queues(ndev);
 }
 
-static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
-			     phy_interface_t interface, struct phy_device *phy)
+static void macb_mac_link_up(struct phylink_config *config,
+			     struct phy_device *phy,
+			     unsigned int mode, phy_interface_t interface,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause)
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 84233e467ed1..3a75c5b58f95 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -154,8 +154,11 @@ static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
 		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
 }
 
-static void dpaa2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy)
+static void dpaa2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause)
 {
 	struct dpaa2_mac *mac = phylink_to_dpaa2_mac(config);
 	struct dpmac_link_state *dpmac_state = &mac->state;
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index b7045b6a15c2..8eb5f7fd3bb2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3952,9 +3952,11 @@ static void mvneta_mac_link_down(struct phylink_config *config,
 	mvneta_set_eee(pp, false);
 }
 
-static void mvneta_mac_link_up(struct phylink_config *config, unsigned int mode,
-			       phy_interface_t interface,
-			       struct phy_device *phy)
+static void mvneta_mac_link_up(struct phylink_config *config,
+			       struct phy_device *phy,
+			       unsigned int mode, phy_interface_t interface,
+			       int speed, int duplex,
+			       bool tx_pause, bool rx_pause)
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 72133cbe55d4..ed8042d97e29 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -58,8 +58,11 @@ static struct {
  */
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 			     const struct phylink_link_state *state);
-static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy);
+static void mvpp2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause);
 
 /* Queue modes */
 #define MVPP2_QDIST_SINGLE_MODE	0
@@ -3473,8 +3476,9 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
 			.interface = port->phy_interface,
 		};
 		mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
-		mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
-				  port->phy_interface, NULL);
+		mvpp2_mac_link_up(&port->phylink_config, NULL,
+				  MLO_AN_INBAND, port->phy_interface,
+				  SPEED_UNKNOWN, DUPLEX_UNKNOWN, false, false);
 	}
 
 	netif_tx_start_all_queues(port->dev);
@@ -5141,8 +5145,11 @@ static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 	mvpp2_port_enable(port);
 }
 
-static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
-			      phy_interface_t interface, struct phy_device *phy)
+static void mvpp2_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause)
 {
 	struct net_device *dev = to_net_dev(config->dev);
 	struct mvpp2_port *port = netdev_priv(dev);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8c6cfd15481c..8d28f90acfe7 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -412,9 +412,10 @@ static void mtk_mac_link_down(struct phylink_config *config, unsigned int mode,
 	mtk_w32(mac->hw, mcr, MTK_MAC_MCR(mac->id));
 }
 
-static void mtk_mac_link_up(struct phylink_config *config, unsigned int mode,
-			    phy_interface_t interface,
-			    struct phy_device *phy)
+static void mtk_mac_link_up(struct phylink_config *config,
+			    struct phy_device *phy,
+			    unsigned int mode, phy_interface_t interface,
+			    int speed, int duplex, bool tx_pause, bool rx_pause)
 {
 	struct mtk_mac *mac = container_of(config, struct mtk_mac,
 					   phylink_config);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5836b21edd7e..c1fa77ecdbe1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -950,8 +950,10 @@ static void stmmac_mac_link_down(struct phylink_config *config,
 }
 
 static void stmmac_mac_link_up(struct phylink_config *config,
+			       struct phy_device *phy,
 			       unsigned int mode, phy_interface_t interface,
-			       struct phy_device *phy)
+			       int speed, int duplex,
+			       bool tx_pause, bool rx_pause)
 {
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
 
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 20746b801959..197740781157 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1486,9 +1486,10 @@ static void axienet_mac_link_down(struct phylink_config *config,
 }
 
 static void axienet_mac_link_up(struct phylink_config *config,
-				unsigned int mode,
-				phy_interface_t interface,
-				struct phy_device *phy)
+				struct phy_device *phy,
+				unsigned int mode, phy_interface_t interface,
+				int speed, int duplex,
+				bool tx_pause, bool rx_pause)
 {
 	/* nothing meaningful to do */
 }
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 2899fbe699ab..b4367fab7899 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -480,8 +480,11 @@ static void phylink_mac_link_up(struct phylink *pl,
 	struct net_device *ndev = pl->netdev;
 
 	pl->cur_interface = link_state.interface;
-	pl->ops->mac_link_up(pl->config, pl->cur_link_an_mode,
-			     pl->cur_interface, pl->phydev);
+	pl->ops->mac_link_up(pl->config, pl->phydev,
+			     pl->cur_link_an_mode, pl->cur_interface,
+			     link_state.speed, link_state.duplex,
+			     !!(link_state.pause & MLO_PAUSE_TX),
+			     !!(link_state.pause & MLO_PAUSE_RX));
 
 	if (ndev)
 		netif_carrier_on(ndev);
@@ -547,6 +550,8 @@ static void phylink_resolve(struct work_struct *w)
 				link_state.pause = pl->phy_state.pause;
 				phylink_apply_manual_flow(pl, &link_state);
 				phylink_mac_config(pl, &link_state);
+			} else {
+				phylink_apply_manual_flow(pl, &link_state);
 			}
 			break;
 		}
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 812357c03df4..2180eb1aa254 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -91,9 +91,10 @@ struct phylink_mac_ops {
 	void (*mac_an_restart)(struct phylink_config *config);
 	void (*mac_link_down)(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface);
-	void (*mac_link_up)(struct phylink_config *config, unsigned int mode,
-			    phy_interface_t interface,
-			    struct phy_device *phy);
+	void (*mac_link_up)(struct phylink_config *config,
+			    struct phy_device *phy, unsigned int mode,
+			    phy_interface_t interface, int speed, int duplex,
+			    bool tx_pause, bool rx_pause);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -152,6 +153,9 @@ void mac_pcs_get_state(struct phylink_config *config,
  * guaranteed to be correct, and so any mac_config() implementation must
  * never reference these fields.
  *
+ * (this requires a rewrite - please refer to mac_link_up() for situations
+ *  where the PCS and MAC are not tightly integrated.)
+ *
  * In all negotiation modes, as defined by @mode, @state->pause indicates the
  * pause settings which should be applied as follows. If %MLO_PAUSE_AN is not
  * set, %MLO_PAUSE_TX and %MLO_PAUSE_RX indicate whether the MAC should send
@@ -162,12 +166,20 @@ void mac_pcs_get_state(struct phylink_config *config,
  * The action performed depends on the currently selected mode:
  *
  * %MLO_AN_FIXED, %MLO_AN_PHY:
- *   Configure the specified @state->speed and @state->duplex over a link
- *   specified by @state->interface. @state->advertising may be used, but
- *   is not required. Pause modes as above. Other members of @state must
- *   be ignored.
+ *   Configure for non-inband negotiation mode, where the link settings
+ *   are completely communicated via mac_link_up().  The physical link
+ *   protocol from the MAC is specified by @state->interface.
+ *
+ *   @state->advertising may be used, but is not required.
+ *
+ *   Older drivers (prior to the mac_link_up() change) may use @state->speed,
+ *   @state->duplex and @state->pause to configure the MAC, but this is
+ *   deprecated; such drivers should be converted to use mac_link_up().
  *
- *   Valid state members: interface, speed, duplex, pause, advertising.
+ *   Other members of @state must be ignored.
+ *
+ *   Valid state members: interface, advertising.
+ *   Deprecated state members: speed, duplex, pause.
  *
  * %MLO_AN_INBAND:
  *   place the link in an inband negotiation mode (such as 802.3z
@@ -228,19 +240,34 @@ void mac_link_down(struct phylink_config *config, unsigned int mode,
 /**
  * mac_link_up() - allow the link to come up
  * @config: a pointer to a &struct phylink_config.
+ * @phy: any attached phy
  * @mode: link autonegotiation mode
  * @interface: link &typedef phy_interface_t mode
- * @phy: any attached phy
+ * @speed: link speed
+ * @duplex: link duplex
+ * @tx_pause: link transmit pause enablement status
+ * @rx_pause: link receive pause enablement status
  *
- * If @mode is not an in-band negotiation mode (as defined by
- * phylink_autoneg_inband()), allow the link to come up. If @phy
- * is non-%NULL, configure Energy Efficient Ethernet by calling
+ * Configure the MAC for an established link.
+ *
+ * @speed, @duplex, @tx_pause and @rx_pause indicate the finalised link
+ * settings, and should be used to configure the MAC block appropriately
+ * where these settings are not automatically conveyed from the PCS block,
+ * or if in-band negotiation (as defined by phylink_autoneg_inband(@mode))
+ * is disabled.
+ *
+ * Note that when 802.3z in-band negotiation is in use, it is possible
+ * that the user wishes to override the pause settings, and this should
+ * be allowed when considering the implementation of this method.
+ *
+ * If in-band negotiation mode is disabled, allow the link to come up. If
+ * @phy is non-%NULL, configure Energy Efficient Ethernet by calling
  * phy_init_eee() and perform appropriate MAC configuration for EEE.
  * Interface type selection must be done in mac_config().
  */
-void mac_link_up(struct phylink_config *config, unsigned int mode,
-		 phy_interface_t interface,
-		 struct phy_device *phy);
+void mac_link_up(struct phylink_config *config, struct phy_device *phy,
+		 unsigned int mode, phy_interface_t interface,
+		 int speed, int duplex, bool tx_pause, bool rx_pause);
 #endif
 
 struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 774facb8d547..b2f5262b35cf 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -489,9 +489,11 @@ static void dsa_port_phylink_mac_link_down(struct phylink_config *config,
 }
 
 static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
+					 struct phy_device *phydev,
 					 unsigned int mode,
 					 phy_interface_t interface,
-					 struct phy_device *phydev)
+					 int speed, int duplex,
+					 bool tx_pause, bool rx_pause)
 {
 	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
-- 
2.20.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[CFT 2/8] net: dsa: propagate resolved link config via mac_link_up()

[Russell King]

Propagate the resolved link configuration down via DSA's
phylink_mac_link_up() operation to allow split PCS/MAC to work.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c       | 4 +++-
 drivers/net/dsa/b53/b53_priv.h         | 4 +++-
 drivers/net/dsa/bcm_sf2.c              | 4 +++-
 drivers/net/dsa/lantiq_gswip.c         | 4 +++-
 drivers/net/dsa/mt7530.c               | 4 +++-
 drivers/net/dsa/mv88e6xxx/chip.c       | 4 +++-
 drivers/net/dsa/sja1105/sja1105_main.c | 4 +++-
 include/net/dsa.h                      | 4 +++-
 net/dsa/port.c                         | 3 ++-
 9 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 449a22172e07..0a916fe00176 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1289,7 +1289,9 @@ EXPORT_SYMBOL(b53_phylink_mac_link_down);
 void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     unsigned int mode,
 			     phy_interface_t interface,
-			     struct phy_device *phydev)
+			     struct phy_device *phydev,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause)
 {
 	struct b53_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 3c30f3a7eb29..3d42318bc3f1 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -338,7 +338,9 @@ void b53_phylink_mac_link_down(struct dsa_switch *ds, int port,
 void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     unsigned int mode,
 			     phy_interface_t interface,
-			     struct phy_device *phydev);
+			     struct phy_device *phydev,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause);
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
 int b53_vlan_prepare(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 6feaf8cb0809..a1110133dadf 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -652,7 +652,9 @@ static void bcm_sf2_sw_mac_link_down(struct dsa_switch *ds, int port,
 static void bcm_sf2_sw_mac_link_up(struct dsa_switch *ds, int port,
 				   unsigned int mode,
 				   phy_interface_t interface,
-				   struct phy_device *phydev)
+				   struct phy_device *phydev,
+				   int speed, int duplex,
+				   bool tx_pause, bool rx_pause)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_eee *p = &priv->dev->ports[port].eee;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 0369c22fe3e1..cf6fa8fede33 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1517,7 +1517,9 @@ static void gswip_phylink_mac_link_down(struct dsa_switch *ds, int port,
 static void gswip_phylink_mac_link_up(struct dsa_switch *ds, int port,
 				      unsigned int mode,
 				      phy_interface_t interface,
-				      struct phy_device *phydev)
+				      struct phy_device *phydev,
+				      int speed, int duplex,
+				      bool tx_pause, bool rx_pause)
 {
 	struct gswip_priv *priv = ds->priv;
 
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 022466ca1c19..86818ab3bb07 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1482,7 +1482,9 @@ static void mt7530_phylink_mac_link_down(struct dsa_switch *ds, int port,
 static void mt7530_phylink_mac_link_up(struct dsa_switch *ds, int port,
 				       unsigned int mode,
 				       phy_interface_t interface,
-				       struct phy_device *phydev)
+				       struct phy_device *phydev,
+				       int speed, int duplex,
+				       bool tx_pause, bool rx_pause)
 {
 	struct mt7530_priv *priv = ds->priv;
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 4ec09cc8dcdc..fef3b5e0b291 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -655,7 +655,9 @@ static void mv88e6xxx_mac_link_down(struct dsa_switch *ds, int port,
 
 static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
 				  unsigned int mode, phy_interface_t interface,
-				  struct phy_device *phydev)
+				  struct phy_device *phydev,
+				  int speed, int duplex,
+				  bool tx_pause, bool rx_pause)
 {
 	if (mode == MLO_AN_FIXED)
 		mv88e6xxx_mac_link_force(ds, port, LINK_FORCED_UP);
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 03ba6d25f7fe..c27cc7b37440 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -786,7 +786,9 @@ static void sja1105_mac_link_down(struct dsa_switch *ds, int port,
 static void sja1105_mac_link_up(struct dsa_switch *ds, int port,
 				unsigned int mode,
 				phy_interface_t interface,
-				struct phy_device *phydev)
+				struct phy_device *phydev,
+				int speed, int duplex,
+				bool tx_pause, bool rx_pause)
 {
 	sja1105_inhibit_tx(ds->priv, BIT(port), false);
 }
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 63495e3443ac..7d3d84f0ef42 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -420,7 +420,9 @@ struct dsa_switch_ops {
 	void	(*phylink_mac_link_up)(struct dsa_switch *ds, int port,
 				       unsigned int mode,
 				       phy_interface_t interface,
-				       struct phy_device *phydev);
+				       struct phy_device *phydev,
+				       int speed, int duplex,
+				       bool tx_pause, bool rx_pause);
 	void	(*phylink_fixed_state)(struct dsa_switch *ds, int port,
 				       struct phylink_link_state *state);
 	/*
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b2f5262b35cf..d4450a454249 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -504,7 +504,8 @@ static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
 		return;
 	}
 
-	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
+	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev,
+				     speed, duplex, tx_pause, rx_pause);
 }
 
 const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
-- 
2.20.1

[CFT 2/8] net: dsa: propagate resolved link config via mac_link_up()

[Russell King]

Propagate the resolved link configuration down via DSA's
phylink_mac_link_up() operation to allow split PCS/MAC to work.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/b53/b53_common.c       | 4 +++-
 drivers/net/dsa/b53/b53_priv.h         | 4 +++-
 drivers/net/dsa/bcm_sf2.c              | 4 +++-
 drivers/net/dsa/lantiq_gswip.c         | 4 +++-
 drivers/net/dsa/mt7530.c               | 4 +++-
 drivers/net/dsa/mv88e6xxx/chip.c       | 4 +++-
 drivers/net/dsa/sja1105/sja1105_main.c | 4 +++-
 include/net/dsa.h                      | 4 +++-
 net/dsa/port.c                         | 3 ++-
 9 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 449a22172e07..0a916fe00176 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1289,7 +1289,9 @@ EXPORT_SYMBOL(b53_phylink_mac_link_down);
 void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     unsigned int mode,
 			     phy_interface_t interface,
-			     struct phy_device *phydev)
+			     struct phy_device *phydev,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause)
 {
 	struct b53_device *dev = ds->priv;
 
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 3c30f3a7eb29..3d42318bc3f1 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -338,7 +338,9 @@ void b53_phylink_mac_link_down(struct dsa_switch *ds, int port,
 void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     unsigned int mode,
 			     phy_interface_t interface,
-			     struct phy_device *phydev);
+			     struct phy_device *phydev,
+			     int speed, int duplex,
+			     bool tx_pause, bool rx_pause);
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
 int b53_vlan_prepare(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 6feaf8cb0809..a1110133dadf 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -652,7 +652,9 @@ static void bcm_sf2_sw_mac_link_down(struct dsa_switch *ds, int port,
 static void bcm_sf2_sw_mac_link_up(struct dsa_switch *ds, int port,
 				   unsigned int mode,
 				   phy_interface_t interface,
-				   struct phy_device *phydev)
+				   struct phy_device *phydev,
+				   int speed, int duplex,
+				   bool tx_pause, bool rx_pause)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_eee *p = &priv->dev->ports[port].eee;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 0369c22fe3e1..cf6fa8fede33 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1517,7 +1517,9 @@ static void gswip_phylink_mac_link_down(struct dsa_switch *ds, int port,
 static void gswip_phylink_mac_link_up(struct dsa_switch *ds, int port,
 				      unsigned int mode,
 				      phy_interface_t interface,
-				      struct phy_device *phydev)
+				      struct phy_device *phydev,
+				      int speed, int duplex,
+				      bool tx_pause, bool rx_pause)
 {
 	struct gswip_priv *priv = ds->priv;
 
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 022466ca1c19..86818ab3bb07 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1482,7 +1482,9 @@ static void mt7530_phylink_mac_link_down(struct dsa_switch *ds, int port,
 static void mt7530_phylink_mac_link_up(struct dsa_switch *ds, int port,
 				       unsigned int mode,
 				       phy_interface_t interface,
-				       struct phy_device *phydev)
+				       struct phy_device *phydev,
+				       int speed, int duplex,
+				       bool tx_pause, bool rx_pause)
 {
 	struct mt7530_priv *priv = ds->priv;
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 4ec09cc8dcdc..fef3b5e0b291 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -655,7 +655,9 @@ static void mv88e6xxx_mac_link_down(struct dsa_switch *ds, int port,
 
 static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
 				  unsigned int mode, phy_interface_t interface,
-				  struct phy_device *phydev)
+				  struct phy_device *phydev,
+				  int speed, int duplex,
+				  bool tx_pause, bool rx_pause)
 {
 	if (mode == MLO_AN_FIXED)
 		mv88e6xxx_mac_link_force(ds, port, LINK_FORCED_UP);
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 03ba6d25f7fe..c27cc7b37440 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -786,7 +786,9 @@ static void sja1105_mac_link_down(struct dsa_switch *ds, int port,
 static void sja1105_mac_link_up(struct dsa_switch *ds, int port,
 				unsigned int mode,
 				phy_interface_t interface,
-				struct phy_device *phydev)
+				struct phy_device *phydev,
+				int speed, int duplex,
+				bool tx_pause, bool rx_pause)
 {
 	sja1105_inhibit_tx(ds->priv, BIT(port), false);
 }
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 63495e3443ac..7d3d84f0ef42 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -420,7 +420,9 @@ struct dsa_switch_ops {
 	void	(*phylink_mac_link_up)(struct dsa_switch *ds, int port,
 				       unsigned int mode,
 				       phy_interface_t interface,
-				       struct phy_device *phydev);
+				       struct phy_device *phydev,
+				       int speed, int duplex,
+				       bool tx_pause, bool rx_pause);
 	void	(*phylink_fixed_state)(struct dsa_switch *ds, int port,
 				       struct phylink_link_state *state);
 	/*
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b2f5262b35cf..d4450a454249 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -504,7 +504,8 @@ static void dsa_port_phylink_mac_link_up(struct phylink_config *config,
 		return;
 	}
 
-	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
+	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev,
+				     speed, duplex, tx_pause, rx_pause);
 }
 
 const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
-- 
2.20.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[CFT 3/8] net: mv88e6xxx: use resolved link config in mac_link_up()

[Russell King]

Use the resolved link configuration to set the MAC configuration when
mac_link_up() for non-internal-PHY ports.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 75 +++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fef3b5e0b291..4a4173e63fa5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -632,25 +632,30 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
 		dev_err(ds->dev, "p%d: failed to configure MAC\n", port);
 }
 
-static void mv88e6xxx_mac_link_force(struct dsa_switch *ds, int port, int link)
+static void mv88e6xxx_mac_link_down(struct dsa_switch *ds, int port,
+				    unsigned int mode,
+				    phy_interface_t interface)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
+	const struct mv88e6xxx_ops *ops;
+	int err = 0;
 
-	mv88e6xxx_reg_lock(chip);
-	err = chip->info->ops->port_set_link(chip, port, link);
-	mv88e6xxx_reg_unlock(chip);
+	ops = chip->info->ops;
 
-	if (err)
-		dev_err(chip->dev, "p%d: failed to force MAC link\n", port);
-}
+	/* Internal PHYs propagate their configuration directly to the MAC.
+	 * External PHYs depend on whether the PPU is enabled for this port.
+	 * FIXME: we should be using the PPU enable state here. What about
+	 * an automedia port?
+	 */
+	if (!mv88e6xxx_phy_is_internal(ds, port) && ops->port_set_link) {
+		mv88e6xxx_reg_lock(chip);
+		err = ops->port_set_link(chip, port, LINK_FORCED_DOWN);
+		mv88e6xxx_reg_unlock(chip);
 
-static void mv88e6xxx_mac_link_down(struct dsa_switch *ds, int port,
-				    unsigned int mode,
-				    phy_interface_t interface)
-{
-	if (mode == MLO_AN_FIXED)
-		mv88e6xxx_mac_link_force(ds, port, LINK_FORCED_DOWN);
+		if (err)
+			dev_err(chip->dev,
+				"p%d: failed to force MAC link down\n", port);
+	}
 }
 
 static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
@@ -659,8 +664,46 @@ static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
 				  int speed, int duplex,
 				  bool tx_pause, bool rx_pause)
 {
-	if (mode == MLO_AN_FIXED)
-		mv88e6xxx_mac_link_force(ds, port, LINK_FORCED_UP);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	const struct mv88e6xxx_ops *ops;
+	int err = 0;
+
+	ops = chip->info->ops;
+
+	/* Internal PHYs propagate their configuration directly to the MAC.
+	 * External PHYs depend on whether the PPU is enabled for this port.
+	 * FIXME: we should be using the PPU enable state here. What about
+	 * an automedia port?
+	 */
+	if (!mv88e6xxx_phy_is_internal(ds, port)) {
+		mv88e6xxx_reg_lock(chip);
+		/* FIXME: for an automedia port, should we force the link
+		 * down here - what if the link comes up due to "other" media
+		 * while we're bringing the port up, how is the exclusivity
+		 * handled in the Marvell hardware? E.g. port 4 on 88E6532
+		 * shared between internal PHY and Serdes.
+		 */
+		if (ops->port_set_speed) {
+			err = ops->port_set_speed(chip, port, speed);
+			if (err && err != -EOPNOTSUPP)
+				goto error;
+		}
+
+		if (ops->port_set_duplex) {
+			err = ops->port_set_duplex(chip, port, duplex);
+			if (err && err != -EOPNOTSUPP)
+				goto error;
+		}
+
+		if (ops->port_set_link)
+			err = ops->port_set_link(chip, port, LINK_FORCED_UP);
+error:
+		mv88e6xxx_reg_unlock(chip);
+
+		if (err && err != -EOPNOTSUPP)
+			dev_err(ds->dev,
+				"p%d: failed to configure MAC link up\n", port);
+	}
 }
 
 static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
-- 
2.20.1

[CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Russell King]

Convert the Xilinx AXI ethernet driver to use the finalised link
parameters in mac_link_up() rather than the parameters in mac_config().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++++++++----------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 197740781157..c2f4c5ca2e80 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1440,6 +1440,22 @@ static void axienet_mac_an_restart(struct phylink_config *config)
 
 static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
 			       const struct phylink_link_state *state)
+{
+	/* nothing meaningful to do */
+}
+
+static void axienet_mac_link_down(struct phylink_config *config,
+				  unsigned int mode,
+				  phy_interface_t interface)
+{
+	/* nothing meaningful to do */
+}
+
+static void axienet_mac_link_up(struct phylink_config *config,
+				struct phy_device *phy,
+				unsigned int mode, phy_interface_t interface,
+				int speed, int duplex,
+				bool tx_pause, bool rx_pause)
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct axienet_local *lp = netdev_priv(ndev);
@@ -1448,7 +1464,7 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
 	emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET);
 	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
 
-	switch (state->speed) {
+	switch (speed) {
 	case SPEED_1000:
 		emmc_reg |= XAE_EMMC_LINKSPD_1000;
 		break;
@@ -1467,33 +1483,17 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
 	axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg);
 
 	fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET);
-	if (state->pause & MLO_PAUSE_TX)
+	if (tx_pause)
 		fcc_reg |= XAE_FCC_FCTX_MASK;
 	else
 		fcc_reg &= ~XAE_FCC_FCTX_MASK;
-	if (state->pause & MLO_PAUSE_RX)
+	if (rx_pause)
 		fcc_reg |= XAE_FCC_FCRX_MASK;
 	else
 		fcc_reg &= ~XAE_FCC_FCRX_MASK;
 	axienet_iow(lp, XAE_FCC_OFFSET, fcc_reg);
 }
 
-static void axienet_mac_link_down(struct phylink_config *config,
-				  unsigned int mode,
-				  phy_interface_t interface)
-{
-	/* nothing meaningful to do */
-}
-
-static void axienet_mac_link_up(struct phylink_config *config,
-				struct phy_device *phy,
-				unsigned int mode, phy_interface_t interface,
-				int speed, int duplex,
-				bool tx_pause, bool rx_pause)
-{
-	/* nothing meaningful to do */
-}
-
 static const struct phylink_mac_ops axienet_phylink_ops = {
 	.validate = axienet_validate,
 	.mac_pcs_get_state = axienet_mac_pcs_get_state,
-- 
2.20.1

[CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()

[Russell King]

Convert the DPAA2 ethernet driver to use the finalised link parameters
in mac_link_up() rather than the parameters in mac_config(), which are
more suited to the needs of the DPAA2 MC firmware than those available
via mac_config().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54 +++++++++++--------
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |  1 +
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 3a75c5b58f95..3ee236c5fc37 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
 	struct dpmac_link_state *dpmac_state = &mac->state;
 	int err;
 
-	if (state->speed != SPEED_UNKNOWN)
-		dpmac_state->rate = state->speed;
-
-	if (state->duplex != DUPLEX_UNKNOWN) {
-		if (!state->duplex)
-			dpmac_state->options |= DPMAC_LINK_OPT_HALF_DUPLEX;
-		else
-			dpmac_state->options &= ~DPMAC_LINK_OPT_HALF_DUPLEX;
-	}
-
 	if (state->an_enabled)
 		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
 	else
 		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
 
-	if (state->pause & MLO_PAUSE_RX)
-		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
-	else
-		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
-
-	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause & MLO_PAUSE_TX))
-		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
-	else
-		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
-
 	err = dpmac_set_link_state(mac->mc_io, 0,
 				   mac->mc_dev->mc_handle, dpmac_state);
 	if (err)
-		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
+		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() = %d\n",
+			   __func__, err);
 }
 
 static void dpaa2_mac_link_up(struct phylink_config *config,
@@ -165,10 +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config *config,
 	int err;
 
 	dpmac_state->up = 1;
+
+	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
+		/* If the DPMAC is configured for PHY mode, we need
+		 * to pass the link parameters to the MC firmware.
+		 */
+		dpmac_state->rate = speed;
+
+		if (duplex == DUPLEX_HALF)
+			dpmac_state->options |= DPMAC_LINK_OPT_HALF_DUPLEX;
+		else if (duplex == DUPLEX_FULL)
+			dpmac_state->options &= ~DPMAC_LINK_OPT_HALF_DUPLEX;
+
+		/* This is lossy; the firmware really should take the pause
+		 * enablement status rather than pause/asym pause status.
+		 */
+		if (rx_pause)
+			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
+		else
+			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
+
+		if (rx_pause ^ tx_pause)
+			dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
+		else
+			dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
+	}
+
 	err = dpmac_set_link_state(mac->mc_io, 0,
 				   mac->mc_dev->mc_handle, dpmac_state);
 	if (err)
-		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
+		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() = %d\n",
+			   __func__, err);
 }
 
 static void dpaa2_mac_link_down(struct phylink_config *config,
@@ -241,6 +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
 		goto err_close_dpmac;
 	}
 
+	mac->if_link_type = attr.link_type;
+
 	dpmac_node = dpaa2_mac_get_node(attr.id);
 	if (!dpmac_node) {
 		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index 4da8079b9155..2130d9c7d40e 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -20,6 +20,7 @@ struct dpaa2_mac {
 	struct phylink_config phylink_config;
 	struct phylink *phylink;
 	phy_interface_t if_mode;
+	enum dpmac_link_type if_link_type;
 };
 
 bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
-- 
2.20.1

[CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Russell King]

Convert the macb ethernet driver to use the finalised link
parameters in mac_link_up() rather than the parameters in mac_config().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/cadence/macb.h      |  1 -
 drivers/net/ethernet/cadence/macb_main.c | 46 ++++++++++++++----------
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index dbf7070fcdba..e99608f27d23 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1199,7 +1199,6 @@ struct macb {
 	unsigned int		dma_burst_length;
 
 	phy_interface_t		phy_interface;
-	int			speed;
 
 	/* AT91RM9200 transmit */
 	struct sk_buff *skb;			/* holds skb until xmit interrupt completes */
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 92e0c1035db8..1332523d7dcb 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -572,20 +572,7 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
 	old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
 
 	/* Clear all the bits we might set later */
-	ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE) |
-		  GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
-
-	if (state->speed == SPEED_1000)
-		ctrl |= GEM_BIT(GBE);
-	else if (state->speed == SPEED_100)
-		ctrl |= MACB_BIT(SPD);
-
-	if (state->duplex)
-		ctrl |= MACB_BIT(FD);
-
-	/* We do not support MLO_PAUSE_RX yet */
-	if (state->pause & MLO_PAUSE_TX)
-		ctrl |= MACB_BIT(PAE);
+	ctrl &= ~(GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
 
 	if (state->interface == PHY_INTERFACE_MODE_SGMII)
 		ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
@@ -594,8 +581,6 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
 	if (old_ctrl ^ ctrl)
 		macb_or_gem_writel(bp, NCFGR, ctrl);
 
-	bp->speed = state->speed;
-
 	spin_unlock_irqrestore(&bp->lock, flags);
 }
 
@@ -628,9 +613,34 @@ static void macb_mac_link_up(struct phylink_config *config,
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(ndev);
 	struct macb_queue *queue;
+	unsigned long flags;
 	unsigned int q;
+	u32 ctrl;
+
+	spin_lock_irqsave(&bp->lock, flags);
+
+	ctrl = macb_or_gem_readl(bp, NCFGR);
 
-	macb_set_tx_clk(bp->tx_clk, bp->speed, ndev);
+	/* Clear all the bits we might set later */
+	ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE));
+
+	if (speed == SPEED_1000)
+		ctrl |= GEM_BIT(GBE);
+	else if (speed == SPEED_100)
+		ctrl |= MACB_BIT(SPD);
+
+	if (duplex == DUPLEX_FULL)
+		ctrl |= MACB_BIT(FD);
+
+	/* We do not support rx_pause yet */
+	if (tx_pause)
+		ctrl |= MACB_BIT(PAE);
+
+	macb_or_gem_writel(bp, NCFGR, ctrl);
+
+	spin_unlock_irqrestore(&bp->lock, flags);
+
+	macb_set_tx_clk(bp->tx_clk, speed, ndev);
 
 	/* Initialize rings & buffers as clearing MACB_BIT(TE) in link down
 	 * cleared the pipeline and control registers.
@@ -4424,8 +4434,6 @@ static int macb_probe(struct platform_device *pdev)
 	else
 		bp->phy_interface = interface;
 
-	bp->speed = SPEED_UNKNOWN;
-
 	/* IP specific init */
 	err = init(pdev);
 	if (err)
-- 
2.20.1

[CFT 7/8] net: mvneta: use resolved link config in mac_link_up()

[Russell King]

Convert the Marvell mvneta ethernet driver to use the finalised link
parameters in mac_link_up() rather than the parameters in mac_config().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 55 ++++++++++++++++++---------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 8eb5f7fd3bb2..247e7e7cbfd5 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3817,13 +3817,9 @@ static void mvneta_mac_config(struct phylink_config *config, unsigned int mode,
 	new_clk = gmac_clk & ~MVNETA_GMAC_1MS_CLOCK_ENABLE;
 	new_an = gmac_an & ~(MVNETA_GMAC_INBAND_AN_ENABLE |
 			     MVNETA_GMAC_INBAND_RESTART_AN |
-			     MVNETA_GMAC_CONFIG_MII_SPEED |
-			     MVNETA_GMAC_CONFIG_GMII_SPEED |
 			     MVNETA_GMAC_AN_SPEED_EN |
 			     MVNETA_GMAC_ADVERT_SYM_FLOW_CTRL |
-			     MVNETA_GMAC_CONFIG_FLOW_CTRL |
 			     MVNETA_GMAC_AN_FLOW_CTRL_EN |
-			     MVNETA_GMAC_CONFIG_FULL_DUPLEX |
 			     MVNETA_GMAC_AN_DUPLEX_EN);
 
 	/* Even though it might look weird, when we're configured in
@@ -3838,24 +3834,20 @@ static void mvneta_mac_config(struct phylink_config *config, unsigned int mode,
 
 	if (phylink_test(state->advertising, Pause))
 		new_an |= MVNETA_GMAC_ADVERT_SYM_FLOW_CTRL;
-	if (state->pause & MLO_PAUSE_TXRX_MASK)
-		new_an |= MVNETA_GMAC_CONFIG_FLOW_CTRL;
 
 	if (!phylink_autoneg_inband(mode)) {
-		/* Phy or fixed speed */
-		if (state->duplex)
-			new_an |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
-
-		if (state->speed == SPEED_1000 || state->speed == SPEED_2500)
-			new_an |= MVNETA_GMAC_CONFIG_GMII_SPEED;
-		else if (state->speed == SPEED_100)
-			new_an |= MVNETA_GMAC_CONFIG_MII_SPEED;
+		/* Phy or fixed speed - nothing to do, leave the
+		 * configured speed, duplex and flow control as-is.
+		 */
 	} else if (state->interface == PHY_INTERFACE_MODE_SGMII) {
 		/* SGMII mode receives the state from the PHY */
 		new_ctrl2 |= MVNETA_GMAC2_INBAND_AN_ENABLE;
 		new_clk |= MVNETA_GMAC_1MS_CLOCK_ENABLE;
 		new_an = (new_an & ~(MVNETA_GMAC_FORCE_LINK_DOWN |
-				     MVNETA_GMAC_FORCE_LINK_PASS)) |
+				     MVNETA_GMAC_FORCE_LINK_PASS |
+				     MVNETA_GMAC_CONFIG_MII_SPEED |
+				     MVNETA_GMAC_CONFIG_GMII_SPEED |
+				     MVNETA_GMAC_CONFIG_FULL_DUPLEX)) |
 			 MVNETA_GMAC_INBAND_AN_ENABLE |
 			 MVNETA_GMAC_AN_SPEED_EN |
 			 MVNETA_GMAC_AN_DUPLEX_EN;
@@ -3864,7 +3856,8 @@ static void mvneta_mac_config(struct phylink_config *config, unsigned int mode,
 		new_ctrl0 |= MVNETA_GMAC0_PORT_1000BASE_X;
 		new_clk |= MVNETA_GMAC_1MS_CLOCK_ENABLE;
 		new_an = (new_an & ~(MVNETA_GMAC_FORCE_LINK_DOWN |
-				     MVNETA_GMAC_FORCE_LINK_PASS)) |
+				     MVNETA_GMAC_FORCE_LINK_PASS |
+				     MVNETA_GMAC_CONFIG_MII_SPEED)) |
 			 MVNETA_GMAC_INBAND_AN_ENABLE |
 			 MVNETA_GMAC_CONFIG_GMII_SPEED |
 			 /* The MAC only supports FD mode */
@@ -3964,8 +3957,36 @@ static void mvneta_mac_link_up(struct phylink_config *config,
 
 	if (!phylink_autoneg_inband(mode)) {
 		val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
-		val &= ~MVNETA_GMAC_FORCE_LINK_DOWN;
+		val &= ~(MVNETA_GMAC_FORCE_LINK_DOWN |
+			 MVNETA_GMAC_CONFIG_MII_SPEED |
+			 MVNETA_GMAC_CONFIG_GMII_SPEED |
+			 MVNETA_GMAC_CONFIG_FLOW_CTRL |
+			 MVNETA_GMAC_CONFIG_FULL_DUPLEX);
 		val |= MVNETA_GMAC_FORCE_LINK_PASS;
+
+		if (speed == SPEED_1000 || speed == SPEED_2500)
+			val |= MVNETA_GMAC_CONFIG_GMII_SPEED;
+		else if (speed == SPEED_100)
+			val |= MVNETA_GMAC_CONFIG_MII_SPEED;
+
+		if (duplex == DUPLEX_FULL)
+			val |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
+
+		if (tx_pause || rx_pause)
+			val |= MVNETA_GMAC_CONFIG_FLOW_CTRL;
+
+		mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+	} else {
+		/* When inband doesn't cover flow control or flow control is
+		 * disabled, we need to manually configure it. This bit will
+		 * only have effect if MVNETA_GMAC_AN_FLOW_CTRL_EN is unset.
+		 */
+		val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
+		val &= ~MVNETA_GMAC_CONFIG_FLOW_CTRL;
+
+		if (tx_pause || rx_pause)
+			val |= MVNETA_GMAC_CONFIG_FLOW_CTRL;
+
 		mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
 	}
 
-- 
2.20.1

[CFT 8/8] net: mvpp2: use resolved link config in mac_link_up()

[Russell King]

Convert the Marvell mvpp2 ethernet driver to use the finalised link
parameters in mac_link_up() rather than the parameters in mac_config().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 83 +++++++++++--------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index ed8042d97e29..6b9c7ed2547e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4976,15 +4976,13 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 	old_ctrl2 = ctrl2 = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
 	old_ctrl4 = ctrl4 = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
 
-	an &= ~(MVPP2_GMAC_CONFIG_MII_SPEED | MVPP2_GMAC_CONFIG_GMII_SPEED |
-		MVPP2_GMAC_AN_SPEED_EN | MVPP2_GMAC_FC_ADV_EN |
+	an &= ~(MVPP2_GMAC_AN_SPEED_EN | MVPP2_GMAC_FC_ADV_EN |
 		MVPP2_GMAC_FC_ADV_ASM_EN | MVPP2_GMAC_FLOW_CTRL_AUTONEG |
-		MVPP2_GMAC_CONFIG_FULL_DUPLEX | MVPP2_GMAC_AN_DUPLEX_EN |
-		MVPP2_GMAC_IN_BAND_AUTONEG | MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS);
+		MVPP2_GMAC_AN_DUPLEX_EN | MVPP2_GMAC_IN_BAND_AUTONEG |
+		MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS);
 	ctrl0 &= ~MVPP2_GMAC_PORT_TYPE_MASK;
 	ctrl2 &= ~(MVPP2_GMAC_INBAND_AN_MASK | MVPP2_GMAC_PORT_RESET_MASK |
 		   MVPP2_GMAC_PCS_ENABLE_MASK);
-	ctrl4 &= ~(MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN);
 
 	/* Configure port type */
 	if (phy_interface_mode_is_8023z(state->interface)) {
@@ -5014,31 +5012,20 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 
 	/* Configure negotiation style */
 	if (!phylink_autoneg_inband(mode)) {
-		/* Phy or fixed speed - no in-band AN */
-		if (state->duplex)
-			an |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
-
-		if (state->speed == SPEED_1000 || state->speed == SPEED_2500)
-			an |= MVPP2_GMAC_CONFIG_GMII_SPEED;
-		else if (state->speed == SPEED_100)
-			an |= MVPP2_GMAC_CONFIG_MII_SPEED;
-
-		if (state->pause & MLO_PAUSE_TX)
-			ctrl4 |= MVPP22_CTRL4_TX_FC_EN;
-		if (state->pause & MLO_PAUSE_RX)
-			ctrl4 |= MVPP22_CTRL4_RX_FC_EN;
+		/* Phy or fixed speed - no in-band AN, nothing to do, leave the
+		 * configured speed, duplex and flow control as-is.
+		 */
 	} else if (state->interface == PHY_INTERFACE_MODE_SGMII) {
 		/* SGMII in-band mode receives the speed and duplex from
 		 * the PHY. Flow control information is not received. */
-		an &= ~(MVPP2_GMAC_FORCE_LINK_DOWN | MVPP2_GMAC_FORCE_LINK_PASS);
+		an &= ~(MVPP2_GMAC_FORCE_LINK_DOWN |
+			MVPP2_GMAC_FORCE_LINK_PASS |
+			MVPP2_GMAC_CONFIG_MII_SPEED |
+			MVPP2_GMAC_CONFIG_GMII_SPEED |
+			MVPP2_GMAC_CONFIG_FULL_DUPLEX);
 		an |= MVPP2_GMAC_IN_BAND_AUTONEG |
 		      MVPP2_GMAC_AN_SPEED_EN |
 		      MVPP2_GMAC_AN_DUPLEX_EN;
-
-		if (state->pause & MLO_PAUSE_TX)
-			ctrl4 |= MVPP22_CTRL4_TX_FC_EN;
-		if (state->pause & MLO_PAUSE_RX)
-			ctrl4 |= MVPP22_CTRL4_RX_FC_EN;
 	} else if (phy_interface_mode_is_8023z(state->interface)) {
 		/* 1000BaseX and 2500BaseX ports cannot negotiate speed nor can
 		 * they negotiate duplex: they are always operating with a fixed
@@ -5046,19 +5033,17 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 		 * speed and full duplex here.
 		 */
 		ctrl0 |= MVPP2_GMAC_PORT_TYPE_MASK;
-		an &= ~(MVPP2_GMAC_FORCE_LINK_DOWN | MVPP2_GMAC_FORCE_LINK_PASS);
+		an &= ~(MVPP2_GMAC_FORCE_LINK_DOWN |
+			MVPP2_GMAC_FORCE_LINK_PASS |
+			MVPP2_GMAC_CONFIG_MII_SPEED |
+			MVPP2_GMAC_CONFIG_GMII_SPEED |
+			MVPP2_GMAC_CONFIG_FULL_DUPLEX);
 		an |= MVPP2_GMAC_IN_BAND_AUTONEG |
 		      MVPP2_GMAC_CONFIG_GMII_SPEED |
 		      MVPP2_GMAC_CONFIG_FULL_DUPLEX;
 
-		if (state->pause & MLO_PAUSE_AN && state->an_enabled) {
+		if (state->pause & MLO_PAUSE_AN && state->an_enabled)
 			an |= MVPP2_GMAC_FLOW_CTRL_AUTONEG;
-		} else {
-			if (state->pause & MLO_PAUSE_TX)
-				ctrl4 |= MVPP22_CTRL4_TX_FC_EN;
-			if (state->pause & MLO_PAUSE_RX)
-				ctrl4 |= MVPP22_CTRL4_RX_FC_EN;
-		}
 	}
 
 /* Some fields of the auto-negotiation register require the port to be down when
@@ -5155,18 +5140,44 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 	struct mvpp2_port *port = netdev_priv(dev);
 	u32 val;
 
-	if (!phylink_autoneg_inband(mode)) {
-		if (mvpp2_is_xlg(interface)) {
+	if (mvpp2_is_xlg(interface)) {
+		if (!phylink_autoneg_inband(mode)) {
 			val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 			val &= ~MVPP22_XLG_CTRL0_FORCE_LINK_DOWN;
 			val |= MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
 			writel(val, port->base + MVPP22_XLG_CTRL0_REG);
-		} else {
+		}
+	} else {
+		if (!phylink_autoneg_inband(mode)) {
 			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-			val &= ~MVPP2_GMAC_FORCE_LINK_DOWN;
+			val &= ~(MVPP2_GMAC_FORCE_LINK_DOWN |
+				 MVPP2_GMAC_CONFIG_MII_SPEED |
+				 MVPP2_GMAC_CONFIG_GMII_SPEED |
+				 MVPP2_GMAC_CONFIG_FULL_DUPLEX);
 			val |= MVPP2_GMAC_FORCE_LINK_PASS;
+
+			if (speed == SPEED_1000 || speed == SPEED_2500)
+				val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
+			else if (speed == SPEED_100)
+				val |= MVPP2_GMAC_CONFIG_MII_SPEED;
+
+			if (duplex == DUPLEX_FULL)
+				val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
+
 			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 		}
+
+		/* We can always update the flow control enable bits;
+		 * these will only be effective if flow control AN
+		 * (MVPP2_GMAC_FLOW_CTRL_AUTONEG) is disabled.
+		 */
+		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
+		val &= ~(MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN);
+		if (tx_pause)
+			val |= MVPP22_CTRL4_TX_FC_EN;
+		if (rx_pause)
+			val |= MVPP22_CTRL4_RX_FC_EN;
+		writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
 	}
 
 	mvpp2_port_enable(port);
-- 
2.20.1

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Andrew Lunn]

On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> The following series changes the phylink interface to allow us to
> better support split MAC / MAC PCS setups.  The fundamental change
> required for this turns out to be quite simple.

Hi Russell

Do you have a branch i can pull and test?

Thanks
	Andrew

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Andrew Lunn]

On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> The following series changes the phylink interface to allow us to
> better support split MAC / MAC PCS setups.  The fundamental change
> required for this turns out to be quite simple.

Hi Russell

Do you have a branch i can pull and test?

Thanks
	Andrew

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Andrew Lunn]

On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> The following series changes the phylink interface to allow us to
> better support split MAC / MAC PCS setups.  The fundamental change
> required for this turns out to be quite simple.

Hi Russell

Do you have a branch i can pull and test?

Thanks
	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Matthew Wilcox]

On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> +   Please see :c:func:`mac_link_up` for more information on this.

FYI, Jon recently added the ability to specify functions as

+   Please see mac_link_up() for more information on this.

and it's now the preferred way to do this.  Nothing that should stand in
the way of this patch-set, of course.

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Matthew Wilcox]

On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> +   Please see :c:func:`mac_link_up` for more information on this.

FYI, Jon recently added the ability to specify functions as

+   Please see mac_link_up() for more information on this.

and it's now the preferred way to do this.  Nothing that should stand in
the way of this patch-set, of course.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Matthew Wilcox]

On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> +   Please see :c:func:`mac_link_up` for more information on this.

FYI, Jon recently added the ability to specify functions as

+   Please see mac_link_up() for more information on this.

and it's now the preferred way to do this.  Nothing that should stand in
the way of this patch-set, of course.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 10:03:59AM -0800, Matthew Wilcox wrote:
> On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> > +   Please see :c:func:`mac_link_up` for more information on this.
> 
> FYI, Jon recently added the ability to specify functions as
> 
> +   Please see mac_link_up() for more information on this.
> 
> and it's now the preferred way to do this.  Nothing that should stand in
> the way of this patch-set, of course.

Thanks for letting me know - it sounds like the subject of a future
patch to convert all instances.  In the mean time, I suggest keeping
to the current style in the file for consistency...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 10:03:59AM -0800, Matthew Wilcox wrote:
> On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> > +   Please see :c:func:`mac_link_up` for more information on this.
> 
> FYI, Jon recently added the ability to specify functions as
> 
> +   Please see mac_link_up() for more information on this.
> 
> and it's now the preferred way to do this.  Nothing that should stand in
> the way of this patch-set, of course.

Thanks for letting me know - it sounds like the subject of a future
patch to convert all instances.  In the mean time, I suggest keeping
to the current style in the file for consistency...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 10:03:59AM -0800, Matthew Wilcox wrote:
> On Mon, Feb 17, 2020 at 05:23:54PM +0000, Russell King wrote:
> > +   Please see :c:func:`mac_link_up` for more information on this.
> 
> FYI, Jon recently added the ability to specify functions as
> 
> +   Please see mac_link_up() for more information on this.
> 
> and it's now the preferred way to do this.  Nothing that should stand in
> the way of this patch-set, of course.

Thanks for letting me know - it sounds like the subject of a future
patch to convert all instances.  In the mean time, I suggest keeping
to the current style in the file for consistency...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > Hi,
> > 
> > The following series changes the phylink interface to allow us to
> > better support split MAC / MAC PCS setups.  The fundamental change
> > required for this turns out to be quite simple.
> 
> Hi Russell
> 
> Do you have a branch i can pull and test?

Nothing beyond the branches I've mentioned in the previous heads-up as
yet, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > Hi,
> > 
> > The following series changes the phylink interface to allow us to
> > better support split MAC / MAC PCS setups.  The fundamental change
> > required for this turns out to be quite simple.
> 
> Hi Russell
> 
> Do you have a branch i can pull and test?

Nothing beyond the branches I've mentioned in the previous heads-up as
yet, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > Hi,
> > 
> > The following series changes the phylink interface to allow us to
> > better support split MAC / MAC PCS setups.  The fundamental change
> > required for this turns out to be quite simple.
> 
> Hi Russell
> 
> Do you have a branch i can pull and test?

Nothing beyond the branches I've mentioned in the previous heads-up as
yet, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Florian Fainelli]

On 2/17/2020 9:23 AM, Russell King wrote:
> Propagate the resolved link parameters via the mac_link_up() call for
> MACs that do not automatically track their PCS state.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

[snip]


> -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> -			     phy_interface_t interface, struct phy_device *phy)
> +static void macb_mac_link_up(struct phylink_config *config,
> +			     struct phy_device *phy,
> +			     unsigned int mode, phy_interface_t interface,
> +			     int speed, int duplex,
> +			     bool tx_pause, bool rx_pause)

I have not been able to find an answer so I will ask this question, why
not pass a const struct phylink_link_state reference here instead of
splitting those link settings as individual function parameters? Or
maybe introduce a phylink_link_settings comprised of all of those 4
settings and embed it within phylink_link_state as well?

You would obviously need to squash patch #1 and #2 past this submission
stage to avoid bisection build failures.
-- 
Florian

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Florian Fainelli]

On 2/17/2020 9:23 AM, Russell King wrote:
> Propagate the resolved link parameters via the mac_link_up() call for
> MACs that do not automatically track their PCS state.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

[snip]


> -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> -			     phy_interface_t interface, struct phy_device *phy)
> +static void macb_mac_link_up(struct phylink_config *config,
> +			     struct phy_device *phy,
> +			     unsigned int mode, phy_interface_t interface,
> +			     int speed, int duplex,
> +			     bool tx_pause, bool rx_pause)

I have not been able to find an answer so I will ask this question, why
not pass a const struct phylink_link_state reference here instead of
splitting those link settings as individual function parameters? Or
maybe introduce a phylink_link_settings comprised of all of those 4
settings and embed it within phylink_link_state as well?

You would obviously need to squash patch #1 and #2 past this submission
stage to avoid bisection build failures.
-- 
Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Florian Fainelli]

On 2/17/2020 9:23 AM, Russell King wrote:
> Propagate the resolved link parameters via the mac_link_up() call for
> MACs that do not automatically track their PCS state.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

[snip]


> -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> -			     phy_interface_t interface, struct phy_device *phy)
> +static void macb_mac_link_up(struct phylink_config *config,
> +			     struct phy_device *phy,
> +			     unsigned int mode, phy_interface_t interface,
> +			     int speed, int duplex,
> +			     bool tx_pause, bool rx_pause)

I have not been able to find an answer so I will ask this question, why
not pass a const struct phylink_link_state reference here instead of
splitting those link settings as individual function parameters? Or
maybe introduce a phylink_link_settings comprised of all of those 4
settings and embed it within phylink_link_state as well?

You would obviously need to squash patch #1 and #2 past this submission
stage to avoid bisection build failures.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 01:54:19PM -0800, Florian Fainelli wrote:
> 
> 
> On 2/17/2020 9:23 AM, Russell King wrote:
> > Propagate the resolved link parameters via the mac_link_up() call for
> > MACs that do not automatically track their PCS state.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> [snip]
> 
> 
> > -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> > -			     phy_interface_t interface, struct phy_device *phy)
> > +static void macb_mac_link_up(struct phylink_config *config,
> > +			     struct phy_device *phy,
> > +			     unsigned int mode, phy_interface_t interface,
> > +			     int speed, int duplex,
> > +			     bool tx_pause, bool rx_pause)
> 
> I have not been able to find an answer so I will ask this question, why
> not pass a const struct phylink_link_state reference here instead of
> splitting those link settings as individual function parameters? Or
> maybe introduce a phylink_link_settings comprised of all of those 4
> settings and embed it within phylink_link_state as well?

History of mac_config() has shown that passing something like
struct phylink_link_state results in stuff that should not be used
being used inspite of documentation saying otherwise.  Passing
just the appropriate state ensures that stuff which should not
be used can't be got at.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 01:54:19PM -0800, Florian Fainelli wrote:
> 
> 
> On 2/17/2020 9:23 AM, Russell King wrote:
> > Propagate the resolved link parameters via the mac_link_up() call for
> > MACs that do not automatically track their PCS state.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> [snip]
> 
> 
> > -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> > -			     phy_interface_t interface, struct phy_device *phy)
> > +static void macb_mac_link_up(struct phylink_config *config,
> > +			     struct phy_device *phy,
> > +			     unsigned int mode, phy_interface_t interface,
> > +			     int speed, int duplex,
> > +			     bool tx_pause, bool rx_pause)
> 
> I have not been able to find an answer so I will ask this question, why
> not pass a const struct phylink_link_state reference here instead of
> splitting those link settings as individual function parameters? Or
> maybe introduce a phylink_link_settings comprised of all of those 4
> settings and embed it within phylink_link_state as well?

History of mac_config() has shown that passing something like
struct phylink_link_state results in stuff that should not be used
being used inspite of documentation saying otherwise.  Passing
just the appropriate state ensures that stuff which should not
be used can't be got at.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 1/8] net: phylink: propagate resolved link config via mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 01:54:19PM -0800, Florian Fainelli wrote:
> 
> 
> On 2/17/2020 9:23 AM, Russell King wrote:
> > Propagate the resolved link parameters via the mac_link_up() call for
> > MACs that do not automatically track their PCS state.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> [snip]
> 
> 
> > -static void macb_mac_link_up(struct phylink_config *config, unsigned int mode,
> > -			     phy_interface_t interface, struct phy_device *phy)
> > +static void macb_mac_link_up(struct phylink_config *config,
> > +			     struct phy_device *phy,
> > +			     unsigned int mode, phy_interface_t interface,
> > +			     int speed, int duplex,
> > +			     bool tx_pause, bool rx_pause)
> 
> I have not been able to find an answer so I will ask this question, why
> not pass a const struct phylink_link_state reference here instead of
> splitting those link settings as individual function parameters? Or
> maybe introduce a phylink_link_settings comprised of all of those 4
> settings and embed it within phylink_link_state as well?

History of mac_config() has shown that passing something like
struct phylink_link_state results in stuff that should not be used
being used inspite of documentation saying otherwise.  Passing
just the appropriate state ensures that stuff which should not
be used can't be got at.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:51:31PM +0000, Russell King - ARM Linux admin wrote:
> On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> > On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > > Hi,
> > > 
> > > The following series changes the phylink interface to allow us to
> > > better support split MAC / MAC PCS setups.  The fundamental change
> > > required for this turns out to be quite simple.
> > 
> > Hi Russell
> > 
> > Do you have a branch i can pull and test?
> 
> Nothing beyond the branches I've mentioned in the previous heads-up as
> yet, sorry.

In any case, for any particular network driver, there are three patches
maximum that you need - the first, and the one or two patches specific
to the network driver, depending whether it's a DSA driver or not. You
don't need all 8 patches to test this series. All can be applied on top
of yesterday's net-next, specifically

92df9f8a745e ("Merge branch 'mvneta-xdp-ethtool-stats'")

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:51:31PM +0000, Russell King - ARM Linux admin wrote:
> On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> > On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > > Hi,
> > > 
> > > The following series changes the phylink interface to allow us to
> > > better support split MAC / MAC PCS setups.  The fundamental change
> > > required for this turns out to be quite simple.
> > 
> > Hi Russell
> > 
> > Do you have a branch i can pull and test?
> 
> Nothing beyond the branches I've mentioned in the previous heads-up as
> yet, sorry.

In any case, for any particular network driver, there are three patches
maximum that you need - the first, and the one or two patches specific
to the network driver, depending whether it's a DSA driver or not. You
don't need all 8 patches to test this series. All can be applied on top
of yesterday's net-next, specifically

92df9f8a745e ("Merge branch 'mvneta-xdp-ethtool-stats'")

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Mon, Feb 17, 2020 at 06:51:31PM +0000, Russell King - ARM Linux admin wrote:
> On Mon, Feb 17, 2020 at 06:33:24PM +0100, Andrew Lunn wrote:
> > On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> > > Hi,
> > > 
> > > The following series changes the phylink interface to allow us to
> > > better support split MAC / MAC PCS setups.  The fundamental change
> > > required for this turns out to be quite simple.
> > 
> > Hi Russell
> > 
> > Do you have a branch i can pull and test?
> 
> Nothing beyond the branches I've mentioned in the previous heads-up as
> yet, sorry.

In any case, for any particular network driver, there are three patches
maximum that you need - the first, and the one or two patches specific
to the network driver, depending whether it's a DSA driver or not. You
don't need all 8 patches to test this series. All can be applied on top
of yesterday's net-next, specifically

92df9f8a745e ("Merge branch 'mvneta-xdp-ethtool-stats'")

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

It would really help if MAINTAINERS were updated with the correct
information for this driver:

DPAA2 ETHERNET DRIVER
M:      Ioana Radulescu <ruxandra.radulescu@nxp.com>

This address bounces.  Given what I find in the git history, is the
correct person is now:

Ioana Ciornei <ioana.ciornei@nxp.com>

Please submit a patch updating MAINTAINERS.  Thanks.

On Mon, Feb 17, 2020 at 05:24:16PM +0000, Russell King wrote:
> Convert the DPAA2 ethernet driver to use the finalised link parameters
> in mac_link_up() rather than the parameters in mac_config(), which are
> more suited to the needs of the DPAA2 MC firmware than those available
> via mac_config().
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54 +++++++++++--------
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |  1 +
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index 3a75c5b58f95..3ee236c5fc37 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> @@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
>  	struct dpmac_link_state *dpmac_state = &mac->state;
>  	int err;
>  
> -	if (state->speed != SPEED_UNKNOWN)
> -		dpmac_state->rate = state->speed;
> -
> -	if (state->duplex != DUPLEX_UNKNOWN) {
> -		if (!state->duplex)
> -			dpmac_state->options |= DPMAC_LINK_OPT_HALF_DUPLEX;
> -		else
> -			dpmac_state->options &= ~DPMAC_LINK_OPT_HALF_DUPLEX;
> -	}
> -
>  	if (state->an_enabled)
>  		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
>  	else
>  		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
>  
> -	if (state->pause & MLO_PAUSE_RX)
> -		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> -	else
> -		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> -
> -	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause & MLO_PAUSE_TX))
> -		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> -	else
> -		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> -
>  	err = dpmac_set_link_state(mac->mc_io, 0,
>  				   mac->mc_dev->mc_handle, dpmac_state);
>  	if (err)
> -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
> +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() = %d\n",
> +			   __func__, err);
>  }
>  
>  static void dpaa2_mac_link_up(struct phylink_config *config,
> @@ -165,10 +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config *config,
>  	int err;
>  
>  	dpmac_state->up = 1;
> +
> +	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
> +		/* If the DPMAC is configured for PHY mode, we need
> +		 * to pass the link parameters to the MC firmware.
> +		 */
> +		dpmac_state->rate = speed;
> +
> +		if (duplex == DUPLEX_HALF)
> +			dpmac_state->options |= DPMAC_LINK_OPT_HALF_DUPLEX;
> +		else if (duplex == DUPLEX_FULL)
> +			dpmac_state->options &= ~DPMAC_LINK_OPT_HALF_DUPLEX;
> +
> +		/* This is lossy; the firmware really should take the pause
> +		 * enablement status rather than pause/asym pause status.
> +		 */
> +		if (rx_pause)
> +			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> +		else
> +			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> +
> +		if (rx_pause ^ tx_pause)
> +			dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> +		else
> +			dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> +	}
> +
>  	err = dpmac_set_link_state(mac->mc_io, 0,
>  				   mac->mc_dev->mc_handle, dpmac_state);
>  	if (err)
> -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n", err);
> +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() = %d\n",
> +			   __func__, err);
>  }
>  
>  static void dpaa2_mac_link_down(struct phylink_config *config,
> @@ -241,6 +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
>  		goto err_close_dpmac;
>  	}
>  
> +	mac->if_link_type = attr.link_type;
> +
>  	dpmac_node = dpaa2_mac_get_node(attr.id);
>  	if (!dpmac_node) {
>  		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> index 4da8079b9155..2130d9c7d40e 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> @@ -20,6 +20,7 @@ struct dpaa2_mac {
>  	struct phylink_config phylink_config;
>  	struct phylink *phylink;
>  	phy_interface_t if_mode;
> +	enum dpmac_link_type if_link_type;
>  };
>  
>  bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

RE: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()

[Ioana Ciornei]

> Subject: Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()
> 
> It would really help if MAINTAINERS were updated with the correct information
> for this driver:
> 
> DPAA2 ETHERNET DRIVER
> M:      Ioana Radulescu <ruxandra.radulescu@nxp.com>
> 
> This address bounces.  Given what I find in the git history, is the correct person is
> now:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> Please submit a patch updating MAINTAINERS.  Thanks.

Sure thing.  I'll update the MAINTAINERS file and list myself instead of Ioana Radulescu.

> 
> On Mon, Feb 17, 2020 at 05:24:16PM +0000, Russell King wrote:
> > Convert the DPAA2 ethernet driver to use the finalised link parameters
> > in mac_link_up() rather than the parameters in mac_config(), which are
> > more suited to the needs of the DPAA2 MC firmware than those available
> > via mac_config().
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54
> > +++++++++++--------  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |
> > 1 +
> >  2 files changed, 33 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > index 3a75c5b58f95..3ee236c5fc37 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > @@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config
> *config, unsigned int mode,
> >  	struct dpmac_link_state *dpmac_state = &mac->state;
> >  	int err;
> >
> > -	if (state->speed != SPEED_UNKNOWN)
> > -		dpmac_state->rate = state->speed;
> > -
> > -	if (state->duplex != DUPLEX_UNKNOWN) {
> > -		if (!state->duplex)
> > -			dpmac_state->options |=
> DPMAC_LINK_OPT_HALF_DUPLEX;
> > -		else
> > -			dpmac_state->options &=
> ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > -	}
> > -
> >  	if (state->an_enabled)
> >  		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
> >  	else
> >  		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
> >
> > -	if (state->pause & MLO_PAUSE_RX)
> > -		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > -	else
> > -		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > -
> > -	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause &
> MLO_PAUSE_TX))
> > -		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> > -	else
> > -		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > -
> >  	err = dpmac_set_link_state(mac->mc_io, 0,
> >  				   mac->mc_dev->mc_handle, dpmac_state);
> >  	if (err)
> > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> err);
> > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> %d\n",
> > +			   __func__, err);
> >  }
> >
> >  static void dpaa2_mac_link_up(struct phylink_config *config, @@
> > -165,10 +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config
> *config,
> >  	int err;
> >
> >  	dpmac_state->up = 1;
> > +
> > +	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
> > +		/* If the DPMAC is configured for PHY mode, we need
> > +		 * to pass the link parameters to the MC firmware.
> > +		 */
> > +		dpmac_state->rate = speed;
> > +
> > +		if (duplex == DUPLEX_HALF)
> > +			dpmac_state->options |=
> DPMAC_LINK_OPT_HALF_DUPLEX;
> > +		else if (duplex == DUPLEX_FULL)
> > +			dpmac_state->options &=
> ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > +
> > +		/* This is lossy; the firmware really should take the pause
> > +		 * enablement status rather than pause/asym pause status.
> > +		 */
> > +		if (rx_pause)
> > +			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > +		else
> > +			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > +
> > +		if (rx_pause ^ tx_pause)
> > +			dpmac_state->options |=
> DPMAC_LINK_OPT_ASYM_PAUSE;
> > +		else
> > +			dpmac_state->options &=
> ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > +	}
> > +
> >  	err = dpmac_set_link_state(mac->mc_io, 0,
> >  				   mac->mc_dev->mc_handle, dpmac_state);
> >  	if (err)
> > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> err);
> > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> %d\n",
> > +			   __func__, err);
> >  }
> >
> >  static void dpaa2_mac_link_down(struct phylink_config *config, @@
> > -241,6 +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> >  		goto err_close_dpmac;
> >  	}
> >
> > +	mac->if_link_type = attr.link_type;
> > +
> >  	dpmac_node = dpaa2_mac_get_node(attr.id);
> >  	if (!dpmac_node) {
> >  		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
> diff
> > --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > index 4da8079b9155..2130d9c7d40e 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > @@ -20,6 +20,7 @@ struct dpaa2_mac {
> >  	struct phylink_config phylink_config;
> >  	struct phylink *phylink;
> >  	phy_interface_t if_mode;
> > +	enum dpmac_link_type if_link_type;
> >  };
> >
> >  bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> > --
> > 2.20.1
> >
> >
> 
> --
> RMK's Patch system:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.cior
> nei%40nxp.com%7C09d0167191914135433808d7b45e15fd%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C637176188497544105&amp;sdata=t0%2B
> OzkoqRM180UHGBrW6FYAvHsIelx4CaP4oC3QcP1k%3D&amp;reserved=0
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Alexandre Belloni]

Hi,

On 17/02/2020 17:24:21+0000, Russell King wrote:
> Convert the macb ethernet driver to use the finalised link
> parameters in mac_link_up() rather than the parameters in mac_config().
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/cadence/macb.h      |  1 -
>  drivers/net/ethernet/cadence/macb_main.c | 46 ++++++++++++++----------
>  2 files changed, 27 insertions(+), 20 deletions(-)
> 

I did test the series after rebasing on top of the at91rm9200 fix.

Here is what I tested:

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index a3f0f27fc79a..ab827fb4b6b9 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1200,7 +1200,6 @@ struct macb {
 	unsigned int		dma_burst_length;
 
 	phy_interface_t		phy_interface;
-	int			speed;
 
 	/* AT91RM9200 transmit */
 	struct sk_buff *skb;			/* holds skb until xmit interrupt completes */
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7ab0bef5e1bd..3a7c26b08607 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -571,37 +571,20 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
 
 	old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
 
-	/* Clear all the bits we might set later */
-	ctrl &= ~(MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE));
-
 	if (bp->caps & MACB_CAPS_MACB_IS_EMAC) {
 		if (state->interface == PHY_INTERFACE_MODE_RMII)
 			ctrl |= MACB_BIT(RM9200_RMII);
 	} else {
-		ctrl &= ~(GEM_BIT(GBE) | GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
-
-		/* We do not support MLO_PAUSE_RX yet */
-		if (state->pause & MLO_PAUSE_TX)
-			ctrl |= MACB_BIT(PAE);
+		ctrl &= ~(GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
 
 		if (state->interface == PHY_INTERFACE_MODE_SGMII)
 			ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
 	}
 
-	if (state->speed == SPEED_1000)
-		ctrl |= GEM_BIT(GBE);
-	else if (state->speed == SPEED_100)
-		ctrl |= MACB_BIT(SPD);
-
-	if (state->duplex)
-		ctrl |= MACB_BIT(FD);
-
 	/* Apply the new configuration, if any */
 	if (old_ctrl ^ ctrl)
 		macb_or_gem_writel(bp, NCFGR, ctrl);
 
-	bp->speed = state->speed;
-
 	spin_unlock_irqrestore(&bp->lock, flags);
 }
 
@@ -635,10 +618,33 @@ static void macb_mac_link_up(struct phylink_config *config,
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(ndev);
 	struct macb_queue *queue;
+	unsigned long flags;
 	unsigned int q;
+	u32 ctrl;
+
+	spin_lock_irqsave(&bp->lock, flags);
+
+	ctrl = macb_or_gem_readl(bp, NCFGR);
+
+	ctrl &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
+
+	if (speed == SPEED_100)
+		ctrl |= MACB_BIT(SPD);
+
+	if (duplex)
+		ctrl |= MACB_BIT(FD);
 
 	if (!(bp->caps & MACB_CAPS_MACB_IS_EMAC)) {
-		macb_set_tx_clk(bp->tx_clk, bp->speed, ndev);
+		ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(PAE));
+
+		if (speed == SPEED_1000)
+			ctrl |= GEM_BIT(GBE);
+
+		/* We do not support MLO_PAUSE_RX yet */
+		if (tx_pause)
+			ctrl |= MACB_BIT(PAE);
+
+		macb_set_tx_clk(bp->tx_clk, speed, ndev);
 
 		/* Initialize rings & buffers as clearing MACB_BIT(TE) in link down
 		 * cleared the pipeline and control registers.
@@ -651,6 +657,10 @@ static void macb_mac_link_up(struct phylink_config *config,
 				     bp->rx_intr_mask | MACB_TX_INT_FLAGS | MACB_BIT(HRESP));
 	}
 
+	macb_or_gem_writel(bp, NCFGR, ctrl);
+
+	spin_unlock_irqrestore(&bp->lock, flags);
+
 	/* Enable Rx and Tx */
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
 
@@ -4432,8 +4442,6 @@ static int macb_probe(struct platform_device *pdev)
 	else
 		bp->phy_interface = interface;
 
-	bp->speed = SPEED_UNKNOWN;
-
 	/* IP specific init */
 	err = init(pdev);
 	if (err)


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Wed, Feb 19, 2020 at 03:30:36PM +0100, Alexandre Belloni wrote:
> Hi,
> 
> On 17/02/2020 17:24:21+0000, Russell King wrote:
> > Convert the macb ethernet driver to use the finalised link
> > parameters in mac_link_up() rather than the parameters in mac_config().
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/cadence/macb.h      |  1 -
> >  drivers/net/ethernet/cadence/macb_main.c | 46 ++++++++++++++----------
> >  2 files changed, 27 insertions(+), 20 deletions(-)
> > 
> 
> I did test the series after rebasing on top of the at91rm9200 fix.
> 
> Here is what I tested:
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index a3f0f27fc79a..ab827fb4b6b9 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1200,7 +1200,6 @@ struct macb {
>  	unsigned int		dma_burst_length;
>  
>  	phy_interface_t		phy_interface;
> -	int			speed;
>  
>  	/* AT91RM9200 transmit */
>  	struct sk_buff *skb;			/* holds skb until xmit interrupt completes */
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 7ab0bef5e1bd..3a7c26b08607 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -571,37 +571,20 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
>  
>  	old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
>  
> -	/* Clear all the bits we might set later */
> -	ctrl &= ~(MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE));
> -
>  	if (bp->caps & MACB_CAPS_MACB_IS_EMAC) {
>  		if (state->interface == PHY_INTERFACE_MODE_RMII)
>  			ctrl |= MACB_BIT(RM9200_RMII);
>  	} else {
> -		ctrl &= ~(GEM_BIT(GBE) | GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
> -
> -		/* We do not support MLO_PAUSE_RX yet */
> -		if (state->pause & MLO_PAUSE_TX)
> -			ctrl |= MACB_BIT(PAE);
> +		ctrl &= ~(GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
>  
>  		if (state->interface == PHY_INTERFACE_MODE_SGMII)
>  			ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
>  	}
>  
> -	if (state->speed == SPEED_1000)
> -		ctrl |= GEM_BIT(GBE);
> -	else if (state->speed == SPEED_100)
> -		ctrl |= MACB_BIT(SPD);
> -
> -	if (state->duplex)
> -		ctrl |= MACB_BIT(FD);
> -
>  	/* Apply the new configuration, if any */
>  	if (old_ctrl ^ ctrl)
>  		macb_or_gem_writel(bp, NCFGR, ctrl);
>  
> -	bp->speed = state->speed;
> -
>  	spin_unlock_irqrestore(&bp->lock, flags);
>  }
>  
> @@ -635,10 +618,33 @@ static void macb_mac_link_up(struct phylink_config *config,
>  	struct net_device *ndev = to_net_dev(config->dev);
>  	struct macb *bp = netdev_priv(ndev);
>  	struct macb_queue *queue;
> +	unsigned long flags;
>  	unsigned int q;
> +	u32 ctrl;
> +
> +	spin_lock_irqsave(&bp->lock, flags);
> +
> +	ctrl = macb_or_gem_readl(bp, NCFGR);
> +
> +	ctrl &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
> +
> +	if (speed == SPEED_100)
> +		ctrl |= MACB_BIT(SPD);
> +
> +	if (duplex)
> +		ctrl |= MACB_BIT(FD);
>  
>  	if (!(bp->caps & MACB_CAPS_MACB_IS_EMAC)) {
> -		macb_set_tx_clk(bp->tx_clk, bp->speed, ndev);
> +		ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(PAE));
> +
> +		if (speed == SPEED_1000)
> +			ctrl |= GEM_BIT(GBE);
> +
> +		/* We do not support MLO_PAUSE_RX yet */
> +		if (tx_pause)
> +			ctrl |= MACB_BIT(PAE);
> +
> +		macb_set_tx_clk(bp->tx_clk, speed, ndev);
>  
>  		/* Initialize rings & buffers as clearing MACB_BIT(TE) in link down
>  		 * cleared the pipeline and control registers.
> @@ -651,6 +657,10 @@ static void macb_mac_link_up(struct phylink_config *config,
>  				     bp->rx_intr_mask | MACB_TX_INT_FLAGS | MACB_BIT(HRESP));
>  	}
>  
> +	macb_or_gem_writel(bp, NCFGR, ctrl);
> +
> +	spin_unlock_irqrestore(&bp->lock, flags);
> +
>  	/* Enable Rx and Tx */
>  	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
>  
> @@ -4432,8 +4442,6 @@ static int macb_probe(struct platform_device *pdev)
>  	else
>  		bp->phy_interface = interface;
>  
> -	bp->speed = SPEED_UNKNOWN;
> -
>  	/* IP specific init */
>  	err = init(pdev);
>  	if (err)
> 
> 

Thanks, that looks reasonable to me. I'll replace my patch with this
one if it's appropriate for net-next when I send this series for
merging.  However, I see most affected network driver maintainers
haven't responded yet, which is rather disappointing.  So, thanks
for taking the time to look at this.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Tue, Feb 18, 2020 at 10:42:41AM +0000, Ioana Ciornei wrote:
> > Subject: Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()
> > 
> > It would really help if MAINTAINERS were updated with the correct information
> > for this driver:
> > 
> > DPAA2 ETHERNET DRIVER
> > M:      Ioana Radulescu <ruxandra.radulescu@nxp.com>
> > 
> > This address bounces.  Given what I find in the git history, is the correct person is
> > now:
> > 
> > Ioana Ciornei <ioana.ciornei@nxp.com>
> > 
> > Please submit a patch updating MAINTAINERS.  Thanks.
> 
> Sure thing.  I'll update the MAINTAINERS file and list myself instead of Ioana Radulescu.

Any comments on the patch itself?

> > On Mon, Feb 17, 2020 at 05:24:16PM +0000, Russell King wrote:
> > > Convert the DPAA2 ethernet driver to use the finalised link parameters
> > > in mac_link_up() rather than the parameters in mac_config(), which are
> > > more suited to the needs of the DPAA2 MC firmware than those available
> > > via mac_config().
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54
> > > +++++++++++--------  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |
> > > 1 +
> > >  2 files changed, 33 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > index 3a75c5b58f95..3ee236c5fc37 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > @@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config
> > *config, unsigned int mode,
> > >  	struct dpmac_link_state *dpmac_state = &mac->state;
> > >  	int err;
> > >
> > > -	if (state->speed != SPEED_UNKNOWN)
> > > -		dpmac_state->rate = state->speed;
> > > -
> > > -	if (state->duplex != DUPLEX_UNKNOWN) {
> > > -		if (!state->duplex)
> > > -			dpmac_state->options |=
> > DPMAC_LINK_OPT_HALF_DUPLEX;
> > > -		else
> > > -			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > > -	}
> > > -
> > >  	if (state->an_enabled)
> > >  		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
> > >  	else
> > >  		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
> > >
> > > -	if (state->pause & MLO_PAUSE_RX)
> > > -		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > > -	else
> > > -		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > > -
> > > -	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause &
> > MLO_PAUSE_TX))
> > > -		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> > > -	else
> > > -		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > > -
> > >  	err = dpmac_set_link_state(mac->mc_io, 0,
> > >  				   mac->mc_dev->mc_handle, dpmac_state);
> > >  	if (err)
> > > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> > err);
> > > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> > %d\n",
> > > +			   __func__, err);
> > >  }
> > >
> > >  static void dpaa2_mac_link_up(struct phylink_config *config, @@
> > > -165,10 +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config
> > *config,
> > >  	int err;
> > >
> > >  	dpmac_state->up = 1;
> > > +
> > > +	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
> > > +		/* If the DPMAC is configured for PHY mode, we need
> > > +		 * to pass the link parameters to the MC firmware.
> > > +		 */
> > > +		dpmac_state->rate = speed;
> > > +
> > > +		if (duplex == DUPLEX_HALF)
> > > +			dpmac_state->options |=
> > DPMAC_LINK_OPT_HALF_DUPLEX;
> > > +		else if (duplex == DUPLEX_FULL)
> > > +			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > > +
> > > +		/* This is lossy; the firmware really should take the pause
> > > +		 * enablement status rather than pause/asym pause status.
> > > +		 */
> > > +		if (rx_pause)
> > > +			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > > +		else
> > > +			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > > +
> > > +		if (rx_pause ^ tx_pause)
> > > +			dpmac_state->options |=
> > DPMAC_LINK_OPT_ASYM_PAUSE;
> > > +		else
> > > +			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > > +	}
> > > +
> > >  	err = dpmac_set_link_state(mac->mc_io, 0,
> > >  				   mac->mc_dev->mc_handle, dpmac_state);
> > >  	if (err)
> > > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> > err);
> > > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> > %d\n",
> > > +			   __func__, err);
> > >  }
> > >
> > >  static void dpaa2_mac_link_down(struct phylink_config *config, @@
> > > -241,6 +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> > >  		goto err_close_dpmac;
> > >  	}
> > >
> > > +	mac->if_link_type = attr.link_type;
> > > +
> > >  	dpmac_node = dpaa2_mac_get_node(attr.id);
> > >  	if (!dpmac_node) {
> > >  		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
> > diff
> > > --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > index 4da8079b9155..2130d9c7d40e 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > @@ -20,6 +20,7 @@ struct dpaa2_mac {
> > >  	struct phylink_config phylink_config;
> > >  	struct phylink *phylink;
> > >  	phy_interface_t if_mode;
> > > +	enum dpmac_link_type if_link_type;
> > >  };
> > >
> > >  bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> > > --
> > > 2.20.1
> > >
> > >
> > 
> > --
> > RMK's Patch system:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> > mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.cior
> > nei%40nxp.com%7C09d0167191914135433808d7b45e15fd%7C686ea1d3bc2b4
> > c6fa92cd99c5c301635%7C0%7C0%7C637176188497544105&amp;sdata=t0%2B
> > OzkoqRM180UHGBrW6FYAvHsIelx4CaP4oC3QcP1k%3D&amp;reserved=0
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

Hi Andre,

Are you able to give this (along with patch 1) some testing please?
The series can be grabbed from:

  https://patchwork.ozlabs.org/series/159037/mbox/

Strangely, google doesn't find it in the lore.kernel.org archives,
but does in spinics.net archives and patchwork...

On Mon, Feb 17, 2020 at 05:24:09PM +0000, Russell King wrote:
> Convert the Xilinx AXI ethernet driver to use the finalised link
> parameters in mac_link_up() rather than the parameters in mac_config().
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++++++++----------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 197740781157..c2f4c5ca2e80 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1440,6 +1440,22 @@ static void axienet_mac_an_restart(struct phylink_config *config)
>  
>  static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  			       const struct phylink_link_state *state)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_down(struct phylink_config *config,
> +				  unsigned int mode,
> +				  phy_interface_t interface)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_up(struct phylink_config *config,
> +				struct phy_device *phy,
> +				unsigned int mode, phy_interface_t interface,
> +				int speed, int duplex,
> +				bool tx_pause, bool rx_pause)
>  {
>  	struct net_device *ndev = to_net_dev(config->dev);
>  	struct axienet_local *lp = netdev_priv(ndev);
> @@ -1448,7 +1464,7 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET);
>  	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
>  
> -	switch (state->speed) {
> +	switch (speed) {
>  	case SPEED_1000:
>  		emmc_reg |= XAE_EMMC_LINKSPD_1000;
>  		break;
> @@ -1467,33 +1483,17 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg);
>  
>  	fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET);
> -	if (state->pause & MLO_PAUSE_TX)
> +	if (tx_pause)
>  		fcc_reg |= XAE_FCC_FCTX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCTX_MASK;
> -	if (state->pause & MLO_PAUSE_RX)
> +	if (rx_pause)
>  		fcc_reg |= XAE_FCC_FCRX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCRX_MASK;
>  	axienet_iow(lp, XAE_FCC_OFFSET, fcc_reg);
>  }
>  
> -static void axienet_mac_link_down(struct phylink_config *config,
> -				  unsigned int mode,
> -				  phy_interface_t interface)
> -{
> -	/* nothing meaningful to do */
> -}
> -
> -static void axienet_mac_link_up(struct phylink_config *config,
> -				struct phy_device *phy,
> -				unsigned int mode, phy_interface_t interface,
> -				int speed, int duplex,
> -				bool tx_pause, bool rx_pause)
> -{
> -	/* nothing meaningful to do */
> -}
> -
>  static const struct phylink_mac_ops axienet_phylink_ops = {
>  	.validate = axienet_validate,
>  	.mac_pcs_get_state = axienet_mac_pcs_get_state,
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

Hi Andre,

Are you able to give this (along with patch 1) some testing please?
The series can be grabbed from:

  https://patchwork.ozlabs.org/series/159037/mbox/

Strangely, google doesn't find it in the lore.kernel.org archives,
but does in spinics.net archives and patchwork...

On Mon, Feb 17, 2020 at 05:24:09PM +0000, Russell King wrote:
> Convert the Xilinx AXI ethernet driver to use the finalised link
> parameters in mac_link_up() rather than the parameters in mac_config().
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++++++++----------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 197740781157..c2f4c5ca2e80 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1440,6 +1440,22 @@ static void axienet_mac_an_restart(struct phylink_config *config)
>  
>  static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  			       const struct phylink_link_state *state)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_down(struct phylink_config *config,
> +				  unsigned int mode,
> +				  phy_interface_t interface)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_up(struct phylink_config *config,
> +				struct phy_device *phy,
> +				unsigned int mode, phy_interface_t interface,
> +				int speed, int duplex,
> +				bool tx_pause, bool rx_pause)
>  {
>  	struct net_device *ndev = to_net_dev(config->dev);
>  	struct axienet_local *lp = netdev_priv(ndev);
> @@ -1448,7 +1464,7 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET);
>  	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
>  
> -	switch (state->speed) {
> +	switch (speed) {
>  	case SPEED_1000:
>  		emmc_reg |= XAE_EMMC_LINKSPD_1000;
>  		break;
> @@ -1467,33 +1483,17 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg);
>  
>  	fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET);
> -	if (state->pause & MLO_PAUSE_TX)
> +	if (tx_pause)
>  		fcc_reg |= XAE_FCC_FCTX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCTX_MASK;
> -	if (state->pause & MLO_PAUSE_RX)
> +	if (rx_pause)
>  		fcc_reg |= XAE_FCC_FCRX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCRX_MASK;
>  	axienet_iow(lp, XAE_FCC_OFFSET, fcc_reg);
>  }
>  
> -static void axienet_mac_link_down(struct phylink_config *config,
> -				  unsigned int mode,
> -				  phy_interface_t interface)
> -{
> -	/* nothing meaningful to do */
> -}
> -
> -static void axienet_mac_link_up(struct phylink_config *config,
> -				struct phy_device *phy,
> -				unsigned int mode, phy_interface_t interface,
> -				int speed, int duplex,
> -				bool tx_pause, bool rx_pause)
> -{
> -	/* nothing meaningful to do */
> -}
> -
>  static const struct phylink_mac_ops axienet_phylink_ops = {
>  	.validate = axienet_validate,
>  	.mac_pcs_get_state = axienet_mac_pcs_get_state,
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Andrew Lunn]

> Thanks, that looks reasonable to me. I'll replace my patch with this
> one if it's appropriate for net-next when I send this series for
> merging.  However, I see most affected network driver maintainers
> haven't responded yet, which is rather disappointing.  So, thanks
> for taking the time to look at this.

Hi Russell

I suspect most maintainers are lazy. Give them a branch to pull, and
they might be more likely to test.

     Andrew

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Thu, Feb 20, 2020 at 01:38:53PM +0100, Andrew Lunn wrote:
> > Thanks, that looks reasonable to me. I'll replace my patch with this
> > one if it's appropriate for net-next when I send this series for
> > merging.  However, I see most affected network driver maintainers
> > haven't responded yet, which is rather disappointing.  So, thanks
> > for taking the time to look at this.
> 
> Hi Russell
> 
> I suspect most maintainers are lazy. Give them a branch to pull, and
> they might be more likely to test.

While that seems like a trivial solution, it'll take me a while to
set that up. I've no desire to upload a lot of data over my
internet connection during peak hours (and see the upload rate in
my signature below.)

Not everywhere has "fast" internet.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Thu, Feb 20, 2020 at 01:38:53PM +0100, Andrew Lunn wrote:
> > Thanks, that looks reasonable to me. I'll replace my patch with this
> > one if it's appropriate for net-next when I send this series for
> > merging.  However, I see most affected network driver maintainers
> > haven't responded yet, which is rather disappointing.  So, thanks
> > for taking the time to look at this.
> 
> Hi Russell
> 
> I suspect most maintainers are lazy. Give them a branch to pull, and
> they might be more likely to test.

git://git.armlinux.org.uk/~rmk/linux-net-next.git

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Andre Przywara]

On Mon, 17 Feb 2020 17:24:09 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

Hi Russell,

> Convert the Xilinx AXI ethernet driver to use the finalised link
> parameters in mac_link_up() rather than the parameters in mac_config().

Many thanks for this series, a quite neat solution for the problems I saw!

I picked 1/8 and 4/8 on top of net-next/master as of today: c3e042f54107376 ("igmp: remove unused macro IGMP_Vx_UNSOLICITED_REPORT_INTERVAL") and it worked great on my FPGA board using SGMII (but no in-band negotiation over that link). I had the 64-bit DMA patches on top, but that doesn't affect this series.

Tested-by: Andre Przywara <andre.przywara@arm.com>

Is this heading for 5.7?

Cheers,
Andre.

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++++++++----------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 197740781157..c2f4c5ca2e80 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1440,6 +1440,22 @@ static void axienet_mac_an_restart(struct phylink_config *config)
>  
>  static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  			       const struct phylink_link_state *state)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_down(struct phylink_config *config,
> +				  unsigned int mode,
> +				  phy_interface_t interface)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_up(struct phylink_config *config,
> +				struct phy_device *phy,
> +				unsigned int mode, phy_interface_t interface,
> +				int speed, int duplex,
> +				bool tx_pause, bool rx_pause)
>  {
>  	struct net_device *ndev = to_net_dev(config->dev);
>  	struct axienet_local *lp = netdev_priv(ndev);
> @@ -1448,7 +1464,7 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET);
>  	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
>  
> -	switch (state->speed) {
> +	switch (speed) {
>  	case SPEED_1000:
>  		emmc_reg |= XAE_EMMC_LINKSPD_1000;
>  		break;
> @@ -1467,33 +1483,17 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg);
>  
>  	fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET);
> -	if (state->pause & MLO_PAUSE_TX)
> +	if (tx_pause)
>  		fcc_reg |= XAE_FCC_FCTX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCTX_MASK;
> -	if (state->pause & MLO_PAUSE_RX)
> +	if (rx_pause)
>  		fcc_reg |= XAE_FCC_FCRX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCRX_MASK;
>  	axienet_iow(lp, XAE_FCC_OFFSET, fcc_reg);
>  }
>  
> -static void axienet_mac_link_down(struct phylink_config *config,
> -				  unsigned int mode,
> -				  phy_interface_t interface)
> -{
> -	/* nothing meaningful to do */
> -}
> -
> -static void axienet_mac_link_up(struct phylink_config *config,
> -				struct phy_device *phy,
> -				unsigned int mode, phy_interface_t interface,
> -				int speed, int duplex,
> -				bool tx_pause, bool rx_pause)
> -{
> -	/* nothing meaningful to do */
> -}
> -
>  static const struct phylink_mac_ops axienet_phylink_ops = {
>  	.validate = axienet_validate,
>  	.mac_pcs_get_state = axienet_mac_pcs_get_state,

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Andre Przywara]

On Mon, 17 Feb 2020 17:24:09 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

Hi Russell,

> Convert the Xilinx AXI ethernet driver to use the finalised link
> parameters in mac_link_up() rather than the parameters in mac_config().

Many thanks for this series, a quite neat solution for the problems I saw!

I picked 1/8 and 4/8 on top of net-next/master as of today: c3e042f54107376 ("igmp: remove unused macro IGMP_Vx_UNSOLICITED_REPORT_INTERVAL") and it worked great on my FPGA board using SGMII (but no in-band negotiation over that link). I had the 64-bit DMA patches on top, but that doesn't affect this series.

Tested-by: Andre Przywara <andre.przywara@arm.com>

Is this heading for 5.7?

Cheers,
Andre.

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++++++++----------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 197740781157..c2f4c5ca2e80 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1440,6 +1440,22 @@ static void axienet_mac_an_restart(struct phylink_config *config)
>  
>  static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  			       const struct phylink_link_state *state)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_down(struct phylink_config *config,
> +				  unsigned int mode,
> +				  phy_interface_t interface)
> +{
> +	/* nothing meaningful to do */
> +}
> +
> +static void axienet_mac_link_up(struct phylink_config *config,
> +				struct phy_device *phy,
> +				unsigned int mode, phy_interface_t interface,
> +				int speed, int duplex,
> +				bool tx_pause, bool rx_pause)
>  {
>  	struct net_device *ndev = to_net_dev(config->dev);
>  	struct axienet_local *lp = netdev_priv(ndev);
> @@ -1448,7 +1464,7 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET);
>  	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
>  
> -	switch (state->speed) {
> +	switch (speed) {
>  	case SPEED_1000:
>  		emmc_reg |= XAE_EMMC_LINKSPD_1000;
>  		break;
> @@ -1467,33 +1483,17 @@ static void axienet_mac_config(struct phylink_config *config, unsigned int mode,
>  	axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg);
>  
>  	fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET);
> -	if (state->pause & MLO_PAUSE_TX)
> +	if (tx_pause)
>  		fcc_reg |= XAE_FCC_FCTX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCTX_MASK;
> -	if (state->pause & MLO_PAUSE_RX)
> +	if (rx_pause)
>  		fcc_reg |= XAE_FCC_FCRX_MASK;
>  	else
>  		fcc_reg &= ~XAE_FCC_FCRX_MASK;
>  	axienet_iow(lp, XAE_FCC_OFFSET, fcc_reg);
>  }
>  
> -static void axienet_mac_link_down(struct phylink_config *config,
> -				  unsigned int mode,
> -				  phy_interface_t interface)
> -{
> -	/* nothing meaningful to do */
> -}
> -
> -static void axienet_mac_link_up(struct phylink_config *config,
> -				struct phy_device *phy,
> -				unsigned int mode, phy_interface_t interface,
> -				int speed, int duplex,
> -				bool tx_pause, bool rx_pause)
> -{
> -	/* nothing meaningful to do */
> -}
> -
>  static const struct phylink_mac_ops axienet_phylink_ops = {
>  	.validate = axienet_validate,
>  	.mac_pcs_get_state = axienet_mac_pcs_get_state,


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 24, 2020 at 12:24:21PM +0000, Andre Przywara wrote:
> On Mon, 17 Feb 2020 17:24:09 +0000
> Russell King <rmk+kernel@armlinux.org.uk> wrote:
> 
> Hi Russell,
> 
> > Convert the Xilinx AXI ethernet driver to use the finalised link
> > parameters in mac_link_up() rather than the parameters in mac_config().
> 
> Many thanks for this series, a quite neat solution for the problems I saw!
> 
> I picked 1/8 and 4/8 on top of net-next/master as of today: c3e042f54107376 ("igmp: remove unused macro IGMP_Vx_UNSOLICITED_REPORT_INTERVAL") and it worked great on my FPGA board using SGMII (but no in-band negotiation over that link). I had the 64-bit DMA patches on top, but that doesn't affect this series.
> 
> Tested-by: Andre Przywara <andre.przywara@arm.com>

Great, thanks for testing!

> Is this heading for 5.7?

Yes, that is my hope.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 4/8] net: axienet: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Mon, Feb 24, 2020 at 12:24:21PM +0000, Andre Przywara wrote:
> On Mon, 17 Feb 2020 17:24:09 +0000
> Russell King <rmk+kernel@armlinux.org.uk> wrote:
> 
> Hi Russell,
> 
> > Convert the Xilinx AXI ethernet driver to use the finalised link
> > parameters in mac_link_up() rather than the parameters in mac_config().
> 
> Many thanks for this series, a quite neat solution for the problems I saw!
> 
> I picked 1/8 and 4/8 on top of net-next/master as of today: c3e042f54107376 ("igmp: remove unused macro IGMP_Vx_UNSOLICITED_REPORT_INTERVAL") and it worked great on my FPGA board using SGMII (but no in-band negotiation over that link). I had the 64-bit DMA patches on top, but that doesn't affect this series.
> 
> Tested-by: Andre Przywara <andre.przywara@arm.com>

Great, thanks for testing!

> Is this heading for 5.7?

Yes, that is my hope.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin]

On Thu, Feb 20, 2020 at 10:18:28AM +0000, Russell King - ARM Linux admin wrote:
> On Wed, Feb 19, 2020 at 03:30:36PM +0100, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 17/02/2020 17:24:21+0000, Russell King wrote:
> > > Convert the macb ethernet driver to use the finalised link
> > > parameters in mac_link_up() rather than the parameters in mac_config().
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  drivers/net/ethernet/cadence/macb.h      |  1 -
> > >  drivers/net/ethernet/cadence/macb_main.c | 46 ++++++++++++++----------
> > >  2 files changed, 27 insertions(+), 20 deletions(-)
> > > 
> > 
> > I did test the series after rebasing on top of the at91rm9200 fix.

Okay, I've updated my series, which will appear in my "phy" branch
later today.  However, what would you like me to do with the authorship
on the updated patch (I haven't yet checked what the differences were),
and can I add a tested-by to patch 1 for you?  I'll wait until you've
replied before pushing it out.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [CFT 6/8] net: macb: use resolved link config in mac_link_up()

[Alexandre Belloni]

On 24/02/2020 13:15:20+0000, Russell King - ARM Linux admin wrote:
> On Thu, Feb 20, 2020 at 10:18:28AM +0000, Russell King - ARM Linux admin wrote:
> > On Wed, Feb 19, 2020 at 03:30:36PM +0100, Alexandre Belloni wrote:
> > > Hi,
> > > 
> > > On 17/02/2020 17:24:21+0000, Russell King wrote:
> > > > Convert the macb ethernet driver to use the finalised link
> > > > parameters in mac_link_up() rather than the parameters in mac_config().
> > > > 
> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > > ---
> > > >  drivers/net/ethernet/cadence/macb.h      |  1 -
> > > >  drivers/net/ethernet/cadence/macb_main.c | 46 ++++++++++++++----------
> > > >  2 files changed, 27 insertions(+), 20 deletions(-)
> > > > 
> > > 
> > > I did test the series after rebasing on top of the at91rm9200 fix.
> 
> Okay, I've updated my series, which will appear in my "phy" branch
> later today.  However, what would you like me to do with the authorship
> on the updated patch (I haven't yet checked what the differences were),
> and can I add a tested-by to patch 1 for you?  I'll wait until you've
> replied before pushing it out.
> 

I don't mind you keeping the authorship. You can add the tested-by on
patch 1 and this one. Note that I've tested all three of the atmel
revisions of the IP.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

All,

This is now almost four months old, but I see that I didn't copy the
message to everyone who should've been, especially for the five
remaining drivers.

I had asked for input from maintainers to help me convert their
phylink-using drivers to the new style where mac_link_up() performs
the speed, duplex and pause setup rather than mac_config(). So far,
I have had very little assistance with this, and it is now standing
in the way of further changes to phylink, particularly with proper
PCS support. You are effectively blocking this work; I can't break
your code as that will cause a kernel regression.

This is one of the reasons why there were not many phylink patches
merged for the last merge window.

The following drivers in current net-next remain unconverted:

drivers/net/ethernet/mediatek/mtk_eth_soc.c
drivers/net/dsa/ocelot/felix.c
drivers/net/dsa/qca/ar9331.c
drivers/net/dsa/bcm_sf2.c
drivers/net/dsa/b53/b53_common.c

These can be easily identified by grepping for conditionals where the
expression matches the "MLO_PAUSE_.X" regexp.

I have an untested patch that I will be sending out today for
mtk_eth_soc.c, but the four DSA ones definitely require their authors
or maintainers to either make the changes, or assist with that since
their code is not straight forward.

Essentially, if you are listed in this email's To: header, then you
are listed as a maintainer for one of the affected drivers, and I am
requesting assistance from you for this task please.

Thanks.

Russell.

On Mon, Feb 17, 2020 at 05:22:43PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> The following series changes the phylink interface to allow us to
> better support split MAC / MAC PCS setups.  The fundamental change
> required for this turns out to be quite simple.
> 
> Today, mac_config() is used for everything to do with setting the
> parameters for the MAC, and mac_link_up() is used to inform the
> MAC driver that the link is now up (and so to allow packet flow.)
> mac_config() also has had a few implementation issues, with folk
> who believe that members such as "speed" and "duplex" are always
> valid, where "link" gets used inappropriately, etc.
> 
> With the proposed patches, all this changes subtly - but in a
> backwards compatible way at this stage.
> 
> We pass the the full resolved link state (speed, duplex, pause) to
> mac_link_up(), and it is now guaranteed that these parameters to
> this function will always be valid (no more SPEED_UNKNOWN or
> DUPLEX_UNKNOWN here - unless phylink is fed with such things.)
> 
> Drivers should convert over to using the state in mac_link_up()
> rather than configuring the speed, duplex and pause in the
> mac_config() method. The patch series includes a number of MAC
> drivers which I've thought have been easy targets - I've left the
> remainder as I think they need maintainer input. However, *all*
> drivers will need conversion for future phylink development.
> 
>  Documentation/networking/sfp-phylink.rst          |  17 +++-
>  drivers/net/dsa/b53/b53_common.c                  |   4 +-
>  drivers/net/dsa/b53/b53_priv.h                    |   4 +-
>  drivers/net/dsa/bcm_sf2.c                         |   4 +-
>  drivers/net/dsa/lantiq_gswip.c                    |   4 +-
>  drivers/net/dsa/mt7530.c                          |   4 +-
>  drivers/net/dsa/mv88e6xxx/chip.c                  |  79 +++++++++++++----
>  drivers/net/dsa/sja1105/sja1105_main.c            |   4 +-
>  drivers/net/ethernet/cadence/macb.h               |   1 -
>  drivers/net/ethernet/cadence/macb_main.c          |  53 ++++++-----
>  drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  61 ++++++++-----
>  drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
>  drivers/net/ethernet/marvell/mvneta.c             |  63 ++++++++-----
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c   | 102 +++++++++++++---------
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c       |   7 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   4 +-
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c |  37 ++++----
>  drivers/net/phy/phylink.c                         |   9 +-
>  include/linux/phylink.h                           |  57 ++++++++----
>  include/net/dsa.h                                 |   4 +-
>  net/dsa/port.c                                    |   7 +-
>  21 files changed, 350 insertions(+), 176 deletions(-)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
> 

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

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Vladimir Oltean]

Hi Russell,

On Sun, 21 Jun 2020 at 17:34, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> All,
>
> This is now almost four months old, but I see that I didn't copy the
> message to everyone who should've been, especially for the five
> remaining drivers.
>
> I had asked for input from maintainers to help me convert their
> phylink-using drivers to the new style where mac_link_up() performs
> the speed, duplex and pause setup rather than mac_config(). So far,
> I have had very little assistance with this, and it is now standing
> in the way of further changes to phylink, particularly with proper
> PCS support. You are effectively blocking this work; I can't break
> your code as that will cause a kernel regression.
>
> This is one of the reasons why there were not many phylink patches
> merged for the last merge window.
>
> The following drivers in current net-next remain unconverted:
>
> drivers/net/ethernet/mediatek/mtk_eth_soc.c
> drivers/net/dsa/ocelot/felix.c
> drivers/net/dsa/qca/ar9331.c
> drivers/net/dsa/bcm_sf2.c
> drivers/net/dsa/b53/b53_common.c
>
> These can be easily identified by grepping for conditionals where the
> expression matches the "MLO_PAUSE_.X" regexp.
>
> I have an untested patch that I will be sending out today for
> mtk_eth_soc.c, but the four DSA ones definitely require their authors
> or maintainers to either make the changes, or assist with that since
> their code is not straight forward.
>
> Essentially, if you are listed in this email's To: header, then you
> are listed as a maintainer for one of the affected drivers, and I am
> requesting assistance from you for this task please.
>
> Thanks.
>
> Russell.
>

If forcing MAC speed is to be moved in mac_link_up(), and if (as you
requested in the mdio-lynx-pcs thread) configuring the PCS is to be
moved in pcs_link_up() and pcs_config() respectively, then what
remains to be done in mac_config()?

Regards,
-Vladimir

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Russell King - ARM Linux admin]

On Sun, Jun 21, 2020 at 10:37:43PM +0300, Vladimir Oltean wrote:
> Hi Russell,
> 
> On Sun, 21 Jun 2020 at 17:34, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > All,
> >
> > This is now almost four months old, but I see that I didn't copy the
> > message to everyone who should've been, especially for the five
> > remaining drivers.
> >
> > I had asked for input from maintainers to help me convert their
> > phylink-using drivers to the new style where mac_link_up() performs
> > the speed, duplex and pause setup rather than mac_config(). So far,
> > I have had very little assistance with this, and it is now standing
> > in the way of further changes to phylink, particularly with proper
> > PCS support. You are effectively blocking this work; I can't break
> > your code as that will cause a kernel regression.
> >
> > This is one of the reasons why there were not many phylink patches
> > merged for the last merge window.
> >
> > The following drivers in current net-next remain unconverted:
> >
> > drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > drivers/net/dsa/ocelot/felix.c
> > drivers/net/dsa/qca/ar9331.c
> > drivers/net/dsa/bcm_sf2.c
> > drivers/net/dsa/b53/b53_common.c
> >
> > These can be easily identified by grepping for conditionals where the
> > expression matches the "MLO_PAUSE_.X" regexp.
> >
> > I have an untested patch that I will be sending out today for
> > mtk_eth_soc.c, but the four DSA ones definitely require their authors
> > or maintainers to either make the changes, or assist with that since
> > their code is not straight forward.
> >
> > Essentially, if you are listed in this email's To: header, then you
> > are listed as a maintainer for one of the affected drivers, and I am
> > requesting assistance from you for this task please.
> >
> > Thanks.
> >
> > Russell.
> >
> 
> If forcing MAC speed is to be moved in mac_link_up(), and if (as you
> requested in the mdio-lynx-pcs thread) configuring the PCS is to be
> moved in pcs_link_up() and pcs_config() respectively, then what
> remains to be done in mac_config()?

Hopefully very little, but I suspect there will still be a need for
some kind of interface to configure the MAC interface type at the MAC.

Note that I have said many many many times that using state->{speed,
duplex,pause} in mac_config() when in in-band mode is unreliable, yet
still people insist on using them.  There _are_ and always _have been_
paths in phylink where these members will be passed with an unresolved
state, and they will corrupt the link settings when that happens.

I know that phylink was deficient in its handling of a split PCS, but
I have worked to correct that.  That job still is not complete, because
because I'm held up by these drivers that have not yet converted.  I've
already waited a kernel cycle, despite having the next series of
phylink patches ready and waiting since early February.

I'm getting to the point of wishing that phylink did not have users
except my own.

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

Re: [CFT 0/8] rework phylink interface for split MAC/PCS support

[Florian Fainelli]

Le 2020-06-21 à 13:02, Russell King - ARM Linux admin a écrit :
> On Sun, Jun 21, 2020 at 10:37:43PM +0300, Vladimir Oltean wrote:
>> Hi Russell,
>>
>> On Sun, 21 Jun 2020 at 17:34, Russell King - ARM Linux admin
>> <linux@armlinux.org.uk> wrote:
>>>
>>> All,
>>>
>>> This is now almost four months old, but I see that I didn't copy the
>>> message to everyone who should've been, especially for the five
>>> remaining drivers.
>>>
>>> I had asked for input from maintainers to help me convert their
>>> phylink-using drivers to the new style where mac_link_up() performs
>>> the speed, duplex and pause setup rather than mac_config(). So far,
>>> I have had very little assistance with this, and it is now standing
>>> in the way of further changes to phylink, particularly with proper
>>> PCS support. You are effectively blocking this work; I can't break
>>> your code as that will cause a kernel regression.
>>>
>>> This is one of the reasons why there were not many phylink patches
>>> merged for the last merge window.
>>>
>>> The following drivers in current net-next remain unconverted:
>>>
>>> drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>> drivers/net/dsa/ocelot/felix.c
>>> drivers/net/dsa/qca/ar9331.c
>>> drivers/net/dsa/bcm_sf2.c
>>> drivers/net/dsa/b53/b53_common.c
>>>
>>> These can be easily identified by grepping for conditionals where the
>>> expression matches the "MLO_PAUSE_.X" regexp.
>>>
>>> I have an untested patch that I will be sending out today for
>>> mtk_eth_soc.c, but the four DSA ones definitely require their authors
>>> or maintainers to either make the changes, or assist with that since
>>> their code is not straight forward.
>>>
>>> Essentially, if you are listed in this email's To: header, then you
>>> are listed as a maintainer for one of the affected drivers, and I am
>>> requesting assistance from you for this task please.
>>>
>>> Thanks.
>>>
>>> Russell.
>>>
>>
>> If forcing MAC speed is to be moved in mac_link_up(), and if (as you
>> requested in the mdio-lynx-pcs thread) configuring the PCS is to be
>> moved in pcs_link_up() and pcs_config() respectively, then what
>> remains to be done in mac_config()?
> 
> Hopefully very little, but I suspect there will still be a need for
> some kind of interface to configure the MAC interface type at the MAC.
> 
> Note that I have said many many many times that using state->{speed,
> duplex,pause} in mac_config() when in in-band mode is unreliable, yet
> still people insist on using them.  There _are_ and always _have been_
> paths in phylink where these members will be passed with an unresolved
> state, and they will corrupt the link settings when that happens.
> 
> I know that phylink was deficient in its handling of a split PCS, but
> I have worked to correct that.  That job still is not complete, because
> because I'm held up by these drivers that have not yet converted.  I've
> already waited a kernel cycle, despite having the next series of
> phylink patches ready and waiting since early February.
> 
> I'm getting to the point of wishing that phylink did not have users
> except my own.

You have to realize that most people are not capable of working at your
pace either because they just do not have your intellect or because
their day job is not supporting a switch driver 100% of the time. The
other thing to realize is that PHYLINK has seen a fair amount of churn
since its introduction which makes it really hard to follow what is
needed, what was bogus, what ended up working when it should not etc.
You have clearly worked on improving both code and documentation, there
is however inertia for people to catch up.

So please stop with this attitude, just send the patches if it breaks it
will take many cycles for people to realize it and that is on them, not
you, they do not get to complain at all and should have been more
reactive since you warned them.

Thank you
-- 
Florian