Messages in this thread | | | Date | Wed, 8 Feb 2023 14:08:30 +0100 | From | Oleksij Rempel <> | Subject | Re: [PATCH net-next v6 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support |
| |
On Wed, Feb 08, 2023 at 11:32:07AM +0100, Oleksij Rempel wrote: > Add replacement for phy_ethtool_get/set_eee() functions. > > Current phy_ethtool_get/set_eee() implementation is great and it is > possible to make it even better: > - this functionality is for devices implementing parts of IEEE 802.3 > specification beyond Clause 22. The better place for this code is > phy-c45.c > - currently it is able to do read/write operations on PHYs with > different abilities to not existing registers. It is better to > use stored supported_eee abilities to avoid false read/write > operations. > - the eee_active detection will provide wrong results on not supported > link modes. It is better to validate speed/duplex properties against > supported EEE link modes. > - it is able to support only limited amount of link modes. We have more > EEE link modes... > > By refactoring this code I address most of this point except of the last > one. Adding additional EEE link modes will need more work. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> +/** > + * genphy_c45_eee_is_active - get EEE supported and status > + * @phydev: target phy_device struct > + * @data: ethtool_eee data
I forgot to update this comment.
> + * > + * Description: it reports the possible state of EEE functionality. > + */ > +int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv, > + unsigned long *lp, bool *is_enabled) > +{ > + __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_adv) = {}; > + __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_lp) = {}; > + __ETHTOOL_DECLARE_LINK_MODE_MASK(common); > + bool eee_enabled, eee_active; > + int ret; > + > + ret = genphy_c45_read_eee_adv(phydev, tmp_adv); > + if (ret) > + return ret; > + > + ret = genphy_c45_read_eee_lpa(phydev, tmp_lp); > + if (ret) > + return ret; > + > + eee_enabled = !linkmode_empty(tmp_adv); > + linkmode_and(common, tmp_adv, tmp_lp); > + if (eee_enabled && !linkmode_empty(common))i
and linkmode_empty() can be replaced with linkmode_intersects()
I'll wait if more comments will come and make a new version tomorrow.
Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
| |