diff mbox series

[net-next,v7,5/7] net: stmmac: dwmac-socfpga: use pcs_init/pcs_exit

Message ID 20240513-rzn1-gmac1-v7-5-6acf58b5440d@bootlin.com (mailing list archive)
State Accepted
Commit 81b418a6565757128afc6f74dd997598f8adb1b2
Delegated to: Netdev Maintainers
Headers show
Series net: stmmac: Add support for RZN1 GMAC devices | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 925 this patch: 925
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 936 this patch: 936
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 936 this patch: 936
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 137 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-14--00-00 (tests: 1022)

Commit Message

Romain Gantois May 13, 2024, 7:25 a.m. UTC 
From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>

Use the newly introduced pcs_init() and pcs_exit() operations to
create and destroy the PCS instance at a more appropriate moment during
the driver lifecycle, thereby avoiding publishing a network device to
userspace that has not yet finished its PCS initialisation.

There are other similar issues with this driver which remain
unaddressed, but these are out of scope for this patch.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
[rgantois: removed second parameters of new callbacks]
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 107 ++++++++++-----------
 1 file changed, 53 insertions(+), 54 deletions(-)

Comments

Hariprasad Kelam May 13, 2024, 10:10 a.m. UTC | #1 
> From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
> 
> Use the newly introduced pcs_init() and pcs_exit() operations to create and
> destroy the PCS instance at a more appropriate moment during the driver
> lifecycle, thereby avoiding publishing a network device to userspace that has
> not yet finished its PCS initialisation.
> 
> There are other similar issues with this driver which remain unaddressed, but
> these are out of scope for this patch.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> [rgantois: removed second parameters of new callbacks]
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 107 ++++++++++-------
> ----
>  1 file changed, 53 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index 12b4a80ea3aa1..b3d45f9dfb556 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -379,6 +379,56 @@ static int socfpga_gen10_set_phy_mode(struct
> socfpga_dwmac *dwmac)
>  	return 0;
>  }
> 
> +static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv) {
> +	struct socfpga_dwmac *dwmac = priv->plat->bsp_priv;
> +	struct regmap_config pcs_regmap_cfg = {
> +		.reg_bits = 16,
> +		.val_bits = 16,
> +		.reg_shift = REGMAP_UPSHIFT(1),
> +	};
> +	struct mdio_regmap_config mrc;
> +	struct regmap *pcs_regmap;
> +	struct phylink_pcs *pcs;
> +	struct mii_bus *pcs_bus;
> +
> +	if (!dwmac->tse_pcs_base)
> +		return 0;
> +
> +	pcs_regmap = devm_regmap_init_mmio(priv->device, dwmac-
> >tse_pcs_base,
> +					   &pcs_regmap_cfg);
> +	if (IS_ERR(pcs_regmap))
> +		return PTR_ERR(pcs_regmap);
> +
> +	memset(&mrc, 0, sizeof(mrc));
> +	mrc.regmap = pcs_regmap;
> +	mrc.parent = priv->device;
> +	mrc.valid_addr = 0x0;
> +	mrc.autoscan = false;
> +
> +	/* Can't use ndev->name here because it will not have been
> initialised,
> +	 * and in any case, the user can rename network interfaces at
> runtime.
> +	 */
> +	snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii",
> +		 dev_name(priv->device));
> +	pcs_bus = devm_mdio_regmap_register(priv->device, &mrc);
> +	if (IS_ERR(pcs_bus))
> +		return PTR_ERR(pcs_bus);
> +
> +	pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
> +	if (IS_ERR(pcs))
> +		return PTR_ERR(pcs);
> +
> +	priv->hw->phylink_pcs = pcs;
> +	return 0;
> +}
> +
> +static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv) {
> +	if (priv->hw->phylink_pcs)
> +		lynx_pcs_destroy(priv->hw->phylink_pcs);
> +}
> +
>  static int socfpga_dwmac_probe(struct platform_device *pdev)  {
>  	struct plat_stmmacenet_data *plat_dat; @@ -426,6 +476,8 @@ static
> int socfpga_dwmac_probe(struct platform_device *pdev)
>  	dwmac->ops = ops;
>  	plat_dat->bsp_priv = dwmac;
>  	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
> +	plat_dat->pcs_init = socfpga_dwmac_pcs_init;
> +	plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
> 
>  	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>  	if (ret)
> @@ -444,48 +496,6 @@ static int socfpga_dwmac_probe(struct
> platform_device *pdev)
>  	if (ret)
>  		goto err_dvr_remove;
> 
> -	/* Create a regmap for the PCS so that it can be used by the PCS
> driver,
> -	 * if we have such a PCS
> -	 */
> -	if (dwmac->tse_pcs_base) {
> -		struct regmap_config pcs_regmap_cfg;
> -		struct mdio_regmap_config mrc;
> -		struct regmap *pcs_regmap;
> -		struct mii_bus *pcs_bus;
> -
> -		memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
> -		memset(&mrc, 0, sizeof(mrc));
> -
> -		pcs_regmap_cfg.reg_bits = 16;
> -		pcs_regmap_cfg.val_bits = 16;
> -		pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
> -
> -		pcs_regmap = devm_regmap_init_mmio(&pdev->dev,
> dwmac->tse_pcs_base,
> -						   &pcs_regmap_cfg);
> -		if (IS_ERR(pcs_regmap)) {
> -			ret = PTR_ERR(pcs_regmap);
> -			goto err_dvr_remove;
> -		}
> -
> -		mrc.regmap = pcs_regmap;
> -		mrc.parent = &pdev->dev;
> -		mrc.valid_addr = 0x0;
> -		mrc.autoscan = false;
> -
> -		snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev-
> >name);
> -		pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
> -		if (IS_ERR(pcs_bus)) {
> -			ret = PTR_ERR(pcs_bus);
> -			goto err_dvr_remove;
> -		}
> -
> -		stpriv->hw->phylink_pcs = lynx_pcs_create_mdiodev(pcs_bus,
> 0);
> -		if (IS_ERR(stpriv->hw->phylink_pcs)) {
> -			ret = PTR_ERR(stpriv->hw->phylink_pcs);
> -			goto err_dvr_remove;
> -		}
> -	}
> -
>  	return 0;
> 
>  err_dvr_remove:
> @@ -494,17 +504,6 @@ static int socfpga_dwmac_probe(struct
> platform_device *pdev)
>  	return ret;
>  }
> 
> -static void socfpga_dwmac_remove(struct platform_device *pdev) -{
> -	struct net_device *ndev = platform_get_drvdata(pdev);
> -	struct stmmac_priv *priv = netdev_priv(ndev);
> -	struct phylink_pcs *pcs = priv->hw->phylink_pcs;
> -
> -	stmmac_pltfr_remove(pdev);
> -
> -	lynx_pcs_destroy(pcs);
> -}
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int socfpga_dwmac_resume(struct device *dev)  { @@ -576,7 +575,7
> @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
> 
>  static struct platform_driver socfpga_dwmac_driver = {
>  	.probe  = socfpga_dwmac_probe,
> -	.remove_new = socfpga_dwmac_remove,
> +	.remove_new = stmmac_pltfr_remove,
>  	.driver = {
>  		.name           = "socfpga-dwmac",
>  		.pm		= &socfpga_dwmac_pm_ops,
> 
> --
> 2.44.0
> 
Reviewed-by: Hariprasad Kelam <hkelam@marvell.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 12b4a80ea3aa1..b3d45f9dfb556 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -379,6 +379,56 @@  static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac)
 	return 0;
 }
 
+static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv)
+{
+	struct socfpga_dwmac *dwmac = priv->plat->bsp_priv;
+	struct regmap_config pcs_regmap_cfg = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.reg_shift = REGMAP_UPSHIFT(1),
+	};
+	struct mdio_regmap_config mrc;
+	struct regmap *pcs_regmap;
+	struct phylink_pcs *pcs;
+	struct mii_bus *pcs_bus;
+
+	if (!dwmac->tse_pcs_base)
+		return 0;
+
+	pcs_regmap = devm_regmap_init_mmio(priv->device, dwmac->tse_pcs_base,
+					   &pcs_regmap_cfg);
+	if (IS_ERR(pcs_regmap))
+		return PTR_ERR(pcs_regmap);
+
+	memset(&mrc, 0, sizeof(mrc));
+	mrc.regmap = pcs_regmap;
+	mrc.parent = priv->device;
+	mrc.valid_addr = 0x0;
+	mrc.autoscan = false;
+
+	/* Can't use ndev->name here because it will not have been initialised,
+	 * and in any case, the user can rename network interfaces at runtime.
+	 */
+	snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii",
+		 dev_name(priv->device));
+	pcs_bus = devm_mdio_regmap_register(priv->device, &mrc);
+	if (IS_ERR(pcs_bus))
+		return PTR_ERR(pcs_bus);
+
+	pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
+	if (IS_ERR(pcs))
+		return PTR_ERR(pcs);
+
+	priv->hw->phylink_pcs = pcs;
+	return 0;
+}
+
+static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv)
+{
+	if (priv->hw->phylink_pcs)
+		lynx_pcs_destroy(priv->hw->phylink_pcs);
+}
+
 static int socfpga_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -426,6 +476,8 @@  static int socfpga_dwmac_probe(struct platform_device *pdev)
 	dwmac->ops = ops;
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
+	plat_dat->pcs_init = socfpga_dwmac_pcs_init;
+	plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
@@ -444,48 +496,6 @@  static int socfpga_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_dvr_remove;
 
-	/* Create a regmap for the PCS so that it can be used by the PCS driver,
-	 * if we have such a PCS
-	 */
-	if (dwmac->tse_pcs_base) {
-		struct regmap_config pcs_regmap_cfg;
-		struct mdio_regmap_config mrc;
-		struct regmap *pcs_regmap;
-		struct mii_bus *pcs_bus;
-
-		memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
-		memset(&mrc, 0, sizeof(mrc));
-
-		pcs_regmap_cfg.reg_bits = 16;
-		pcs_regmap_cfg.val_bits = 16;
-		pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
-
-		pcs_regmap = devm_regmap_init_mmio(&pdev->dev, dwmac->tse_pcs_base,
-						   &pcs_regmap_cfg);
-		if (IS_ERR(pcs_regmap)) {
-			ret = PTR_ERR(pcs_regmap);
-			goto err_dvr_remove;
-		}
-
-		mrc.regmap = pcs_regmap;
-		mrc.parent = &pdev->dev;
-		mrc.valid_addr = 0x0;
-		mrc.autoscan = false;
-
-		snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
-		pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
-		if (IS_ERR(pcs_bus)) {
-			ret = PTR_ERR(pcs_bus);
-			goto err_dvr_remove;
-		}
-
-		stpriv->hw->phylink_pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
-		if (IS_ERR(stpriv->hw->phylink_pcs)) {
-			ret = PTR_ERR(stpriv->hw->phylink_pcs);
-			goto err_dvr_remove;
-		}
-	}
-
 	return 0;
 
 err_dvr_remove:
@@ -494,17 +504,6 @@  static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static void socfpga_dwmac_remove(struct platform_device *pdev)
-{
-	struct net_device *ndev = platform_get_drvdata(pdev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
-	struct phylink_pcs *pcs = priv->hw->phylink_pcs;
-
-	stmmac_pltfr_remove(pdev);
-
-	lynx_pcs_destroy(pcs);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int socfpga_dwmac_resume(struct device *dev)
 {
@@ -576,7 +575,7 @@  MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
 
 static struct platform_driver socfpga_dwmac_driver = {
 	.probe  = socfpga_dwmac_probe,
-	.remove_new = socfpga_dwmac_remove,
+	.remove_new = stmmac_pltfr_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
 		.pm		= &socfpga_dwmac_pm_ops,