lkml.org 
[lkml]   [2019]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCHv5 1/6] PCI: mobiveil: Refactor Mobiveil PCIe Host Bridge IP driver
Date
Hi Arnd,

Thanks a lot for your comments!

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 2019年4月12日 22:39
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> l.subrahmanya@mobiveil.co.in; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; lorenzo.pieralisi@arm.com;
> catalin.marinas@arm.com; will.deacon@arm.com; M.h. Lian
> <minghuan.lian@nxp.com>; Xiaowei Bao <xiaowei.bao@nxp.com>; Mingkai
> Hu <mingkai.hu@nxp.com>
> 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@nxp.com> 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
\
 
 \ /
  Last update: 2019-04-30 02:25    [W:1.147 / U:0.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site