RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node

From: Wei Fang <wei.fang@nxp.com>
Sent: Thursday, October 15, 2026 5:17 AM
[...]

Subject: RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for
ENETCv4 Ports via a fixed-link node

quoted

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h

b/drivers/net/ethernet/freescale/enetc/enetc.h
index d9e91832a9c1..ed5402f1b73c 100644

--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h

@@ -500,6 +500,7 @@ struct enetc_ndev_priv {

 	struct clk *ref_clk; /* RGMII/RMII reference clock */
 	u64 sysclk_freq; /* NETC system clock frequency */
+	struct of_changeset *ppm_ocs; /* pseudo-MAC fixed-link changeset

*/

struct enetc_ndev_priv is shared by both PF and VF, but ppm_ocs is only
used by PF. Would it be better to move it to struct enetc_pf?

Ok, will try this out.

quoted

 };

[...]

quoted

+static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
+{
+	struct device_node *np = dev_of_node(priv->dev);
+	struct device_node *fixed_link;
+	struct of_changeset *ocs;
+	int speed, err;
+
+	if (!np)
+		return dev_err_probe(priv->dev, -ENODEV,
+				     "Missing of_node for Pseudo-MAC port\n");
+
+	enetc4_get_pcr_speed(&priv->si->hw, &speed);
+	speed = enetc_phylink_match_pseudo_mac_speed(speed);
+
+	ocs = kzalloc_obj(*ocs);
+	if (!ocs)
+		return -ENOMEM;
+
+	of_changeset_init(ocs);
+
+	fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
+	if (!fixed_link) {
+		err = -ENOMEM;
+		goto err_destroy;
+	}
+
+	err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
+	if (err)
+		goto err_put_node;
+
+	err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
+	if (err)
+		goto err_put_node;
+
+	err = of_changeset_apply(ocs);
+	if (err)
+		goto err_put_node;
+
+	of_node_put(fixed_link);
+	priv->ppm_ocs = ocs;
+
+	return 0;
+
+err_put_node:
+	of_node_put(fixed_link);
+err_destroy:
+	of_changeset_destroy(ocs);
+	kfree(ocs);
+
+	return dev_err_probe(priv->dev, err,
+			     "Failed to add fixed-link node\n");
+}
+
 static int enetc4_link_init(struct enetc_ndev_priv *priv,
 			    struct device_node *node)
 {
+	bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
 	struct enetc_pf *pf = enetc_si_priv(priv->si);
 	struct device *dev = priv->dev;
 	int err;

-	err = of_get_phy_mode(node, &pf->if_mode);
-	if (err) {
-		dev_err(dev, "Failed to get PHY mode\n");
-		return err;

[...]

quoted

+	if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {

This indicates that the node pointer can be NULL, but
enetc4_add_ppm_fixed_link() subsequently does not allow node
to be NULL. This is puzzling.

enetc4_add_ppm_fixed_link() checks dev_of_node(priv->dev) and
if NULL, it returns error and prints following err message:
"Missing of_node for Pseudo-MAC port\n".
Which is fine for the enetc_is_pseudo_mac() case.

One alternative would be to take the common 'else' branch in case of NULL
node for the pseudo-MAC, and let of_get_phy_mode() return with error, but 
in that case "Failed to get PHY mode\n" is not a good error indication.

So, nothing to change here, the 'if' branch check for the pseudo-MAC case is
correct as it is.

quoted

+		pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;
+
+		err = enetc4_add_ppm_fixed_link(priv);
+		if (err)
+			return err;
+
+		/* The fixed-link node was spliced onto dev's own of_node, so
+		 * phylink will find it via dev_fwnode(dev). Use that node for
+		 * the subsequent MDIO/phylink setup below.
+		 */
+		node = dev_of_node(dev);

Based on the implementation of enetc4_add_ppm_fixed_link(), the
input parameter dev->of_node cannot be NULL. I'm not familiar
with the of_changeset_* APIs. Will enetc4_add_ppm_fixed_link()
update this pointer?

Sure, all these functions take 'np' as argument, and 'np' is
dev_of_node(priv->dev), which is the same as dev->of_node.

quoted

+	} else {
+		err = of_get_phy_mode(node, &pf->if_mode);
+		if (err) {
+			dev_err(dev, "Failed to get PHY mode\n");
+			return err;
+		}
 	}

 	err = enetc_mdiobus_create(pf, node);
 	if (err) {
 		dev_err(dev, "Failed to create MDIO bus\n");
-		return err;
+		goto err_mdiobus_create;
 	}

 	err = enetc_phylink_create(priv, node, &enetc_pl_mac_ops);