Hello Andrew, thank you very much for your review. I have fixed this issue. Piergiorgio On Sat, Jan 07, 2023 at 06:37:52PM +0100, Andrew Lunn wrote: > > + // if not enabling PLCA, skip a few sanity checks > > + if (plca_cfg->enabled <= 0) > > + goto apply_cfg; > > + > > + if (!linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT, > > + phydev->advertising)) { > > + ret = -EOPNOTSUPP; > > + NL_SET_ERR_MSG(extack, > > + "Point to Multi-Point mode is not enabled"); > > + } > > + > > + // allow setting node_id concurrently with enabled > > + if (plca_cfg->node_id >= 0) > > + curr_plca_cfg->node_id = plca_cfg->node_id; > > + > > + if (curr_plca_cfg->node_id >= 255) { > > + NL_SET_ERR_MSG(extack, "PLCA node ID is not set"); > > + ret = -EINVAL; > > + goto out_drv; > > + } > > + > > +apply_cfg: > > + ret = phydev->drv->set_plca_cfg(phydev, plca_cfg); > > Goto which don't jump to the end of the function is generally frowned > upon. I suggest you put these sanity checks into a little helper, so > you can avoid the goto. > > With that change make, feel free to add my reviewed-by. > > Andrew