netdev - 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: <840346cf-52a3-4e40-bf38-9d66231366d7@lunn.ch>
Date: Tue, 15 Apr 2025 14:08:55 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Minda Chen <minda.chen@...rfivetech.com>
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, linux-kernel@...r.kernel.org,
	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