On Fri, Jul 15, 2022 at 08:06:46AM +0800, Liang He wrote: > At 2022-07-15 00:37:51, "Bjorn Helgaas" <helgaas@xxxxxxxxxx> wrote: > >On Mon, Jul 04, 2022 at 02:26:08PM +0800, Liang He wrote: > >> In ls_g4_pcie_probe(), we should hold the reference returned by > >> of_parse_phandle() and use it to call of_node_put() for refcount > >> balance. > >> > >> Fixes: d29ad70a813b ("PCI: mobiveil: Add PCIe Gen4 RC driver for Layerscape SoCs") > >> Co-authored-by: Miaoqian Lin <linmq006@xxxxxxxxx> > >> Signed-off-by: Liang He <windhl@xxxxxxx> > >> --- > >> drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c b/drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c > >> index d7b7350f02dd..075aa487f92e 100644 > >> --- a/drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c > >> +++ b/drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c > >> @@ -204,13 +204,15 @@ static int __init ls_g4_pcie_probe(struct platform_device *pdev) > >> struct pci_host_bridge *bridge; > >> struct mobiveil_pcie *mv_pci; > >> struct ls_g4_pcie *pcie; > >> - struct device_node *np = dev->of_node; > >> + struct device_node *np = dev->of_node, *parse_np; > >> int ret; > >> > >> - if (!of_parse_phandle(np, "msi-parent", 0)) { > >> + parse_np = of_parse_phandle(np, "msi-parent", 0); > > > >I don't understand what's going on here. Where is "msi-parent" > >actually used? If we just need to know whether "msi-parent" exists, > >can we use of_property_read_bool() instead? Or can we call > >of_parse_phandle() closer to where it is used? > > I think this code is used to check the existence of the device_node > whose property is "msi-parent". of_property_read_bool() can only be > used to check current device_node 'np'. > > If this is right, I think we can use a helper to make thing simple > like this: > > https://lore.kernel.org/all/20220712061417.363145-1-windhl@xxxxxxx/ > > If it is ok, I will prepare the helper in new version. I dunno. The helper in that patch has nothing ftgmac100-specific in it, so if the helper is correct and useful, it seems like it should be somewhere generic so we don't end up with a zillion copies of it. Maybe Rob will chime in. I just want drivers/pci/controller/* to deal with "msi-parent" consistently. dw_pcie_host_init() uses of_property_read_bool(). ls_g4_pcie_probe(), brcm_pcie_probe(), and iproc_pcie_msi_enable() use of_parse_phandle(). It's not clear to me that these uses are fundamentally different and need to use different interfaces. > >> + if (!parse_np) { > >> dev_err(dev, "Failed to find msi-parent\n"); > >> return -EINVAL; > >> } > >> + of_node_put(parse_np); > >> > >> bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > >> if (!bridge) > >> -- > >> 2.25.1 > >>