Re: [PATCHv5 1/6] PCI: mobiveil: Refactor Mobiveil PCIe Host Bridge IP driver

[Arnd Bergmann]

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.

> +
> +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.

     Arnd