Re: [PATCH net-next v3 1/3] net: phylink: provide phylink_mac_implements_lpi()

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Feb 10, 2025 at 10:36:44AM +0000, Russell King (Oracle) wrote:
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 898b00451bbf..0de78673172d 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -737,6 +737,18 @@ static inline int phylink_get_link_timer_ns(phy_interface_t interface)
>  	}
>  }
>  
> +/**
> + * phylink_mac_implements_lpi() - determine if MAC implements LPI ops
> + * @ops: phylink_mac_ops structure
> + *
> + * Returns true if the phylink MAC operations structure indicates that the
> + * LPI operations have been implemented, false otherwise.

This is something that I only noticed for v3 because I wanted to leave a
review tag, so I first checked the status in patchwork, but there it says:

include/linux/phylink.h:749: warning: No description found for return value of 'phylink_mac_implements_lpi'

I am aware of this conversation from November where you raised the point
about tooling being able to accept the syntax without the colon as well:
https://lore.kernel.org/netdev/87v7wjffo6.fsf@trenco.lwn.net/

but it looks like it didn't go anywhere, with Jon still preferring the
strict syntax for now, and no follow-up that I can see. So, the current
conventions are not these, and you haven't specifically said anywhere
that you are deliberately ignoring them.

In the end it's not something for me to decide, but I thought maybe I'm
speeding things up a little bit by opening up the discussion now, rather
than wait for a maintainer to look at it.

> + */
> +static inline bool phylink_mac_implements_lpi(const struct phylink_mac_ops *ops)
> +{
> +	return ops && ops->mac_disable_tx_lpi && ops->mac_enable_tx_lpi;
> +}
> +
>  void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
>  				      unsigned int neg_mode, u16 bmsr, u16 lpa);
>  void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,