On 14/02/2023 13:13, Shradha Todi wrote: > Some resources might differ based on platforms and we Please wrap commit message according to Linux coding style / submission process (neither too early nor over the limit): https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586 Wrapping looks a bit short... > need platform specific functions to initialize or alter > them. For better code reusibility, making a separate typo, I think it is: re-usability > res_ops which will hold all such function pointers or > other resource specific data. Are you saying that interrupts differ in different devices? > > This patch includes adding function pointer for IRQ Do not use "This commit/patch". https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > initialization which will help to move common operations for > host init into the probe sequence. > > Suggested-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx> > Signed-off-by: Shradha Todi <shradha.t@xxxxxxxxxxx> > --- > drivers/pci/controller/dwc/pci-samsung.c | 26 ++++++++++++++++-------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-samsung.c b/drivers/pci/controller/dwc/pci-samsung.c > index 47ca2a6a545d..01882f2d06c7 100644 > --- a/drivers/pci/controller/dwc/pci-samsung.c > +++ b/drivers/pci/controller/dwc/pci-samsung.c > @@ -55,6 +55,7 @@ struct samsung_pcie_pdata { > struct pci_ops *pci_ops; > const struct dw_pcie_ops *dwc_ops; > const struct dw_pcie_host_ops *host_ops; > + const struct samsung_res_ops *res_ops; > }; > > /* > @@ -77,6 +78,10 @@ struct samsung_pcie { > struct regulator_bulk_data supplies[2]; > }; > > +struct samsung_res_ops { > + int (*irq_init)(struct samsung_pcie *sp, struct platform_device *pdev); > +}; > + > static int samsung_pcie_init_clk_resources(struct samsung_pcie *sp) > { > struct device *dev = sp->pci.dev; > @@ -276,7 +281,7 @@ static const struct dw_pcie_host_ops exynos_pcie_host_ops = { > .host_init = exynos_pcie_host_init, > }; > > -static int exynos_add_pcie_port(struct samsung_pcie *sp, > +static int exynos_irq_init(struct samsung_pcie *sp, > struct platform_device *pdev) > { > struct dw_pcie *pci = &sp->pci; > @@ -295,15 +300,8 @@ static int exynos_add_pcie_port(struct samsung_pcie *sp, > return ret; > } > > - pp->ops = &exynos_pcie_host_ops; > pp->msi_irq[0] = -ENODEV; > > - ret = dw_pcie_host_init(pp); > - if (ret) { > - dev_err(dev, "failed to initialize host\n"); > - return ret; > - } > - > return 0; > } > > @@ -314,6 +312,10 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = { > .start_link = exynos_pcie_start_link, > }; > > +static const struct samsung_res_ops exynos_res_ops_data = { > + .irq_init = exynos_irq_init, > +}; > + > static int samsung_pcie_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -357,7 +359,12 @@ static int samsung_pcie_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, sp); > > - ret = exynos_add_pcie_port(sp, pdev); > + if (pdata->res_ops->irq_init) > + pdata->res_ops->irq_init(sp, pdev); Check return value and handle errors. > + > + sp->pci.pp.ops = pdata->host_ops; > + > + ret = dw_pcie_host_init(&sp->pci.pp); > if (ret < 0) > goto fail_probe; > > @@ -428,6 +435,7 @@ static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = { > .dwc_ops = &exynos_dw_pcie_ops, > .pci_ops = &exynos_pci_ops, > .host_ops = &exynos_pcie_host_ops, > + .res_ops = &exynos_res_ops_data, > }; > > static const struct of_device_id samsung_pcie_of_match[] = { Best regards, Krzysztof