[PATCH net] net: ibm: emac: attach the fixed-link PHY with of_phy_connect()

[PATCH net] net: ibm: emac: attach the fixed-link PHY with of_phy_connect()

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

emac_init_phy() handles a "fixed-link" property by registering a fixed
PHY for the MAC's own OF node and then assigning net_device->phydev to
it by hand:

	res = of_phy_register_fixed_link(np);
	ndev->phydev = of_phy_find_device(np);
	if (res || !ndev->phydev)
		return res ? res : -EINVAL;
	emac_adjust_link(dev->ndev);
	put_device(&ndev->phydev->mdio.dev);

net_device->phydev is meant to be set by phy_attach_direct(), which is
what the of_phy_connect() helper reaches. Assigning it by hand skips
everything attach does: phydev->attached_dev stays NULL, no link change
handler is wired up, the PHY never enters the netdev's phy_link_topology,
and the put_device() drops the reference the driver just took, so the
phy_device is kept alive afterwards only by the fixed MDIO bus it is
registered on.

The missing topology entry is what prompted this. ethnl_req_get_phydev()
returns dev->phydev as-is when an ethtool netlink request carries no
phy-index, so ETHTOOL_MSG_PHY_GET on an EMAC port with a fixed-link finds
a phydev and then looks it up in a link_topo that was never allocated:

	struct phy_link_topology *topo = reply_data->dev->link_topo;
	pdn = xa_load(&topo->phys, phydev->phyindex);

which dereferences NULL in phy_prepare_data().

Every other phylib driver with fixed-link support registers the fixed PHY
and then passes the MAC's own OF node to of_phy_connect(): gianfar, fec,
davinci_emac and nixge all do it, and so does emac_dt_phy_connect() for
the phy-handle case. Do the same here. of_phy_connect() takes the attach
reference and drops the of_phy_find_device() one, so the explicit
put_device() goes away. emac_adjust_link() is still called once by hand
afterwards and the PHY state machine is still not started, because a
fixed link does not change. The rest of the driver already treats
dev->phy.address < 0 as "no PHY to poll" and never reaches
dev->phy.def->ops, so nothing else changes.

The hand assignment itself dates back to commit 08e39982ef64 ("net: emac:
fix fixed-link setup for the RTL8363SB switch"), but it only started
affecting the net_device when commit baab9de385a8 ("net: ibm: emac: use
netdev's phydev directly") moved the pointer from the driver private
dev->phy_dev into ndev->phydev, so that is the commit fixed here.

No in-tree DTS pairs "ibm,emac" with a "fixed-link" property, so this is
latent in mainline; it is out-of-tree users such as the Netgear WNDAP620
that hit it. There is no such hardware here, so the change is verified by
cross compilation and inspection only.

emac_remove() still does not call phy_disconnect(), which the phy-handle
path gets wrong today as well. That is left to a separate change.

Fixes: baab9de385a8 ("net: ibm: emac: use netdev's phydev directly")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 drivers/net/ethernet/ibm/emac/core.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 640618c..6e8b711 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -2674,11 +2674,14 @@ static int emac_init_phy(struct emac_instance *dev)
 				return res;
 
 			res = of_phy_register_fixed_link(np);
-			ndev->phydev = of_phy_find_device(np);
-			if (res || !ndev->phydev)
-				return res ? res : -EINVAL;
-			emac_adjust_link(dev->ndev);
-			put_device(&ndev->phydev->mdio.dev);
+			if (res)
+				return res;
+
+			if (!of_phy_connect(ndev, np, &emac_adjust_link, 0,
+					    dev->phy_mode))
+				return -EINVAL;
+
+			emac_adjust_link(ndev);
 		}
 		return 0;
 	}
-- 
2.25.1