Re: [net-next v1] net: stmmac: starfive: Add serdes PHY init/deinit function

[Andrew Lunn <andrew@lunn.ch>]

> +static int starfive_dwmac_serdes_powerup(struct net_device *ndev, void *priv)
> +{
> +	struct starfive_dwmac *dwmac = priv;
> +	int ret;
> +
> +	ret = phy_init(dwmac->serdes_phy);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(dwmac->serdes_phy);
> +}

static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
{
        struct qcom_ethqos *ethqos = priv;
        int ret;

        ret = phy_init(ethqos->serdes_phy);
        if (ret)
                return ret;

        ret = phy_power_on(ethqos->serdes_phy);
        if (ret)
                return ret;

        return phy_set_speed(ethqos->serdes_phy, ethqos->speed);
}

Similar?

> +static void starfive_dwmac_serdes_powerdown(struct net_device *ndev, void *priv)
> +{
> +	struct starfive_dwmac *dwmac = priv;
> +
> +	phy_power_off(dwmac->serdes_phy);
> +	phy_exit(dwmac->serdes_phy);
> +}

static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
{
        struct qcom_ethqos *ethqos = priv;

        phy_power_off(ethqos->serdes_phy);
        phy_exit(ethqos->serdes_phy);
}

Pretty much cut & paste.

>  static int starfive_dwmac_probe(struct platform_device *pdev)
>  {
>  	struct plat_stmmacenet_data *plat_dat;
> @@ -102,6 +125,11 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>  	if (!dwmac)
>  		return -ENOMEM;
>  
> +	dwmac->serdes_phy = devm_phy_optional_get(&pdev->dev, NULL);
> +	if (IS_ERR(dwmac->serdes_phy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->serdes_phy),
> +				     "Failed to get serdes phy\n");
> +

        ethqos->serdes_phy = devm_phy_optional_get(dev, "serdes");
        if (IS_ERR(ethqos->serdes_phy))
                return dev_err_probe(dev, PTR_ERR(ethqos->serdes_phy),
                                     "Failed to get serdes phy\n");


>  	dwmac->data = device_get_match_data(&pdev->dev);
>  
>  	plat_dat->clk_tx_i = devm_clk_get_enabled(&pdev->dev, "tx");
> @@ -132,6 +160,11 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	if (dwmac->serdes_phy) {
> +		plat_dat->serdes_powerup = starfive_dwmac_serdes_powerup;
> +		plat_dat->serdes_powerdown  = starfive_dwmac_serdes_powerdown;
> +	}
> +

        if (ethqos->serdes_phy) {
                plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
                plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
        }


I assume you have seen all the work Russell King has been doing
recently cleaning up all the copy/paste code between various glue
drivers. Please don't add to that mess. Please consider how you can
refactor the ethqos code to make is generic for any stmmac driver
which has a generic phy.

    Andrew

---
pw-bot: cr