On Tue, May 5, 2020 at 4:29 PM Calvin Johnson <calvin.johnson@xxxxxxxxxxx> wrote: > > Define phylink_fwnode_phy_connect() to connect phy specified by > a fwnode to a phylink instance. ... > + int ret = 0; Redundant assignment. > + if ((IS_ERR(phy_fwnode)) && pl->cfg_link_an_mode == MLO_AN_PHY) No Lisp, please. > + return -ENODEV; ... > + phy_dev = fwnode_phy_find_device(phy_fwnode); > + fwnode_handle_put(phy_fwnode); Hmm... Isn't it racy? I mean if you put fwnode here the phy_dev may already be gone before you call phy_attach_direct, right? > + if (!phy_dev) > + return -ENODEV; > + > + ret = phy_attach_direct(pl->netdev, phy_dev, flags, > + pl->link_interface); > + if (ret) > + return ret; > + > + ret = phylink_bringup_phy(pl, phy_dev, pl->link_config.interface); > + if (ret) > + phy_detach(phy_dev); > + > + return ret; -- With Best Regards, Andy Shevchenko
