Re: [PATCH net-next v4 3/4] net: cadence: macb: add ethtool EEE support

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Thu, Feb 26, 2026 at 10:06:07AM +0100, Nicolai Buchwitz wrote:
> Implement get_eee and set_eee ethtool ops for GEM, delegating to
> phylink_ethtool_get_eee() and phylink_ethtool_set_eee(). Both
> handlers return -EOPNOTSUPP when MACB_CAPS_EEE is absent, keeping
> the ops safe to register unconditionally in gem_ethtool_ops.
> 
> Phylink owns the full EEE lifecycle: PHY advertisement, link
> partner negotiation, timer management and ethtool state; the MAC
> driver only provides the mac_enable_tx_lpi / mac_disable_tx_lpi
> callbacks wired up in the previous patch.
> 
> Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 6b9c63195970..a7e00b99e9a8 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4034,6 +4034,26 @@ static const struct ethtool_ops macb_ethtool_ops = {
>  	.set_ringparam		= macb_set_ringparam,
>  };
>  
> +static int macb_get_eee(struct net_device *dev, struct ethtool_keee *eee)
> +{
> +	struct macb *bp = netdev_priv(dev);
> +
> +	if (!(bp->caps & MACB_CAPS_EEE))
> +		return -EOPNOTSUPP;

There's no need for these checks, because phylink implements them
(phylink's implmentation is designed to relieve the driver writer of as
much burden as possible.)

You already tell phylink that you implement LPI support by populating
the two operations in struct phylink_mac_ops. This results in
pl->mac_supports_eee_ops being set true during phylink_create().
However, if you then don't supply both config->lpi_capabilities and
config->lpi_interfaces, then pl->mac_supports_eee will be false. This
will cause both phylink EEE ops to return -EOPNOTSUPP.

As you're already conditionally populating those depending on
MACB_CAPS_EEE, you've done everything already for phylink to return
-EOPNOTSUPP when MACB_CAPS_EEE is not set.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!