> +/** > + * genphy_c45_plca_set_cfg - set PLCA configuration using standard registers > + * @phydev: target phy_device struct > + * @plca_cfg: structure containing the PLCA configuration. Fields set to -1 are > + * not to be changed. > + * > + * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA > + * Management Registers specifications, this function can be used to modify > + * the PLCA configuration using the standard registers in MMD 31. > + */ > +int genphy_c45_plca_set_cfg(struct phy_device *phydev, > + const struct phy_plca_cfg *plca_cfg) > +{ > + int ret; > + u16 val; > + > + // PLCA IDVER is read-only > + if (plca_cfg->version >= 0) > + return -EINVAL; > + > + // first of all, disable PLCA if required > + if (plca_cfg->enabled == 0) { > + ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2, > + MDIO_OATC14_PLCA_CTRL0, > + MDIO_OATC14_PLCA_EN); > + > + if (ret < 0) > + return ret; > + } > + > + if (plca_cfg->node_cnt >= 0 || plca_cfg->node_id >= 0) { > + if (plca_cfg->node_cnt < 0 || plca_cfg->node_id < 0) { I think it would be good to add some comments since this code is not immediately obvious to me. I had to think about it for a while. > + if (plca_cfg->burst_cnt >= 0 || plca_cfg->burst_tmr >= 0) { > + if (plca_cfg->burst_cnt < 0 || plca_cfg->burst_tmr < 0) { > + ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, > + MDIO_OATC14_PLCA_BURST); > + This follows the same patterns, so maybe comments here as well? With that, you can add my Reviewed-by. Andrew