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

On Tue, Feb 18, 2020 at 10:42:41AM +0000, Ioana Ciornei wrote:

quoted

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 [off-list ref]

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

Ioana Ciornei [off-list ref]

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?

quoted

On Mon, Feb 17, 2020 at 05:24:16PM +0000, Russell King wrote:

quoted

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 <redacted>
---
 .../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,

quoted

 	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;

quoted

-		else
-			dpmac_state->options &=

~DPMAC_LINK_OPT_HALF_DUPLEX;

quoted

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

quoted

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

quoted

+		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =

%d\n",

quoted

+			   __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,

quoted

 	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;

quoted

+		else if (duplex == DUPLEX_FULL)
+			dpmac_state->options &=

~DPMAC_LINK_OPT_HALF_DUPLEX;

quoted

+
+		/* 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;

quoted

+		else
+			dpmac_state->options &=

~DPMAC_LINK_OPT_ASYM_PAUSE;

quoted

+	}
+
 	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);

quoted

+		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =

%d\n",

quoted

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

quoted

--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&data=02%7C01%7Cioana.cior
nei%40nxp.com%7C09d0167191914135433808d7b45e15fd%7C686ea1d3bc2b4
c6fa92cd99c5c301635%7C0%7C0%7C637176188497544105&sdata=t0%2B
OzkoqRM180UHGBrW6FYAvHsIelx4CaP4oC3QcP1k%3D&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