Russell,
On Sat, 03 Oct 2015 19:13:02 +0100, Russell King wrote:
> + ret = devm_add_action(dev, mvebu_pcie_port_clk_put, port);
I didn't know about devm_add_action(). Definitely very useful for such
situations.
> @@ -1052,10 +1086,12 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
> struct mvebu_pcie_port *port = &pcie->ports[i];
>
> ret = mvebu_pcie_parse_port(pcie, port, child);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(child);
> return ret;
> - else if (ret == 0)
> + } else if (ret == 0) {
> continue;
> + }
This is not trivial. If I understand correctly,
for_each_available_child_of_node() will automatically release the
reference on the previous node and take the reference on the new one
before entering the loop code. So in the skipping case, we don't need
to release the reference as it will be done by the next iteration of
the loop, but in the error case, since we are unexpectedly breaking the
loop, we need to do it manually.
The sort of tricky thing that should be documented near
for_each_child_of_node(), since I believe a lot of code gets this
wrong. See:
http://lxr.free-electrons.com/source/arch/arm/mach-shmobile/pm-rmobile.c#L367
http://lxr.free-electrons.com/source/drivers/phy/phy-miphy365x.c#L564
and many more.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
