[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <73e02ee9-6631-4473-8219-d75253aaf0fc@huawei.com>
Date: Tue, 3 Sep 2024 10:27:52 +0800
From: Li Zetao <lizetao1@...wei.com>
To: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....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()
在 2024/9/1 20:28, Pandey, Radhey Shyam 写道:
>> -----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.
Thanks for the reminder, I considered solving this problem by using
devm_add_action_or_reset
Thanks,
Li Zetao.
>
>> 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