[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<MN0PR12MB595396074210F8390ACE409DB7912@MN0PR12MB5953.namprd12.prod.outlook.com>
Date: Sun, 1 Sep 2024 12:28:26 +0000
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To: Li Zetao <lizetao1@...wei.com>, "florian.fainelli@...adcom.com"
<florian.fainelli@...adcom.com>, "andrew@...n.ch" <andrew@...n.ch>,
"olteanv@...il.com" <olteanv@...il.com>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"wens@...e.org" <wens@...e.org>, "jernej.skrabec@...il.com"
<jernej.skrabec@...il.com>, "samuel@...lland.org" <samuel@...lland.org>,
"heiko@...ech.de" <heiko@...ech.de>, "yisen.zhuang@...wei.com"
<yisen.zhuang@...wei.com>, "salil.mehta@...wei.com" <salil.mehta@...wei.com>,
"hauke@...ke-m.de" <hauke@...ke-m.de>, "alexandre.torgue@...s.st.com"
<alexandre.torgue@...s.st.com>, "joabreu@...opsys.com"
<joabreu@...opsys.com>, "mcoquelin.stm32@...il.com"
<mcoquelin.stm32@...il.com>, "wellslutw@...il.com" <wellslutw@...il.com>,
"Simek, Michal" <michal.simek@....com>, "ajay.kathat@...rochip.com"
<ajay.kathat@...rochip.com>, "claudiu.beznea@...on.dev"
<claudiu.beznea@...on.dev>, "kvalo@...nel.org" <kvalo@...nel.org>,
"u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
"jacky_chou@...eedtech.com" <jacky_chou@...eedtech.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-sunxi@...ts.linux.dev"
<linux-sunxi@...ts.linux.dev>, "linux-rockchip@...ts.infradead.org"
<linux-rockchip@...ts.infradead.org>,
"linux-stm32@...md-mailman.stormreply.com"
<linux-stm32@...md-mailman.stormreply.com>, "linux-wireless@...r.kernel.org"
<linux-wireless@...r.kernel.org>
Subject: RE: [PATCH net-next 10/12] net: xilinx: axienet: Convert using
devm_clk_get_optional_enabled() in axienet_probe()
> -----Original Message-----
> From: Li Zetao <lizetao1@...wei.com>
> Sent: Saturday, August 31, 2024 7:44 AM
> To: florian.fainelli@...adcom.com; andrew@...n.ch; olteanv@...il.com;
> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; wens@...e.org; jernej.skrabec@...il.com;
> samuel@...lland.org; heiko@...ech.de; yisen.zhuang@...wei.com;
> salil.mehta@...wei.com; hauke@...ke-m.de;
> alexandre.torgue@...s.st.com; joabreu@...opsys.com;
> mcoquelin.stm32@...il.com; wellslutw@...il.com; Pandey, Radhey
> Shyam <radhey.shyam.pandey@....com>; Simek, Michal
> <michal.simek@....com>; ajay.kathat@...rochip.com;
> claudiu.beznea@...on.dev; kvalo@...nel.org; lizetao1@...wei.com;
> u.kleine-koenig@...gutronix.de; jacky_chou@...eedtech.com
> Cc: netdev@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> sunxi@...ts.linux.dev; linux-rockchip@...ts.infradead.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-wireless@...r.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@...wei.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@....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
Powered by blists - more mailing lists