On Sat, Oct 03, 2020 at 11:09:49PM +0530, Calvin Johnson wrote: > Hi Grant, > > On Fri, Oct 02, 2020 at 12:22:37PM +0100, Grant Likely wrote: > > > > <snip> > > > -static int dpaa2_mac_get_if_mode(struct device_node *node, > > > +static int dpaa2_mac_get_if_mode(struct fwnode_handle *dpmac_node, > > > struct dpmac_attr attr) > > > { > > > phy_interface_t if_mode; > > > int err; > > > - err = of_get_phy_mode(node, &if_mode); > > > - if (!err) > > > - return if_mode; > > > + err = fwnode_get_phy_mode(dpmac_node); > > > + if (err > 0) > > > + return err; > > > > Is this correct? The function prototype from patch 2 is: > > > > struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode) > > > > It returns struct fwnode_handle *. Does this compile? > > Will correct this. Sorry, this change looks correct to me. Actully, the function prototype is: int fwnode_get_phy_mode(struct fwnode_handle *fwnode); It is from drivers/base/property.c. fwnode_get_phy_node() defined in patch-2 is used in phylink_fwnode_phy_connect() The confusion maybe due to one letter difference in the fn names. Thanks Calvin