| Message ID | 20230618184119.4017149-8-andrew@lunn.ch (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: Rework EEE | expand |
On 6/18/2023 7:41 PM, Andrew Lunn wrote: > In order for EEE to operate, both the MAC and the PHY need to support > it, similar to how pause works. Copy the pause concept and add the > call phy_support_eee() which the MAC makes after connecting the PHY to > indicate it supports EEE. phylib will then advertise EEE when auto-neg > is performed. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/net/phy/phy_device.c | 18 ++++++++++++++++++ > include/linux/phy.h | 3 ++- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 2cad9cc3f6b8..ae2ebe1df15c 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -2762,6 +2762,24 @@ void phy_advertise_supported(struct phy_device *phydev) > } > EXPORT_SYMBOL(phy_advertise_supported); > > +/** > + * phy_support_eee - Enable support of EEE > + * @phydev: target phy_device struct > + * > + * Description: Called by the MAC to indicate is supports Energy typo: is/it > + * Efficient Ethernet. This should be called before phy_start() in > + * order that EEE is negotiated when the link comes up as part of > + * phy_start(). EEE is enabled by default when the hardware supports > + * it. > + */ > +void phy_support_eee(struct phy_device *phydev) > +{ > + linkmode_copy(phydev->advertising_eee, phydev->supported_eee); > + phydev->eee_cfg.tx_lpi_enabled = true; > + phydev->eee_cfg.eee_enabled = true; > +} > +EXPORT_SYMBOL(phy_support_eee); A bit worried that naming this function might be confusing driver authors that this is a function that reports whether EEE is supported, though I am not able to come up with better names. > + > /** > * phy_support_sym_pause - Enable support of symmetrical pause > * @phydev: target phy_device struct > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 473ddf62bee9..29ae45d37011 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -693,7 +693,7 @@ struct phy_device { > __ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising); > /* used with phy_speed_down */ > __ETHTOOL_DECLARE_LINK_MODE_MASK(adv_old); > - /* used for eee validation */ > + /* used for eee validation and configuration*/ While at it, maybe capitalize to EEE? > __ETHTOOL_DECLARE_LINK_MODE_MASK(supported_eee); > __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising_eee); > bool eee_enabled; > @@ -1903,6 +1903,7 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode); > void phy_advertise_supported(struct phy_device *phydev); > void phy_support_sym_pause(struct phy_device *phydev); > void phy_support_asym_pause(struct phy_device *phydev); > +void phy_support_eee(struct phy_device *phydev); > void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx, > bool autoneg); > void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
On Mon, Jun 19, 2023 at 04:21:09PM +0100, Florian Fainelli wrote: > > > On 6/18/2023 7:41 PM, Andrew Lunn wrote: > > In order for EEE to operate, both the MAC and the PHY need to support > > it, similar to how pause works. Copy the pause concept and add the > > call phy_support_eee() which the MAC makes after connecting the PHY to > > indicate it supports EEE. phylib will then advertise EEE when auto-neg > > is performed. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/net/phy/phy_device.c | 18 ++++++++++++++++++ > > include/linux/phy.h | 3 ++- > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index 2cad9cc3f6b8..ae2ebe1df15c 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -2762,6 +2762,24 @@ void phy_advertise_supported(struct phy_device *phydev) > > } > > EXPORT_SYMBOL(phy_advertise_supported); > > +/** > > + * phy_support_eee - Enable support of EEE > > + * @phydev: target phy_device struct > > + * > > + * Description: Called by the MAC to indicate is supports Energy > > typo: is/it > > > + * Efficient Ethernet. This should be called before phy_start() in > > + * order that EEE is negotiated when the link comes up as part of > > + * phy_start(). EEE is enabled by default when the hardware supports > > + * it. > > + */ > > +void phy_support_eee(struct phy_device *phydev) > > +{ > > + linkmode_copy(phydev->advertising_eee, phydev->supported_eee); > > + phydev->eee_cfg.tx_lpi_enabled = true; > > + phydev->eee_cfg.eee_enabled = true; > > +} > > +EXPORT_SYMBOL(phy_support_eee); > > A bit worried that naming this function might be confusing driver authors > that this is a function that reports whether EEE is supported, though I am > not able to come up with better names. Possibly phy_enable_eee_support() ?
> > > + * Efficient Ethernet. This should be called before phy_start() in > > > + * order that EEE is negotiated when the link comes up as part of > > > + * phy_start(). EEE is enabled by default when the hardware supports > > > + * it. > > > + */ > > > +void phy_support_eee(struct phy_device *phydev) > > > +{ > > > + linkmode_copy(phydev->advertising_eee, phydev->supported_eee); > > > + phydev->eee_cfg.tx_lpi_enabled = true; > > > + phydev->eee_cfg.eee_enabled = true; > > > +} > > > +EXPORT_SYMBOL(phy_support_eee); > > > > A bit worried that naming this function might be confusing driver authors > > that this is a function that reports whether EEE is supported, though I am > > not able to come up with better names. > > Possibly phy_enable_eee_support() ? As i said in the commit message, i followed what we do for pause: void phy_support_sym_pause(struct phy_device *phydev); void phy_support_asym_pause(struct phy_device *phydev); but phy_enable_eee_support() is less ambiguous. Andrew
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 2cad9cc3f6b8..ae2ebe1df15c 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2762,6 +2762,24 @@ void phy_advertise_supported(struct phy_device *phydev) } EXPORT_SYMBOL(phy_advertise_supported); +/** + * phy_support_eee - Enable support of EEE + * @phydev: target phy_device struct + * + * Description: Called by the MAC to indicate is supports Energy + * Efficient Ethernet. This should be called before phy_start() in + * order that EEE is negotiated when the link comes up as part of + * phy_start(). EEE is enabled by default when the hardware supports + * it. + */ +void phy_support_eee(struct phy_device *phydev) +{ + linkmode_copy(phydev->advertising_eee, phydev->supported_eee); + phydev->eee_cfg.tx_lpi_enabled = true; + phydev->eee_cfg.eee_enabled = true; +} +EXPORT_SYMBOL(phy_support_eee); + /** * phy_support_sym_pause - Enable support of symmetrical pause * @phydev: target phy_device struct diff --git a/include/linux/phy.h b/include/linux/phy.h index 473ddf62bee9..29ae45d37011 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -693,7 +693,7 @@ struct phy_device { __ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising); /* used with phy_speed_down */ __ETHTOOL_DECLARE_LINK_MODE_MASK(adv_old); - /* used for eee validation */ + /* used for eee validation and configuration*/ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported_eee); __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising_eee); bool eee_enabled; @@ -1903,6 +1903,7 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode); void phy_advertise_supported(struct phy_device *phydev); void phy_support_sym_pause(struct phy_device *phydev); void phy_support_asym_pause(struct phy_device *phydev); +void phy_support_eee(struct phy_device *phydev); void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx, bool autoneg); void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
In order for EEE to operate, both the MAC and the PHY need to support it, similar to how pause works. Copy the pause concept and add the call phy_support_eee() which the MAC makes after connecting the PHY to indicate it supports EEE. phylib will then advertise EEE when auto-neg is performed. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/phy/phy_device.c | 18 ++++++++++++++++++ include/linux/phy.h | 3 ++- 2 files changed, 20 insertions(+), 1 deletion(-)