Re: [PATCHv9 02/12] PCI: mobiveil: Move the host initialization into a routine

Andrew Murray
2020-01-13T10:19:16Z

On Wed, Nov 20, 2019 at 03:45:30AM +0000, Z.q. Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>
> Move the host initialization related operations into a new
> routine to make it can be reused by other incoming platform's

s/to make/such that/

'function' is probably a better word than 'routine'.

> PCIe host driver, in which the Mobiveil GPEX is integrated.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> V9:
> - New patch splited from the #1 of V8 patches to make it easy to review.
>
> drivers/pci/controller/pcie-mobiveil.c | 38 +++++++++++++++-----------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index 5fd26e376af2..97f682ca7c7a 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -873,27 +873,15 @@ static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> return 0;
> }
>
> -static int mobiveil_pcie_probe(struct platform_device *pdev)
> +int mobiveil_pcie_host_probe(struct mobiveil_pcie *pcie)

This is no longer static - but do you need to add a header file somewhere?

> {
> - struct mobiveil_pcie *pcie;
> + struct root_port *rp = &pcie->rp;
> + struct pci_host_bridge *bridge = rp->bridge;
> + struct device *dev = &pcie->pdev->dev;
> struct pci_bus *bus;
> struct pci_bus *child;
> - struct pci_host_bridge *bridge;
> - struct device *dev = &pdev->dev;
> - struct root_port *rp;
> 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);
> - rp = &pcie->rp;
> - rp->bridge = bridge;
> -
> - pcie->pdev = pdev;
> -
> ret = mobiveil_pcie_parse_dt(pcie);
> if (ret) {
> dev_err(dev, "Parsing DT failed, ret: %x\n", ret);
> @@ -956,6 +944,24 @@ static int mobiveil_pcie_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int mobiveil_pcie_probe(struct platform_device *pdev)
> +{
> + struct mobiveil_pcie *pcie;
> + struct pci_host_bridge *bridge;
> + struct device *dev = &pdev->dev;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));

You've lost the comment that was above this.

Thanks,

Andrew Murray

> + if (!bridge)
> + return -ENOMEM;
> +
> + pcie = pci_host_bridge_priv(bridge);
> + pcie->rp.bridge = bridge;
> +
> + pcie->pdev = pdev;
> +
> + return mobiveil_pcie_host_probe(pcie);
> +}
> +
> static const struct of_device_id mobiveil_pcie_of_match[] = {
> {.compatible = "mbvl,gpex40-pcie",},
> {},
> --
> 2.17.1
>