> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxx] > Sent: 16 February 2023 16:42 > To: Shradha Todi <shradha.t@xxxxxxxxxxx>; lpieralisi@xxxxxxxxxx; > kw@xxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx; > jingoohan1@xxxxxxxxx; Sergey.Semin@xxxxxxxxxxxxxxxxxxxx; > lukas.bulwahn@xxxxxxxxx; hongxing.zhu@xxxxxxx; tglx@xxxxxxxxxxxxx; > m.szyprowski@xxxxxxxxxxx; jh80.chung@xxxxxxxxxx; > pankaj.dubey@xxxxxxxxxxx > Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 15/16] PCI: samsung: Add structure to hold resource > operations > > 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://protect2.fireeye.com/v1/url?k=66656d8a-07ee78a5-6664e6c5- > 74fe485cbfe7-a61191c61bcf38f7&q=1&e=80994c2d-d0ca-4b83-a7ca- > 5242c4bb701f&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.18- > rc4%2Fsource%2FDocumentation%2Fprocess%2Fsubmitting- > patches.rst%23L586 > > Wrapping looks a bit short... Ack > > > need platform specific functions to initialize or alter them. For > > better code reusibility, making a separate > > typo, I think it is: re-usability Ack > > > res_ops which will hold all such function pointers or other resource > > specific data. > > Are you saying that interrupts differ in different devices? > Yes, the interrupts are routed and integrated differently for the different platforms > > > > This patch includes adding function pointer for IRQ > > Do not use "This commit/patch". > https://protect2.fireeye.com/v1/url?k=ffdc2502-9e57302d-ffddae4d- > 74fe485cbfe7-49aeaacd1141660f&q=1&e=80994c2d-d0ca-4b83-a7ca- > 5242c4bb701f&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.17.1%2 > Fsource%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L95 > Ack > > 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. > Ack > > + > > + 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
