diff mbox series

[net-next,v3,7/8] dpaa2-mac: configure the SerDes phy on a protocol change

Message ID 20220310145200.3645763-8-ioana.ciornei@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series dpaa2-mac: add support for changing the protocol at runtime | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ioana Ciornei March 10, 2022, 2:51 p.m. UTC
This patch integrates the dpaa2-eth driver with the generic PHY
infrastructure in order to search, find and reconfigure the SerDes lanes
in case of a protocol change.

On the .mac_config() callback, the phy_set_mode_ext() API is called so
that the Lynx 28G SerDes PHY driver can change the lane's configuration.
In the same phylink callback the MC firmware is called so that it
reconfigures the MAC side to run using the new protocol.

The consumer drivers - dpaa2-eth and dpaa2-switch - are updated to call
the dpaa2_mac_start/stop functions newly added which will
power_on/power_off the associated SerDes lane.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v2:
	- none
Changes in v3:
	- 7/8: reverse order of dpaa2_mac_start() and phylink_start()
	- 7/8: treat all RGMII variants in dpmac_eth_if_mode
	- 7/8: remove the .mac_prepare callback
	- 7/8: ignore PHY_INTERFACE_MODE_NA in validate

 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |  5 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 88 +++++++++++++++++++
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |  6 ++
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   |  5 +-
 4 files changed, 102 insertions(+), 2 deletions(-)

Comments

Russell King (Oracle) March 10, 2022, 3:05 p.m. UTC | #1
On Thu, Mar 10, 2022 at 04:51:59PM +0200, Ioana Ciornei wrote:
> This patch integrates the dpaa2-eth driver with the generic PHY
> infrastructure in order to search, find and reconfigure the SerDes lanes
> in case of a protocol change.
> 
> On the .mac_config() callback, the phy_set_mode_ext() API is called so
> that the Lynx 28G SerDes PHY driver can change the lane's configuration.
> In the same phylink callback the MC firmware is called so that it
> reconfigures the MAC side to run using the new protocol.
> 
> The consumer drivers - dpaa2-eth and dpaa2-switch - are updated to call
> the dpaa2_mac_start/stop functions newly added which will
> power_on/power_off the associated SerDes lane.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Looks better, there's a minor thing that I missed, sorry:

> +	if (mac->features & DPAA2_MAC_FEATURE_PROTOCOL_CHANGE &&
> +	    !phy_interface_mode_is_rgmii(mac->if_mode) &&
> +	    is_of_node(dpmac_node)) {
> +		serdes_phy = of_phy_get(to_of_node(dpmac_node), NULL);
> +
> +		if (IS_ERR(serdes_phy)) {
> +			if (PTR_ERR(serdes_phy) == -ENODEV)
> +				serdes_phy = NULL;
> +			else
> +				return PTR_ERR(serdes_phy);
> +		} else {
> +			phy_init(serdes_phy);
> +		}

Would:
		if (PTR_ERR(serdes_phy) == -ENODEV)
			serdes_phy = NULL;
		else if (IS_ERR(serdes_phy))
			return PTR_ERR(serdes_phy);
		else
			phy_init(serdes_phy);

be neater? There is no need to check IS_ERR() before testing PTR_ERR().
One may also prefer the pointer-comparison approach:

		if (serdes_phy == ERR_PTR(-ENODEV))

to remove any question about PTR_ERR(p) on a !IS_ERR(p) value too, but
it really doesn't make any difference.

I suspect this is just a code formatting issue, I'd think the compiler
would generate reasonable code either way, so as I said above, it's
quite minor.
Ioana Ciornei March 10, 2022, 3:57 p.m. UTC | #2
On Thu, Mar 10, 2022 at 03:05:50PM +0000, Russell King (Oracle) wrote:
> On Thu, Mar 10, 2022 at 04:51:59PM +0200, Ioana Ciornei wrote:
> > This patch integrates the dpaa2-eth driver with the generic PHY
> > infrastructure in order to search, find and reconfigure the SerDes lanes
> > in case of a protocol change.
> > 
> > On the .mac_config() callback, the phy_set_mode_ext() API is called so
> > that the Lynx 28G SerDes PHY driver can change the lane's configuration.
> > In the same phylink callback the MC firmware is called so that it
> > reconfigures the MAC side to run using the new protocol.
> > 
> > The consumer drivers - dpaa2-eth and dpaa2-switch - are updated to call
> > the dpaa2_mac_start/stop functions newly added which will
> > power_on/power_off the associated SerDes lane.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> Looks better, there's a minor thing that I missed, sorry:
> 
> > +	if (mac->features & DPAA2_MAC_FEATURE_PROTOCOL_CHANGE &&
> > +	    !phy_interface_mode_is_rgmii(mac->if_mode) &&
> > +	    is_of_node(dpmac_node)) {
> > +		serdes_phy = of_phy_get(to_of_node(dpmac_node), NULL);
> > +
> > +		if (IS_ERR(serdes_phy)) {
> > +			if (PTR_ERR(serdes_phy) == -ENODEV)
> > +				serdes_phy = NULL;
> > +			else
> > +				return PTR_ERR(serdes_phy);
> > +		} else {
> > +			phy_init(serdes_phy);
> > +		}
> 
> Would:
> 		if (PTR_ERR(serdes_phy) == -ENODEV)
> 			serdes_phy = NULL;
> 		else if (IS_ERR(serdes_phy))
> 			return PTR_ERR(serdes_phy);
> 		else
> 			phy_init(serdes_phy);
> 

Yes, it wouldn't be an if inside another if statement.

> be neater? There is no need to check IS_ERR() before testing PTR_ERR().
> One may also prefer the pointer-comparison approach:
> 
> 		if (serdes_phy == ERR_PTR(-ENODEV))
> 
> to remove any question about PTR_ERR(p) on a !IS_ERR(p) value too, but
> it really doesn't make any difference.
> 
> I suspect this is just a code formatting issue, I'd think the compiler
> would generate reasonable code either way, so as I said above, it's
> quite minor.
> 

As you said, since it's quite minor I am going to wait to see if more
comments will appear, if not I am going to fix this up in another patch.

Thanks!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 939fa9db6a2e..4b047255d928 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -2077,8 +2077,10 @@  static int dpaa2_eth_open(struct net_device *net_dev)
 		goto enable_err;
 	}
 
-	if (dpaa2_eth_is_type_phy(priv))
+	if (dpaa2_eth_is_type_phy(priv)) {
+		dpaa2_mac_start(priv->mac);
 		phylink_start(priv->mac->phylink);
+	}
 
 	return 0;
 
@@ -2153,6 +2155,7 @@  static int dpaa2_eth_stop(struct net_device *net_dev)
 
 	if (dpaa2_eth_is_type_phy(priv)) {
 		phylink_stop(priv->mac->phylink);
+		dpaa2_mac_stop(priv->mac);
 	} else {
 		netif_tx_stop_all_queues(net_dev);
 		netif_carrier_off(net_dev);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index e6e758eaafea..0a5430bae3fb 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -3,6 +3,7 @@ 
 
 #include <linux/acpi.h>
 #include <linux/pcs-lynx.h>
+#include <linux/phy/phy.h>
 #include <linux/property.h>
 
 #include "dpaa2-eth.h"
@@ -60,6 +61,29 @@  static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)
 	return 0;
 }
 
+static enum dpmac_eth_if dpmac_eth_if_mode(phy_interface_t if_mode)
+{
+	switch (if_mode) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		return DPMAC_ETH_IF_RGMII;
+	case PHY_INTERFACE_MODE_USXGMII:
+		return DPMAC_ETH_IF_USXGMII;
+	case PHY_INTERFACE_MODE_QSGMII:
+		return DPMAC_ETH_IF_QSGMII;
+	case PHY_INTERFACE_MODE_SGMII:
+		return DPMAC_ETH_IF_SGMII;
+	case PHY_INTERFACE_MODE_10GBASER:
+		return DPMAC_ETH_IF_XFI;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		return DPMAC_ETH_IF_1000BASEX;
+	default:
+		return DPMAC_ETH_IF_MII;
+	}
+}
+
 static struct fwnode_handle *dpaa2_mac_get_node(struct device *dev,
 						u16 dpmac_id)
 {
@@ -147,6 +171,19 @@  static void dpaa2_mac_config(struct phylink_config *config, unsigned int mode,
 	if (err)
 		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() = %d\n",
 			   __func__, err);
+
+	if (!mac->serdes_phy)
+		return;
+
+	/* This happens only if we support changing of protocol at runtime */
+	err = dpmac_set_protocol(mac->mc_io, 0, mac->mc_dev->mc_handle,
+				 dpmac_eth_if_mode(state->interface));
+	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);
 }
 
 static void dpaa2_mac_link_up(struct phylink_config *config,
@@ -259,6 +296,8 @@  static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
 
 static void dpaa2_mac_set_supported_interfaces(struct dpaa2_mac *mac)
 {
+	int intf, err;
+
 	/* We support the current interface mode, and if we have a PCS
 	 * similar interface modes that do not require the SerDes lane to be
 	 * reconfigured.
@@ -278,12 +317,43 @@  static void dpaa2_mac_set_supported_interfaces(struct dpaa2_mac *mac)
 			break;
 		}
 	}
+
+	if (!mac->serdes_phy)
+		return;
+
+	/* In case we have access to the SerDes phy/lane, then ask the SerDes
+	 * driver what interfaces are supported based on the current PLL
+	 * configuration.
+	 */
+	for (intf = 0; intf < PHY_INTERFACE_MODE_MAX; intf++) {
+		if (intf == PHY_INTERFACE_MODE_NA)
+			continue;
+
+		err = phy_validate(mac->serdes_phy, PHY_MODE_ETHERNET, intf, NULL);
+		if (err)
+			continue;
+
+		__set_bit(intf, mac->phylink_config.supported_interfaces);
+	}
+}
+
+void dpaa2_mac_start(struct dpaa2_mac *mac)
+{
+	if (mac->serdes_phy)
+		phy_power_on(mac->serdes_phy);
+}
+
+void dpaa2_mac_stop(struct dpaa2_mac *mac)
+{
+	if (mac->serdes_phy)
+		phy_power_off(mac->serdes_phy);
 }
 
 int dpaa2_mac_connect(struct dpaa2_mac *mac)
 {
 	struct net_device *net_dev = mac->net_dev;
 	struct fwnode_handle *dpmac_node;
+	struct phy *serdes_phy = NULL;
 	struct phylink *phylink;
 	int err;
 
@@ -300,6 +370,22 @@  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) &&
+	    is_of_node(dpmac_node)) {
+		serdes_phy = of_phy_get(to_of_node(dpmac_node), NULL);
+
+		if (IS_ERR(serdes_phy)) {
+			if (PTR_ERR(serdes_phy) == -ENODEV)
+				serdes_phy = NULL;
+			else
+				return PTR_ERR(serdes_phy);
+		} else {
+			phy_init(serdes_phy);
+		}
+	}
+	mac->serdes_phy = serdes_phy;
+
 	/* The MAC does not have the capability to add RGMII delays so
 	 * error out if the interface mode requests them and there is no PHY
 	 * to act upon them
@@ -363,6 +449,8 @@  void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
 	phylink_disconnect_phy(mac->phylink);
 	phylink_destroy(mac->phylink);
 	dpaa2_pcs_destroy(mac);
+	of_phy_put(mac->serdes_phy);
+	mac->serdes_phy = NULL;
 }
 
 int dpaa2_mac_open(struct dpaa2_mac *mac)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index d2e51d21c80c..a58cab188a99 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -26,6 +26,8 @@  struct dpaa2_mac {
 	enum dpmac_link_type if_link_type;
 	struct phylink_pcs *pcs;
 	struct fwnode_handle *fw_node;
+
+	struct phy *serdes_phy;
 };
 
 bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
@@ -45,4 +47,8 @@  void dpaa2_mac_get_strings(u8 *data);
 
 void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data);
 
+void dpaa2_mac_start(struct dpaa2_mac *mac);
+
+void dpaa2_mac_stop(struct dpaa2_mac *mac);
+
 #endif /* DPAA2_MAC_H */
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 9a561072aa4a..e507e9065214 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -703,8 +703,10 @@  static int dpaa2_switch_port_open(struct net_device *netdev)
 
 	dpaa2_switch_enable_ctrl_if_napi(ethsw);
 
-	if (dpaa2_switch_port_is_type_phy(port_priv))
+	if (dpaa2_switch_port_is_type_phy(port_priv)) {
+		dpaa2_mac_start(port_priv->mac);
 		phylink_start(port_priv->mac->phylink);
+	}
 
 	return 0;
 }
@@ -717,6 +719,7 @@  static int dpaa2_switch_port_stop(struct net_device *netdev)
 
 	if (dpaa2_switch_port_is_type_phy(port_priv)) {
 		phylink_stop(port_priv->mac->phylink);
+		dpaa2_mac_stop(port_priv->mac);
 	} else {
 		netif_tx_stop_all_queues(netdev);
 		netif_carrier_off(netdev);