linux-kernel - [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri,  3 Mar 2023 19:31:59 -0500
From:   Sean Anderson <sean.anderson@...o.com>
To:     Ioana Ciornei <ioana.ciornei@....com>,
        "David S . Miller" <davem@...emloft.net>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Sean Anderson <sean.anderson@...o.com>
Subject: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links

When commenting on what would become commit 085f1776fa03 ("net: dpaa2-mac:
add backplane link mode support"), Ioana Ciornei said [1]:

> ...DPMACs in TYPE_BACKPLANE can have both their PCS and SerDes managed
> by Linux (since the firmware is not touching these). That being said,
> DPMACs in TYPE_PHY (the type that is already supported in dpaa2-mac) can
> also have their PCS managed by Linux (no interraction from the
> firmware's part with the PCS, just the SerDes).

This implies that Linux only manages the SerDes when the link type is
backplane. Modify the condition in dpaa2_mac_connect to reflect this,
moving the existing conditions to more appropriate places.

[1] https://lore.kernel.org/netdev/20210120221900.i6esmk6uadgqpdtu@skbuf/

Fixes: f978fe85b8d1 ("dpaa2-mac: configure the SerDes phy on a protocol change")
Signed-off-by: Sean Anderson <sean.anderson@...o.com>
---
For v2 I tested a variety of setups to try and determine what the
behavior is. I evaluated the following branches on a variety of commits
on an LS1088ARDB:

- net/master
- this commit alone
- my lynx10g series [1] alone
- both of the above together

I also switched between MC firmware 10.30 (no protocol change support)
and 10.34 (with protocol change support), and I tried MAC link types of
of FIXED, PHY, and BACKPLANE. After loading the MC firmware, DPC,
kernel, and dtb, I booted up and ran

$ ls-addni dpmac.1

I had a 10G fiber SFP module plugged in and connected on the other end
to my computer.

My results are as follows:

- When the link type is FIXED, all configurations work.
- PHY and BACKPLANE do not work on net/master.
- I occasionally saw an ENOTSUPP error from dpmac_set_protocol with MC
  version 10.30. I am not sure what the cause of this is, as I was
  unable to reproduce it reliably.
- Occasionally, the link did not come up with my lynx10g series without
  this commit. Like the above issue, this would persist across reboots,
  but switching to another configuration and back would often fix this
  issue.

Unfortunately, I was unable to pinpoint any "smoking gun" due to
difficulty in reproducing errors.  However, I still think this commit is
correct, and should be applied. If Linux and the MC are out of sync,
most of the time things will work correctly but occasionally they won't.

[1] https://lore.kernel.org/linux-arm-kernel/20221230000139.2846763-1-sean.anderson@seco.com/
    But with some additional changes for v10.

Changes in v2:
- Fix incorrect condition in dpaa2_mac_set_supported_interfaces

 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index c886f33f8c6f..9b40c862d807 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -179,9 +179,13 @@ static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
 	if (err)
 		netdev_err(mac->net_dev,  "dpmac_set_protocol() = %d\n", err);
 
-	err = phy_set_mode_ext(mac->serdes_phy, PHY_MODE_ETHERNET, state->interface);
-	if (err)
-		netdev_err(mac->net_dev, "phy_set_mode_ext() = %d\n", err);
+	if (!phy_interface_mode_is_rgmii(mode)) {
+		err = phy_set_mode_ext(mac->serdes_phy, PHY_MODE_ETHERNET,
+				       state->interface);
+		if (err)
+			netdev_err(mac->net_dev, "phy_set_mode_ext() = %d\n",
+				   err);
+	}
 }
 
 static void dpaa2_mac_link_up(struct phylink_config *config,
@@ -317,7 +321,8 @@ static void dpaa2_mac_set_supported_interfaces(struct dpaa2_mac *mac)
 		}
 	}
 
-	if (!mac->serdes_phy)
+	if (!(mac->features & DPAA2_MAC_FEATURE_PROTOCOL_CHANGE) ||
+	    !mac->serdes_phy)
 		return;
 
 	/* In case we have access to the SerDes phy/lane, then ask the SerDes
@@ -377,8 +382,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
 		return -EINVAL;
 	mac->if_mode = err;
 
-	if (mac->features & DPAA2_MAC_FEATURE_PROTOCOL_CHANGE &&
-	    !phy_interface_mode_is_rgmii(mac->if_mode) &&
+	if (mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE &&
 	    is_of_node(dpmac_node)) {
 		serdes_phy = of_phy_get(to_of_node(dpmac_node), NULL);
 
-- 
2.35.1.1320.gc452695387.dirty