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

[Minda Chen]

> 
> > +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
OK. I will move it to generic. And I will send the serdes PHY code in next version.