Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Russell King (Oracle)]

On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)

There shouldn't be any need to make this conditional.

> +static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
> +			      phy_interface_t interface)
> +{
> +	struct net_device *ndev = to_net_dev(config->dev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int ret = 0;
> +
> +	priv->plat->phy_interface = interface;
> +
> +	if (priv->plat->config_serdes)
> +		ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);

Please call this "phylink_mac_prepare" and pass the parameters that
phylink passes you to this function, so we don't end up at a later
date with people needing to extend this function to do other stuff,
thus repeating mistakes from earlier.

This is what has led to some very yucky code in all those
"fix_mac_speed" implementations, with duplicated data in the BSPs
to get the PHY mode and store it separately, etc.

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