Re: [PATCH RFC 03/18] net: marvell: mvneta: Simplify EEE configuration

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

On Fri, Feb 17, 2023 at 04:42:15AM +0100, Andrew Lunn wrote:
> @@ -4221,10 +4218,8 @@ static void mvneta_mac_link_up(struct phylink_config *config,
>  
>  	mvneta_port_up(pp);
>  
> -	if (phy && pp->eee_enabled) {
> -		pp->eee_active = phy_init_eee(phy, false) >= 0;
> -		mvneta_set_eee(pp, pp->eee_active && pp->tx_lpi_enabled);
> -	}
> +	if (phy)
> +		mvneta_set_eee(pp, phy->eee_active && pp->tx_lpi_enabled);

Thinking about this a bit more, I'm not convinced this is properly safe.
What protects phy->eee_active from changing here? The phydev mutex won't
be held at this point.

As I mentioned in my reply to the cover letter about passing a flag to
mac_link_up() for EEE status, this would mean phylink could save the
EEE active status just like it does with the other phydev parameters
in phylink_phy_change() (which is called under the phydev mutex).

That ensures that we grab all the phylib state under the phydev mutex
and then use that state when bringing the MAC side up without needing
to worry about phydev mutexes.

It's way more invasive, as it requires the mac_link_up() method
signature to change, but I think it would be a cleaner approach
overall.

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