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

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:
 <BJXPR01MB085530E2951C3A18D79C75BFE6BD2@BJXPR01MB0855.CHNPR01.prod.partner.outlook.cn>
Date: Wed, 16 Apr 2025 03:14:45 +0000
From: Minda Chen <minda.chen@...rfivetech.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Emil Renner Berthing <kernel@...il.dk>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S . Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre
 Torgue <alexandre.torgue@...s.st.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>, Russell King
	<rmk+kernel@...linux.org.uk>
Subject: Re: [net-next v1] net: stmmac: starfive: Add serdes PHY init/deinit
 function



> 
> > +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.