Hi Jim, On 7/3/24 21:02, Jim Quinlan wrote: > Instead of invoking "clk_disable_unprepare(pcie->clk)" in > a number of error paths, we can just use a "clk_out" label > and goto the label after setting the return value. > > Signed-off-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx> > --- > drivers/pci/controller/pcie-brcmstb.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index c08683febdd4..c2eb29b886f7 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1620,24 +1620,25 @@ static int brcm_pcie_probe(struct platform_device *pdev) > } > pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal"); > if (IS_ERR(pcie->rescal)) { > - clk_disable_unprepare(pcie->clk); > - return PTR_ERR(pcie->rescal); > + ret = PTR_ERR(pcie->rescal); > + goto clk_out; > } > pcie->perst_reset = devm_reset_control_get_optional_exclusive(&pdev->dev, "perst"); > if (IS_ERR(pcie->perst_reset)) { > - clk_disable_unprepare(pcie->clk); > - return PTR_ERR(pcie->perst_reset); > + ret = PTR_ERR(pcie->perst_reset); > + goto clk_out; > } > ret = clk_prepare_enable(pcie->clk); Have you considered enabling pcie->clk clock here ^^^ and avoid jumping to clk_out label? Basically, it'd be easier from error-path POV to get all required resources and just after that start the initialization sequence of pcie controller. ~Stan > ret = reset_control_reset(pcie->rescal); > - if (ret) > + if (ret) { > dev_err(&pdev->dev, "failed to deassert 'rescal'\n"); > + goto clk_out; > + } > > 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 +1677,9 @@ static int brcm_pcie_probe(struct platform_device *pdev) > > return 0; > > +clk_out: > + clk_disable_unprepare(pcie->clk); > + return ret; > fail: > __brcm_pcie_remove(pcie); > return ret;