On Tue, Feb 20, 2024 at 03:57:38AM +0800, Yang Xiwen via B4 Relay wrote: > From: Yang Xiwen <forbidden405@xxxxxxxxxxx> > > Register the sub MDIO bus if it is found. Also implement the internal > PHY reset procedure as needed. > > Note it's unable to put the MDIO bus node outside of MAC controller > (i.e. at the same level in the parent bus node). Because we need to > control all clocks and resets in FEMAC driver due to the phy reset > procedure. So the clocks can't be assigned to MDIO bus device, which is > an essential resource for the MDIO bus to work. > > No backward compatibility is maintained since the only existing > user(Hi3516DV300) has not received any updates from HiSilicon for about > 8 years already. And till today, no mainline dts for this SoC is found. > It seems unlikely that there are still existing mainline Hi3516DV300 > users. The effort to maintain the old binding seems gain a little. > > Signed-off-by: Yang Xiwen <forbidden405@xxxxxxxxxxx> > --- > drivers/net/ethernet/hisilicon/hisi_femac.c | 77 +++++++++++++++++++++++------ > 1 file changed, 61 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c ... > @@ -826,15 +844,34 @@ static int hisi_femac_drv_probe(struct platform_device *pdev) > priv->phy_reset_delays, > DELAYS_NUM); > if (ret) > - goto out_disable_clk; > + goto out_free_netdev; > hisi_femac_phy_reset(priv); > } > > + // Register the optional MDIO bus > + for_each_available_child_of_node(node, mdio_np) { > + if (of_node_name_prefix(mdio_np, "mdio")) { > + priv->mdio_pdev = of_platform_device_create(mdio_np, NULL, dev); > + of_node_put(mdio_np); > + if (!priv->mdio_pdev) { > + dev_err(dev, "failed to register MDIO bus device\n"); > + ret = -ENODEV; > + goto out_free_netdev; > + } > + mdio_registered = true; > + break; > + } > + of_node_put(mdio_np); Sorry for not noticing this earlier. I think that of_node_put() only needs to be called in the case of terminating the loop (via break, goto, return, etc...). But should not be called otherwise (when the loop cycles) as for_each_available_child_of_node() calls of_node_put(). Flagged by Coccinelle. > + } > + > + if (!mdio_registered) > + dev_warn(dev, "MDIO subnode not found. This is usually a bug.\n"); > + > phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link); > if (!phy) { > dev_err(dev, "connect to PHY failed!\n"); > ret = -ENODEV; > - goto out_disable_clk; > + goto out_unregister_mdio_bus; > } > > phy_attached_print(phy, "phy_id=0x%.8lx, phy_mode=%s\n", ...