[PATCH net-next 10/12] net: xilinx: axienet: Convert using devm_clk_get_optional_enabled() in axienet_probe()

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Li Zetao <lizetao1 at huawei.com>
> Sent: Saturday, August 31, 2024 7:44 AM
> To: florian.fainelli at broadcom.com; andrew at lunn.ch; olteanv at gmail.com;
> davem at davemloft.net; edumazet at google.com; kuba at kernel.org;
> pabeni at redhat.com; wens at csie.org; jernej.skrabec at gmail.com;
> samuel at sholland.org; heiko at sntech.de; yisen.zhuang at huawei.com;
> salil.mehta at huawei.com; hauke at hauke-m.de;
> alexandre.torgue at foss.st.com; joabreu at synopsys.com;
> mcoquelin.stm32 at gmail.com; wellslutw at gmail.com; Pandey, Radhey
> Shyam <radhey.shyam.pandey at amd.com>; Simek, Michal
> <michal.simek at amd.com>; ajay.kathat at microchip.com;
> claudiu.beznea at tuxon.dev; kvalo at kernel.org; lizetao1 at huawei.com;
> u.kleine-koenig at pengutronix.de; jacky_chou at aspeedtech.com
> Cc: netdev at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> sunxi at lists.linux.dev; linux-rockchip at lists.infradead.org; linux-stm32 at st-
> md-mailman.stormreply.com; linux-wireless at vger.kernel.org
> Subject: [PATCH net-next 10/12] net: xilinx: axienet: Convert using
> devm_clk_get_optional_enabled() in axienet_probe()
> 
> Use devm_clk_get_optional_enabled() instead of devm_clk_get_optional() +
> clk_prepare_enable(), which can make the clk consistent with the device life
> cycle and reduce the risk of unreleased clk resources. Since the device
> framework has automatically released the clk resource, there is no need to
> execute clk_disable_unprepare(clk) on the error path.
> 
> Signed-off-by: Li Zetao <lizetao1 at huawei.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey at amd.com>
Thanks!

> ---
>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index fe6a0e2e463f..48b41e95aa74 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -2584,22 +2584,17 @@ static int axienet_probe(struct platform_device
> *pdev)
>  	seqcount_mutex_init(&lp->hw_stats_seqcount, &lp->stats_lock);
>  	INIT_DEFERRABLE_WORK(&lp->stats_work, axienet_refresh_stats);
> 
> -	lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk");
> -	if (!lp->axi_clk) {
> +	lp->axi_clk = devm_clk_get_optional_enabled(&pdev->dev,
> "s_axi_lite_clk");
> +	if (!lp->axi_clk)
>  		/* For backward compatibility, if named AXI clock is not
> present,
>  		 * treat the first clock specified as the AXI clock.
>  		 */
> -		lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL);
> -	}
> +		lp->axi_clk = devm_clk_get_optional_enabled(&pdev->dev,
> NULL);
> +
>  	if (IS_ERR(lp->axi_clk)) {
>  		ret = PTR_ERR(lp->axi_clk);
>  		goto free_netdev;
>  	}
> -	ret = clk_prepare_enable(lp->axi_clk);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n",
> ret);
> -		goto free_netdev;
> -	}
> 
>  	lp->misc_clks[0].id = "axis_clk";
>  	lp->misc_clks[1].id = "ref_clk";
> @@ -2915,7 +2910,6 @@ static int axienet_probe(struct platform_device
> *pdev)
>  		axienet_mdio_teardown(lp);
>  cleanup_clk:

I also find that there is goto to cleanup_clk when devm_clk_bulk_get_optional/
clk_bulk_prepare_enable fails which is not correct but as it is existing bug it 
can go a separate patch.

>  	clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp-
> >misc_clks);
> -	clk_disable_unprepare(lp->axi_clk);
> 
>  free_netdev:
>  	free_netdev(ndev);
> @@ -2939,7 +2933,6 @@ static void axienet_remove(struct platform_device
> *pdev)
>  	axienet_mdio_teardown(lp);
> 
>  	clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp-
> >misc_clks);
> -	clk_disable_unprepare(lp->axi_clk);
> 
>  	free_netdev(ndev);
>  }
> --
> 2.34.1