[PATCH net-next v2 07/23] net: stmmac: dwmac-intel-plat: use devm_stmmac_probe_config_dt()

[Serge Semin]

On Sat, Sep 16, 2023 at 03:58:13PM +0800, Jisheng Zhang wrote:
> Simplify the driver's probe() function by using the devres
> variant of stmmac_probe_config_dt().
> 
> The calling of stmmac_pltfr_remove() now needs to be switched to
> stmmac_pltfr_remove_no_dt().
> 
> Signed-off-by: Jisheng Zhang <jszhang at kernel.org>
> ---
>  .../stmicro/stmmac/dwmac-intel-plat.c         | 27 +++++++------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
> index d352a14f9d48..d1aec2ca2b42 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel-plat.c
> @@ -85,17 +85,15 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> +	plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
>  	if (IS_ERR(plat_dat)) {
>  		dev_err(&pdev->dev, "dt configuration failed\n");
>  		return PTR_ERR(plat_dat);
>  	}
>  
>  	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> -	if (!dwmac) {
> -		ret = -ENOMEM;
> -		goto err_remove_config_dt;
> -	}
> +	if (!dwmac)
> +		return -ENOMEM;
>  
>  	dwmac->dev = &pdev->dev;
>  	dwmac->tx_clk = NULL;
> @@ -110,10 +108,8 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
>  		/* Enable TX clock */
>  		if (dwmac->data->tx_clk_en) {
>  			dwmac->tx_clk = devm_clk_get(&pdev->dev, "tx_clk");
> -			if (IS_ERR(dwmac->tx_clk)) {
> -				ret = PTR_ERR(dwmac->tx_clk);
> -				goto err_remove_config_dt;
> -			}
> +			if (IS_ERR(dwmac->tx_clk))
> +				return PTR_ERR(dwmac->tx_clk);
>  
>  			clk_prepare_enable(dwmac->tx_clk);
>  
> @@ -126,7 +122,7 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
>  				if (ret) {
>  					dev_err(&pdev->dev,
>  						"Failed to set tx_clk\n");
> -					goto err_remove_config_dt;
> +					return ret;
>  				}
>  			}
>  		}
> @@ -140,7 +136,7 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
>  			if (ret) {
>  				dev_err(&pdev->dev,
>  					"Failed to set clk_ptp_ref\n");
> -				goto err_remove_config_dt;
> +				return ret;
>  			}
>  		}
>  	}
> @@ -158,22 +154,17 @@ static int intel_eth_plat_probe(struct platform_device *pdev)
>  	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>  	if (ret) {

>  		clk_disable_unprepare(dwmac->tx_clk);
> -		goto err_remove_config_dt;
> +		return ret;

Just a general note IMO it's better for maintainability to have the
clean-up-on-error block than reverting the previous changes in the if
body. Thus should you add new functions call before the block in
subject you won't need to add duplicated cleanup functions calls, but
just add new reverting method invocation to the cleanup-on-error path.

-Serge(y)

>  	}
>  
>  	return 0;
> -
> -err_remove_config_dt:
> -	stmmac_remove_config_dt(pdev, plat_dat);
> -
> -	return ret;
>  }
>  
>  static void intel_eth_plat_remove(struct platform_device *pdev)
>  {
>  	struct intel_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
>  
> -	stmmac_pltfr_remove(pdev);
> +	stmmac_pltfr_remove_no_dt(pdev);
>  	clk_disable_unprepare(dwmac->tx_clk);
>  }
>  
> -- 
> 2.40.1
> 
>