| Message ID | 20230618184119.4017149-2-andrew@lunn.ch (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: Rework EEE | expand |
On Sun, Jun 18, 2023 at 08:41:11PM +0200, Andrew Lunn wrote: > The text has been cut/paste from genphy_c45_ethtool_get_eee but not > changed to reflect it performs set. > > Additionally, extend the comment. This function implements the logic > that eee_enabled has global control over EEE. When eee_enabled is > false, no link modes will be advertised, and as a result, the MAC > should not transmit LPI. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/net/phy/phy-c45.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index fee514b96ab1..d1d7cf34ac0b 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -1425,12 +1425,15 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, > EXPORT_SYMBOL(genphy_c45_ethtool_get_eee); > > /** > - * genphy_c45_ethtool_set_eee - get EEE supported and status > + * genphy_c45_ethtool_set_eee - set EEE supported and status > * @phydev: target phy_device struct > * @data: ethtool_eee data > * > - * Description: it reportes the Supported/Advertisement/LP Advertisement > - * capabilities. > + * Description: it set the Supported/Advertisement/LP Advertisement Description: sets the ... > + * capabilities. If eee_enabled is false, no links modes are > + * advertised, but the previously advertised link modes are I'd suggest moving "advertised," to the preceding line to fill it more... > + * retained. This allows EEE to be enabled/disabled in a none > + * destructive way. which then would allow "non-destructive" here rather than the slightly more awkward (but correct) "non- destructive" formatting here. Thanks!
> > /** > > - * genphy_c45_ethtool_set_eee - get EEE supported and status > > + * genphy_c45_ethtool_set_eee - set EEE supported and status > > * @phydev: target phy_device struct > > * @data: ethtool_eee data > > * > > - * Description: it reportes the Supported/Advertisement/LP Advertisement > > - * capabilities. > > + * Description: it set the Supported/Advertisement/LP Advertisement > > Description: sets the ... > > > + * capabilities. If eee_enabled is false, no links modes are > > + * advertised, but the previously advertised link modes are > > I'd suggest moving "advertised," to the preceding line to fill it > more... > > > + * retained. This allows EEE to be enabled/disabled in a none > > + * destructive way. > > which then would allow "non-destructive" here rather than the slightly > more awkward (but correct) "non- > destructive" formatting here. Thanks for the comments. I actually ended up submitting this patch twice, one standalone and then again as part of this patchset. I should stop submitting patches today, i'm getting too many things wrong :-) I will fixup the standalone version. Andrew
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index fee514b96ab1..d1d7cf34ac0b 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -1425,12 +1425,15 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, EXPORT_SYMBOL(genphy_c45_ethtool_get_eee); /** - * genphy_c45_ethtool_set_eee - get EEE supported and status + * genphy_c45_ethtool_set_eee - set EEE supported and status * @phydev: target phy_device struct * @data: ethtool_eee data * - * Description: it reportes the Supported/Advertisement/LP Advertisement - * capabilities. + * Description: it set the Supported/Advertisement/LP Advertisement + * capabilities. If eee_enabled is false, no links modes are + * advertised, but the previously advertised link modes are + * retained. This allows EEE to be enabled/disabled in a none + * destructive way. */ int genphy_c45_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
The text has been cut/paste from genphy_c45_ethtool_get_eee but not changed to reflect it performs set. Additionally, extend the comment. This function implements the logic that eee_enabled has global control over EEE. When eee_enabled is false, no link modes will be advertised, and as a result, the MAC should not transmit LPI. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/phy/phy-c45.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)