> -----Original Message-----
> From: Li Zetao <lizetao1@xxxxxxxxxx>
> Sent: Saturday, August 31, 2024 7:44 AM
> To: florian.fainelli@xxxxxxxxxxxx; andrew@xxxxxxx; olteanv@xxxxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; wens@xxxxxxxx; jernej.skrabec@xxxxxxxxx;
> samuel@xxxxxxxxxxxx; heiko@xxxxxxxxx; yisen.zhuang@xxxxxxxxxx;
> salil.mehta@xxxxxxxxxx; hauke@xxxxxxxxxx;
> alexandre.torgue@xxxxxxxxxxx; joabreu@xxxxxxxxxxxx;
> mcoquelin.stm32@xxxxxxxxx; wellslutw@xxxxxxxxx; Pandey, Radhey
> Shyam <radhey.shyam.pandey@xxxxxxx>; Simek, Michal
> <michal.simek@xxxxxxx>; ajay.kathat@xxxxxxxxxxxxx;
> claudiu.beznea@xxxxxxxxx; kvalo@xxxxxxxxxx; lizetao1@xxxxxxxxxx;
> u.kleine-koenig@xxxxxxxxxxxxxx; jacky_chou@xxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> sunxi@xxxxxxxxxxxxxxx; linux-rockchip@xxxxxxxxxxxxxxxxxxx; linux-stm32@st-
> md-mailman.stormreply.com; linux-wireless@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxx>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
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
