On 03/19/2018 11:34 AM, Simon Horman wrote: > On Mon, Mar 19, 2018 at 10:54:47AM +0100, Marek Vasut wrote: >> On 03/19/2018 09:44 AM, Simon Horman wrote: >>> On Sun, Mar 18, 2018 at 11:52:09AM +0100, Marek Vasut wrote: >>>> From: Dien Pham <[email protected]> >>>> >>>> The controller clock can be switched off during suspend/resume, >>>> let runtime PM take care of that. >>>> >>>> Signed-off-by: Dien Pham <[email protected]> >>>> Signed-off-by: Hien Dang <[email protected]> >>>> Signed-off-by: Marek Vasut <[email protected]> >>>> Cc: Geert Uytterhoeven <[email protected]> >>>> Cc: Lorenzo Pieralisi <[email protected]> >>>> Cc: Phil Edworthy <[email protected]> >>>> Cc: Simon Horman <[email protected]> >>>> Cc: Wolfram Sang <[email protected]> >>>> Cc: [email protected] >>>> To: [email protected] >>>> --- >>>> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the >>>> reordering of function calls in probe >>>> - Dispose of fail_clk in rcar_pcie_get_resources() >>>> V3: - Fix up the failpath in probe function >>>> V4: - Rebase on recent linux-next >>>> --- >>>> drivers/pci/host/pcie-rcar.c | 40 ++++++++++++---------------------------- >>>> 1 file changed, 12 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c >>>> index b4c4aad2cf66..93d59f15c589 100644 >>>> --- a/drivers/pci/host/pcie-rcar.c >>>> +++ b/drivers/pci/host/pcie-rcar.c >>>> @@ -142,7 +142,6 @@ struct rcar_pcie { >>>> void __iomem *base; >>>> struct list_head resources; >>>> int root_bus_nr; >>>> - struct clk *clk; >>>> struct clk *bus_clk; >>>> struct rcar_msi msi; >>>> }; >>>> @@ -914,24 +913,14 @@ static int rcar_pcie_get_resources(struct rcar_pcie >>>> *pcie) >>>> if (IS_ERR(pcie->base)) >>>> return PTR_ERR(pcie->base); >>>> >>>> - pcie->clk = devm_clk_get(dev, "pcie"); >>>> - if (IS_ERR(pcie->clk)) { >>>> - dev_err(dev, "cannot get platform clock\n"); >>>> - return PTR_ERR(pcie->clk); >>>> - } >>>> - err = clk_prepare_enable(pcie->clk); >>>> - if (err) >>>> - return err; >>>> - >>>> pcie->bus_clk = devm_clk_get(dev, "pcie_bus"); >>>> if (IS_ERR(pcie->bus_clk)) { >>>> dev_err(dev, "cannot get pcie bus clock\n"); >>>> - err = PTR_ERR(pcie->bus_clk); >>>> - goto fail_clk; >>>> + return PTR_ERR(pcie->bus_clk); >>>> } >>>> err = clk_prepare_enable(pcie->bus_clk); >>>> if (err) >>>> - goto fail_clk; >>>> + return err; >>>> >>>> i = irq_of_parse_and_map(dev->of_node, 0); >>>> if (!i) { >>>> @@ -953,8 +942,6 @@ static int rcar_pcie_get_resources(struct rcar_pcie >>>> *pcie) >>>> >>>> err_map_reg: >>>> clk_disable_unprepare(pcie->bus_clk); >>>> -fail_clk: >>>> - clk_disable_unprepare(pcie->clk); >>>> >>>> return err; >>>> } >>>> @@ -1124,22 +1111,22 @@ static int rcar_pcie_probe(struct platform_device >>>> *pdev) >>>> if (err) >>>> goto err_free_bridge; >>> >>> This error path now calls pci_free_resource_list() and in this case >>> the path is taken if rcar_pcie_parse_request_of_pci_ranges() fails. >>> Is that ok? >> >> I think so, or did I miss something obvious? > > I feel that I am the one missing something obvious. > But here is what I am worried about: > > 1. rcar_pcie_probe() calls rcar_pcie_parse_request_of_pci_ranges() > 2. rcar_pcie_parse_request_of_pci_ranges() calls > devm_request_pci_bus_resources() but that fails so > pci_free_resource_list() is called and an error is returned. > 3. rcar_pcie_probe() branches to err_free_bridge and calls > pci_free_resource_list() again.
I see what you mean now, thanks for the details explanation. So the change needed here is to move pci_free_resource_list(&pcie->resources); above err_free_bridge, so it's not called twice, right ? -- Best regards, Marek Vasut
