> From: "Russell King (Oracle)" <rmk+kernel@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> [rgantois: removed second parameters of new callbacks]
> Signed-off-by: Romain Gantois <romain.gantois@xxxxxxxxxxx>
> ---
> .../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@xxxxxxxxxxx>
