Hi Arnd, Thanks a lot for your comments! > -----Original Message----- > From: Arnd Bergmann [mailto:arnd@xxxxxxxx] > Sent: 2019年4月12日 22:39 > To: Z.q. Hou <zhiqiang.hou@xxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > bhelgaas@xxxxxxxxxx; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; > l.subrahmanya@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; Leo Li > <leoyang.li@xxxxxxx>; lorenzo.pieralisi@xxxxxxx; > catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; M.h. Lian > <minghuan.lian@xxxxxxx>; Xiaowei Bao <xiaowei.bao@xxxxxxx>; Mingkai > Hu <mingkai.hu@xxxxxxx> > Subject: [EXT] Re: [PATCHv5 1/6] PCI: mobiveil: Refactor Mobiveil PCIe Host > Bridge IP driver > > WARNING: This email was created outside of NXP. DO NOT CLICK links or > attachments unless you recognize the sender and know the content is safe. > > > > On Fri, Apr 12, 2019 at 11:53 AM Z.q. Hou <zhiqiang.hou@xxxxxxx> wrote: > > > +int mobiveil_pcie_host_probe(struct mobiveil_pcie *pcie) > > { > > - struct mobiveil_pcie *pcie; > > struct pci_bus *bus; > > struct pci_bus *child; > > struct pci_host_bridge *bridge; > > - struct device *dev = &pdev->dev; > > + struct device *dev = &pcie->pdev->dev; > > resource_size_t iobase; > > int ret; > > > > - /* allocate the PCIe port */ > > - bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > > - if (!bridge) > > - return -ENOMEM; > > - > > - pcie = pci_host_bridge_priv(bridge); > > - > > - pcie->pdev = pdev; > > + INIT_LIST_HEAD(&pcie->resources); > > > > ret = mobiveil_pcie_parse_dt(pcie); > > if (ret) { > > @@ -928,7 +560,10 @@ static int mobiveil_pcie_probe(struct > platform_device *pdev) > > return ret; > > } > > > > - INIT_LIST_HEAD(&pcie->resources); > > + /* allocate the PCIe port */ > > + bridge = devm_pci_alloc_host_bridge(dev, 0); > > + if (!bridge) > > + return -ENOMEM; > > > > > +static int mobiveil_pcie_probe(struct platform_device *pdev) { > > + struct mobiveil_pcie *pcie; > > + struct device *dev = &pdev->dev; > > + > > + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > > + if (!pcie) > > + return -ENOMEM; > > + > > + pcie->pdev = pdev; > > + > > + return mobiveil_pcie_host_probe(pcie); } > > I think you need to pull the allocation of the host structure out into the main > driver here, to keep both allocations together, otherwise the release function > will free the mobiveil_pcie structure before freeing the pci_host_bridge if that > is still in use. Yes, that will be arranged in v6. > > > + > > +struct mobiveil_pcie { > > + struct platform_device *pdev; > > + struct list_head resources; > > These two should not be needed here, as they are already part of the > pci_host_bridge structure. The resources list structure has been changed to a pointer to the bridge->windows. It is better to keep these 2 pointers in the private structure to make it easy to use them in the PCIe controller driver. > > Arnd Regards, Zhiqiang
