-----Original Message-----
From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
Sent: 16 February 2023 16:42
To: Shradha Todi <redacted>; lpieralisi@kernel.org;
kw@linux.com; robh@kernel.org; bhelgaas@google.com;
krzysztof.kozlowski+dt@linaro.org; alim.akhtar@samsung.com;
jingoohan1@gmail.com; Sergey.Semin@baikalelectronics.ru;
lukas.bulwahn@gmail.com; hongxing.zhu@nxp.com; tglx@linutronix.de;
m.szyprowski@samsung.com; jh80.chung@samsung.co;
pankaj.dubey@samsung.com
Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH 15/16] PCI: samsung: Add structure to hold resource
operations

On 14/02/2023 13:13, Shradha Todi wrote:

quoted

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

quoted

need platform specific functions to initialize or alter them. For
better code reusibility, making a separate

typo, I think it is: re-usability

Ack

quoted

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

quoted

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

quoted

initialization which will help to move common operations for host init
into the probe sequence.

Suggested-by: Pankaj Dubey <redacted>
Signed-off-by: Shradha Todi <redacted>
---
 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 = {

quoted

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

quoted

 		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 = {

quoted

 	.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

quoted

+
+	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 = {

quoted

 	.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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel