net: stmmac: starfive: Add serdes PHY init/deinit function

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

Posted by Minda Chen 8 months, 1 week ago

To support SGMII interface, add internal serdes PHY powerup/
powerdown function.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../ethernet/stmicro/stmmac/dwmac-starfive.c  | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index 2013d7477eb7..f5923f847100 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -9,6 +9,8 @@
 
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/property.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
@@ -28,6 +30,7 @@ struct starfive_dwmac_data {
 struct starfive_dwmac {
 	struct device *dev;
 	const struct starfive_dwmac_data *data;
+	struct phy *serdes_phy;
 };
 
 static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
@@ -80,6 +83,26 @@ static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
 	return 0;
 }
 
+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 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 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");
+
 	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;
+	}
+
 	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 }
 
-- 
2.17.1

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

Posted by Andrew Lunn 8 months, 1 week ago

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

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

Posted by Paolo Abeni 8 months, 1 week ago

On 4/10/25 9:04 AM, Minda Chen wrote:
> To support SGMII interface, add internal serdes PHY powerup/
> powerdown function.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-starfive.c  | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> index 2013d7477eb7..f5923f847100 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -9,6 +9,8 @@
>  
>  #include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
>  #include <linux/property.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> @@ -28,6 +30,7 @@ struct starfive_dwmac_data {
>  struct starfive_dwmac {
>  	struct device *dev;
>  	const struct starfive_dwmac_data *data;
> +	struct phy *serdes_phy;
>  };
>  
>  static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
> @@ -80,6 +83,26 @@ static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
>  	return 0;
>  }
>  
> +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;

This is called also in case of PM suspend/resume. Do you need to keep
the init here, or should that moved at probe time only? Similar question
for phy_exit() below.

Thanks!

Paolo