Re: [PATCH v4 03/12] PCI: brcmstb: Use common error handling code in brcm_pcie_probe()
From: Manivannan Sadhasivam <hidden>
Date: 2024-07-25 04:31:17
Also in:
linux-pci, lkml
On Tue, Jul 16, 2024 at 05:31:18PM -0400, Jim Quinlan wrote:
quoted hunk ↗ jump to hunk
o Move the clk_prepare_enable() below the resource allocations. o Add a jump target (clk_out) so that a bit of exception handling can be better reused at the end of this function implementation. Signed-off-by: Jim Quinlan <redacted> Reviewed-by: Stanimir Varbanov <redacted> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> --- drivers/pci/controller/pcie-brcmstb.c | 29 +++++++++++++++------------ 1 file changed, 16 insertions(+), 13 deletions(-)diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index c08683febdd4..c257434edc08 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c@@ -1613,31 +1613,30 @@ static int brcm_pcie_probe(struct platform_device *pdev) pcie->ssc = of_property_read_bool(np, "brcm,enable-ssc"); - ret = clk_prepare_enable(pcie->clk); - if (ret) { - dev_err(&pdev->dev, "could not enable clock\n"); - return ret; - } pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal"); - if (IS_ERR(pcie->rescal)) { - clk_disable_unprepare(pcie->clk); + if (IS_ERR(pcie->rescal)) return PTR_ERR(pcie->rescal); - } + pcie->perst_reset = devm_reset_control_get_optional_exclusive(&pdev->dev, "perst"); - if (IS_ERR(pcie->perst_reset)) { - clk_disable_unprepare(pcie->clk); + if (IS_ERR(pcie->perst_reset)) return PTR_ERR(pcie->perst_reset); + + ret = clk_prepare_enable(pcie->clk); + if (ret) { + dev_err(&pdev->dev, "could not enable clock\n"); + return ret; } ret = reset_control_reset(pcie->rescal); - if (ret) + if (ret) { dev_err(&pdev->dev, "failed to deassert 'rescal'\n"); + goto clk_out;
Please use a descriptive name for the err labels. Here this err path disables and unprepares the clk, so use 'clk_disable_unprepare'.
quoted hunk ↗ jump to hunk
+ } ret = brcm_phy_start(pcie); if (ret) { reset_control_rearm(pcie->rescal); - clk_disable_unprepare(pcie->clk); - return ret; + goto clk_out; } ret = brcm_pcie_setup(pcie);@@ -1676,6 +1675,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) return 0; +clk_out: + clk_disable_unprepare(pcie->clk); + return ret; +
This is leaking the resources. Move this new label below 'fail'. - Mani -- மணிவண்ணன் சதாசிவம்