Subject: Re: [PATCH net-next 05/10] net: phylink: add EEE management

From: Simon Horman <[email-masked]>
Date: Fri, 13 Dec 2024 09:37:09 +0000

On Mon, Dec 09, 2024 at 02:23:38PM +0000, Russell King (Oracle) wrote:
> Add EEE management to phylink, making use of the phylib implementation.
> This will only be used where a MAC driver populates the methods and
> capabilities bitfield, otherwise we keep our old behaviour.
> 
> Phylink will keep track of the EEE configuration, including the clock
> stop abilities at each end of the MAC to PHY link, programming the PHY
> appropriately and preserving the EEE configuration should the PHY go
> away.
> 
> It will also call into the MAC driver when LPI needs to be enabled or
> disabled, with the expectation that the MAC have LPI disabled during
> probe.
> 
> Support for phylink managed EEE is enabled by populating both tx_lpi
> MAC operations method pointers.
> 
> Signed-off-by: Russell King (Oracle) <[email-masked]>

...

> diff --git a/include/linux/phylink.h b/include/linux/phylink.h

...

> @@ -143,11 +145,17 @@ enum phylink_op_type {
>   *                    possible and avoid stopping it during suspend events.
>   * @default_an_inband: if true, defaults to MLO_AN_INBAND rather than
>   *		       MLO_AN_PHY. A fixed-link specification will override.
> + * @eee_rx_clk_stop_enable: if true, PHY can stop the receive clock during LPI
>   * @get_fixed_state: callback to execute to determine the fixed link state,
>   *		     if MAC link is at %MLO_AN_FIXED mode.
>   * @supported_interfaces: bitmap describing which PHY_INTERFACE_MODE_xxx
>   *                        are supported by the MAC/PCS.
> + * @lpi_interfaces: bitmap describing which PHY interface modes can support
> + *		    LPI signalling.
>   * @mac_capabilities: MAC pause/speed/duplex capabilities.
> + * @lpi_capabilities: MAC speeds which can support LPI signalling
> + * @eee: default EEE configuration.
> + * @lpi_timer_limit_us: Maximum (inclusive) value of the EEE LPI timer.
>   */
>  struct phylink_config {
>  	struct device *dev;
> @@ -156,10 +164,16 @@ struct phylink_config {
>  	bool mac_managed_pm;
>  	bool mac_requires_rxc;
>  	bool default_an_inband;
> +	bool eee_rx_clk_stop_enable;
>  	void (*get_fixed_state)(struct phylink_config *config,
>  				struct phylink_link_state *state);
>  	DECLARE_PHY_INTERFACE_MASK(supported_interfaces);
> +	DECLARE_PHY_INTERFACE_MASK(lpi_interfaces);
>  	unsigned long mac_capabilities;
> +	unsigned long lpi_capabilities;
> +	u32 lpi_timer_limit_us;
> +	u32 lpi_timer_default;
> +	bool eee_enabled_default;
>  };

Hi Russell,

A minor nit from my side.

The Kernel doc updates don't correspond entirely to the structure updates:
- @eee is documented but not present in the structure
- Conversely, @lpi_timer_default and @ee_enabled_default are
  present in the structure but not documented.

...