netdev - Re: [PATCH v4 net-next 7/9] net: phy: Add phy_support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6951e7fa-a922-c498-9bb9-eaae5f47faaf@gmail.com>
Date: Mon, 19 Jun 2023 16:21:09 +0100
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>
Cc: Russell King <rmk+kernel@...linux.org.uk>,
 Heiner Kallweit <hkallweit1@...il.com>,
 Oleksij Rempel <linux@...pel-privat.de>
Subject: Re: [PATCH v4 net-next 7/9] net: phy: Add phy_support_eee()
 indicating MAC support EEE



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@...n.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);

-- 
Florian