[PATCH v3 00/12] Add PCIe support for Tesla FSD SoC

[PATCH v3 00/12] Add PCIe support for Tesla FSD SoC

[Shradha Todi]

FSD platform has three instances of DesignWare based PCIe IP,
one is in FSYS0 block and other two in FSYS1 block.
This patch series add required DT binding, DT file modifications,
Controller driver support and PHY driver support for the same.

To keep single PCIe controller driver and PHY driver for all Samsung
manufactured SoC, we have made changes to Exynos files to extend
support for FSD platform and other Samsung manufactured SoCs which
shall be upstreamed soon.

First a version was posted as a separate driver file:
https://lore.kernel.org/lkml/20221121105210.68596-1-shradha.t@samsung.com/
This was rejected and request was made to add the support in exynos file
itself.

Then another patchset was posted to refactor existing exynos file:
https://lore.kernel.org/lkml/649a8d88-0504-5aa9-d167-d25d394f3f26@linaro.org/T/
This requested some major changes

Taking both these reviews into consideration, I have posted a fresh
patchset where both changes to exynos framework and addition of new FSD
support is present.

v3:
 - Made separate DT bindings for Exynos5433 and FSD platforms as not
   much is shared
 - Moved all common changes to separate patch for controller and PHY
   and added FSD related changes on top
 - Removed alias method of differentiating between instances of PHY and
   added two separate compatibles.
 - Fixed smatch issues pointed out by Dan and other styling issues.

v2: https://lore.kernel.org/all/20250625165229.3458-1-shradha.t@samsung.com/ 
 - Reordered patches for removing unused MACROs and renaming them
 - Fixed all incomplete DT bindings
 - Modified PHY driver code to adopt better design
 - Removed patch to add alignment data in PCI endpoint test driver
 - Added dts changes in the patchset itself

v1: https://lore.kernel.org/lkml/20250518193152.63476-1-shradha.t@samsung.com/ 


*** BLURB HERE ***

Shradha Todi (12):
  PCI: exynos: Remove unused MACROs in exynos PCIe file
  PCI: exynos: Change macro names to exynos specific
  PCI: exynos: Reorder MACROs to maintain consistency
  PCI: exynos: Add platform device private data
  PCI: exynos: Add resource ops, soc variant and device mode
  dt-bindings: PCI: Split exynos host into two files
  dt-bindings: PCI: Add support for Tesla FSD SoC
  dt-bindings: phy: Add PCIe PHY support for FSD SoC
  phy: exynos: Add platform device private data
  phy: exynos: Add PCIe PHY support for FSD SoC
  PCI: exynos: Add support for Tesla FSD SoC
  arm64: dts: fsd: Add PCIe support for Tesla FSD SoC

 .../bindings/pci/samsung,exynos-pcie.yaml     |  70 +--
 .../bindings/pci/samsung,exynos5433-pcie.yaml |  89 +++
 .../bindings/pci/tesla,fsd-pcie-ep.yaml       |  91 +++
 .../bindings/pci/tesla,fsd-pcie.yaml          |  77 +++
 .../bindings/phy/samsung,exynos-pcie-phy.yaml |  27 +-
 arch/arm64/boot/dts/tesla/fsd-evb.dts         |  34 ++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi    |  65 ++
 arch/arm64/boot/dts/tesla/fsd.dtsi            | 147 +++++
 drivers/pci/controller/dwc/pci-exynos.c       | 567 +++++++++++++++---
 drivers/phy/samsung/phy-exynos-pcie.c         | 295 ++++++++-
 10 files changed, 1300 insertions(+), 162 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml

-- 
2.49.0

[PATCH v3 01/12] PCI: exynos: Remove unused MACROs in exynos PCIe file

[Shradha Todi]

Some MACROs are defined in the exynos PCIe file but are
not used anywhere within the file. Remove such unused
MACROs.

Suggested-by: Hrishikesh Dileep <hrishikesh.d@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 1f0e98d07109..f9140d1f1d19 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -31,8 +31,6 @@
 #define IRQ_INTB_ASSERT			BIT(2)
 #define IRQ_INTC_ASSERT			BIT(4)
 #define IRQ_INTD_ASSERT			BIT(6)
-#define PCIE_IRQ_LEVEL			0x004
-#define PCIE_IRQ_SPECIAL		0x008
 #define PCIE_IRQ_EN_PULSE		0x00c
 #define PCIE_IRQ_EN_LEVEL		0x010
 #define PCIE_IRQ_EN_SPECIAL		0x014
-- 
2.49.0

[PATCH v3 02/12] PCI: exynos: Change macro names to exynos specific

[Shradha Todi]

Prefix macro names in exynos file with the term "EXYNOS" as the current
macro names seem to be generic to PCIe.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 112 ++++++++++++------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index f9140d1f1d19..30d12ff9b0c6 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -26,28 +26,28 @@
 #define to_exynos_pcie(x)	dev_get_drvdata((x)->dev)
 
 /* PCIe ELBI registers */
-#define PCIE_IRQ_PULSE			0x000
-#define IRQ_INTA_ASSERT			BIT(0)
-#define IRQ_INTB_ASSERT			BIT(2)
-#define IRQ_INTC_ASSERT			BIT(4)
-#define IRQ_INTD_ASSERT			BIT(6)
-#define PCIE_IRQ_EN_PULSE		0x00c
-#define PCIE_IRQ_EN_LEVEL		0x010
-#define PCIE_IRQ_EN_SPECIAL		0x014
-#define PCIE_SW_WAKE			0x018
-#define PCIE_BUS_EN			BIT(1)
-#define PCIE_CORE_RESET			0x01c
-#define PCIE_CORE_RESET_ENABLE		BIT(0)
-#define PCIE_STICKY_RESET		0x020
-#define PCIE_NONSTICKY_RESET		0x024
-#define PCIE_APP_INIT_RESET		0x028
-#define PCIE_APP_LTSSM_ENABLE		0x02c
-#define PCIE_ELBI_RDLH_LINKUP		0x074
-#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
-#define PCIE_ELBI_LTSSM_ENABLE		0x1
-#define PCIE_ELBI_SLV_AWMISC		0x11c
-#define PCIE_ELBI_SLV_ARMISC		0x120
-#define PCIE_ELBI_SLV_DBI_ENABLE	BIT(21)
+#define EXYNOS_PCIE_IRQ_PULSE			0x000
+#define EXYNOS_IRQ_INTA_ASSERT			BIT(0)
+#define EXYNOS_IRQ_INTB_ASSERT			BIT(2)
+#define EXYNOS_IRQ_INTC_ASSERT			BIT(4)
+#define EXYNOS_IRQ_INTD_ASSERT			BIT(6)
+#define EXYNOS_PCIE_IRQ_EN_PULSE		0x00c
+#define EXYNOS_PCIE_IRQ_EN_LEVEL		0x010
+#define EXYNOS_PCIE_IRQ_EN_SPECIAL		0x014
+#define EXYNOS_PCIE_SW_WAKE			0x018
+#define EXYNOS_PCIE_BUS_EN			BIT(1)
+#define EXYNOS_PCIE_CORE_RESET			0x01c
+#define EXYNOS_PCIE_CORE_RESET_ENABLE		BIT(0)
+#define EXYNOS_PCIE_STICKY_RESET		0x020
+#define EXYNOS_PCIE_NONSTICKY_RESET		0x024
+#define EXYNOS_PCIE_APP_INIT_RESET		0x028
+#define EXYNOS_PCIE_APP_LTSSM_ENABLE		0x02c
+#define EXYNOS_PCIE_ELBI_RDLH_LINKUP		0x074
+#define EXYNOS_PCIE_ELBI_XMLH_LINKUP		BIT(4)
+#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE		0x1
+#define EXYNOS_PCIE_ELBI_SLV_AWMISC		0x11c
+#define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
+#define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
 
 struct exynos_pcie {
 	struct dw_pcie			pci;
@@ -71,49 +71,49 @@ static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
 {
 	u32 val;
 
-	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_AWMISC);
+	val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_SLV_AWMISC);
 	if (on)
-		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+		val |= EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
 	else
-		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_AWMISC);
+		val &= ~EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_ELBI_SLV_AWMISC);
 }
 
 static void exynos_pcie_sideband_dbi_r_mode(struct exynos_pcie *ep, bool on)
 {
 	u32 val;
 
-	val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_SLV_ARMISC);
+	val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_SLV_ARMISC);
 	if (on)
-		val |= PCIE_ELBI_SLV_DBI_ENABLE;
+		val |= EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
 	else
-		val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_ELBI_SLV_ARMISC);
+		val &= ~EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE;
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_ELBI_SLV_ARMISC);
 }
 
 static void exynos_pcie_assert_core_reset(struct exynos_pcie *ep)
 {
 	u32 val;
 
-	val = exynos_pcie_readl(ep->elbi_base, PCIE_CORE_RESET);
-	val &= ~PCIE_CORE_RESET_ENABLE;
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_CORE_RESET);
-	exynos_pcie_writel(ep->elbi_base, 0, PCIE_STICKY_RESET);
-	exynos_pcie_writel(ep->elbi_base, 0, PCIE_NONSTICKY_RESET);
+	val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_CORE_RESET);
+	val &= ~EXYNOS_PCIE_CORE_RESET_ENABLE;
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_CORE_RESET);
+	exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_STICKY_RESET);
+	exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_NONSTICKY_RESET);
 }
 
 static void exynos_pcie_deassert_core_reset(struct exynos_pcie *ep)
 {
 	u32 val;
 
-	val = exynos_pcie_readl(ep->elbi_base, PCIE_CORE_RESET);
-	val |= PCIE_CORE_RESET_ENABLE;
+	val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_CORE_RESET);
+	val |= EXYNOS_PCIE_CORE_RESET_ENABLE;
 
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_CORE_RESET);
-	exynos_pcie_writel(ep->elbi_base, 1, PCIE_STICKY_RESET);
-	exynos_pcie_writel(ep->elbi_base, 1, PCIE_NONSTICKY_RESET);
-	exynos_pcie_writel(ep->elbi_base, 1, PCIE_APP_INIT_RESET);
-	exynos_pcie_writel(ep->elbi_base, 0, PCIE_APP_INIT_RESET);
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_CORE_RESET);
+	exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_STICKY_RESET);
+	exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_NONSTICKY_RESET);
+	exynos_pcie_writel(ep->elbi_base, 1, EXYNOS_PCIE_APP_INIT_RESET);
+	exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_APP_INIT_RESET);
 }
 
 static int exynos_pcie_start_link(struct dw_pcie *pci)
@@ -121,21 +121,21 @@ static int exynos_pcie_start_link(struct dw_pcie *pci)
 	struct exynos_pcie *ep = to_exynos_pcie(pci);
 	u32 val;
 
-	val = exynos_pcie_readl(ep->elbi_base, PCIE_SW_WAKE);
-	val &= ~PCIE_BUS_EN;
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_SW_WAKE);
+	val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_SW_WAKE);
+	val &= ~EXYNOS_PCIE_BUS_EN;
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_SW_WAKE);
 
 	/* assert LTSSM enable */
-	exynos_pcie_writel(ep->elbi_base, PCIE_ELBI_LTSSM_ENABLE,
-			  PCIE_APP_LTSSM_ENABLE);
+	exynos_pcie_writel(ep->elbi_base, EXYNOS_PCIE_ELBI_LTSSM_ENABLE,
+			  EXYNOS_PCIE_APP_LTSSM_ENABLE);
 	return 0;
 }
 
 static void exynos_pcie_clear_irq_pulse(struct exynos_pcie *ep)
 {
-	u32 val = exynos_pcie_readl(ep->elbi_base, PCIE_IRQ_PULSE);
+	u32 val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_IRQ_PULSE);
 
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_PULSE);
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_IRQ_PULSE);
 }
 
 static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
@@ -148,12 +148,12 @@ static irqreturn_t exynos_pcie_irq_handler(int irq, void *arg)
 
 static void exynos_pcie_enable_irq_pulse(struct exynos_pcie *ep)
 {
-	u32 val = IRQ_INTA_ASSERT | IRQ_INTB_ASSERT |
-		  IRQ_INTC_ASSERT | IRQ_INTD_ASSERT;
+	u32 val = EXYNOS_IRQ_INTA_ASSERT | EXYNOS_IRQ_INTB_ASSERT |
+		  EXYNOS_IRQ_INTC_ASSERT | EXYNOS_IRQ_INTD_ASSERT;
 
-	exynos_pcie_writel(ep->elbi_base, val, PCIE_IRQ_EN_PULSE);
-	exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_LEVEL);
-	exynos_pcie_writel(ep->elbi_base, 0, PCIE_IRQ_EN_SPECIAL);
+	exynos_pcie_writel(ep->elbi_base, val, EXYNOS_PCIE_IRQ_EN_PULSE);
+	exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_IRQ_EN_LEVEL);
+	exynos_pcie_writel(ep->elbi_base, 0, EXYNOS_PCIE_IRQ_EN_SPECIAL);
 }
 
 static u32 exynos_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
@@ -210,9 +210,9 @@ static struct pci_ops exynos_pci_ops = {
 static bool exynos_pcie_link_up(struct dw_pcie *pci)
 {
 	struct exynos_pcie *ep = to_exynos_pcie(pci);
-	u32 val = exynos_pcie_readl(ep->elbi_base, PCIE_ELBI_RDLH_LINKUP);
+	u32 val = exynos_pcie_readl(ep->elbi_base, EXYNOS_PCIE_ELBI_RDLH_LINKUP);
 
-	return val & PCIE_ELBI_XMLH_LINKUP;
+	return val & EXYNOS_PCIE_ELBI_XMLH_LINKUP;
 }
 
 static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
-- 
2.49.0

[PATCH v3 03/12] PCI: exynos: Reorder MACROs to maintain consistency

[Shradha Todi]

Exynos PCI file follows MACRO definition order where
register offset is defined in ascending order and each
bit field within the offset is defined right after offset
definition. Some MACROs are out of order and so reorder
those MACROs to maintain consistency.

Suggested-by: Hrishikesh Dileep <hrishikesh.d@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 30d12ff9b0c6..b4ec167b0583 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -27,11 +27,11 @@
 
 /* PCIe ELBI registers */
 #define EXYNOS_PCIE_IRQ_PULSE			0x000
+#define EXYNOS_PCIE_IRQ_EN_PULSE		0x00c
 #define EXYNOS_IRQ_INTA_ASSERT			BIT(0)
 #define EXYNOS_IRQ_INTB_ASSERT			BIT(2)
 #define EXYNOS_IRQ_INTC_ASSERT			BIT(4)
 #define EXYNOS_IRQ_INTD_ASSERT			BIT(6)
-#define EXYNOS_PCIE_IRQ_EN_PULSE		0x00c
 #define EXYNOS_PCIE_IRQ_EN_LEVEL		0x010
 #define EXYNOS_PCIE_IRQ_EN_SPECIAL		0x014
 #define EXYNOS_PCIE_SW_WAKE			0x018
@@ -42,9 +42,9 @@
 #define EXYNOS_PCIE_NONSTICKY_RESET		0x024
 #define EXYNOS_PCIE_APP_INIT_RESET		0x028
 #define EXYNOS_PCIE_APP_LTSSM_ENABLE		0x02c
+#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE		0x1
 #define EXYNOS_PCIE_ELBI_RDLH_LINKUP		0x074
 #define EXYNOS_PCIE_ELBI_XMLH_LINKUP		BIT(4)
-#define EXYNOS_PCIE_ELBI_LTSSM_ENABLE		0x1
 #define EXYNOS_PCIE_ELBI_SLV_AWMISC		0x11c
 #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
 #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
-- 
2.49.0

[PATCH v3 04/12] PCI: exynos: Add platform device private data

[Shradha Todi]

In order to extend this driver to all Samsung manufactured SoCs having
DWC PCIe controller, add private data structure which will hold platform
device specific information. It holds function ops like DWC host ops,
DWC generic ops, and PCI read/write ops which will be used as driver
data for different compatibles.

Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 32 ++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index b4ec167b0583..c830b20d54f0 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -49,9 +49,16 @@
 #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
 #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
 
+struct samsung_pcie_pdata {
+	struct pci_ops				*pci_ops;
+	const struct dw_pcie_ops		*dwc_ops;
+	const struct dw_pcie_host_ops		*host_ops;
+};
+
 struct exynos_pcie {
 	struct dw_pcie			pci;
 	void __iomem			*elbi_base;
+	const struct samsung_pcie_pdata	*pdata;
 	struct clk_bulk_data		*clks;
 	struct phy			*phy;
 	struct regulator_bulk_data	supplies[2];
@@ -220,7 +227,7 @@ static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct exynos_pcie *ep = to_exynos_pcie(pci);
 
-	pp->bridge->ops = &exynos_pci_ops;
+	pp->bridge->ops = ep->pdata->pci_ops;
 
 	exynos_pcie_assert_core_reset(ep);
 
@@ -268,7 +275,7 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
 	return 0;
 }
 
-static const struct dw_pcie_ops dw_pcie_ops = {
+static const struct dw_pcie_ops exynos_dw_pcie_ops = {
 	.read_dbi = exynos_pcie_read_dbi,
 	.write_dbi = exynos_pcie_write_dbi,
 	.link_up = exynos_pcie_link_up,
@@ -279,6 +286,7 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct exynos_pcie *ep;
+	const struct samsung_pcie_pdata *pdata;
 	struct device_node *np = dev->of_node;
 	int ret;
 
@@ -286,8 +294,11 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 	if (!ep)
 		return -ENOMEM;
 
+	pdata = of_device_get_match_data(dev);
+
+	ep->pdata = pdata;
 	ep->pci.dev = dev;
-	ep->pci.ops = &dw_pcie_ops;
+	ep->pci.ops = pdata->dwc_ops;
 
 	ep->phy = devm_of_phy_get(dev, np, NULL);
 	if (IS_ERR(ep->phy))
@@ -363,9 +374,9 @@ static int exynos_pcie_resume_noirq(struct device *dev)
 		return ret;
 
 	/* exynos_pcie_host_init controls ep->phy */
-	exynos_pcie_host_init(pp);
+	ep->pdata->host_ops->init(pp);
 	dw_pcie_setup_rc(pp);
-	exynos_pcie_start_link(pci);
+	ep->pdata->dwc_ops->start_link(pci);
 	return dw_pcie_wait_for_link(pci);
 }
 
@@ -374,8 +385,17 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
 				  exynos_pcie_resume_noirq)
 };
 
+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,
+};
+
 static const struct of_device_id exynos_pcie_of_match[] = {
-	{ .compatible = "samsung,exynos5433-pcie", },
+	{
+		.compatible = "samsung,exynos5433-pcie",
+		.data = (void *) &exynos_5433_pcie_rc_pdata,
+	},
 	{ },
 };
 
-- 
2.49.0

[PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode

[Shradha Todi]

Some resources might differ based on platforms and we need platform
specific functions to initialize or alter them. For better code
re-usability, making a separate res_ops which will hold all such
function pointers or other resource specific data. Include ops like
 - init_regulator (initialize the regulator data)
 - pcie_irq_handler (interrupt handler for PCIe)
 - set_device_mode (set device mode to EP or RC)

Some operations maybe specific to certain SoCs and not applicable
to others. For such use cases, adding an SoC variant data field
which can be used to distinguish between the variants.

Some SoCs may have dual-role PCIe controller which can work as
RC or EP. Add device_mode to store the role and take decisions
accordingly.

Make enable/disable of regulator and initialization of IRQ as
common functions to be used by all Samsung SoCs.

Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 143 +++++++++++++++++++-----
 1 file changed, 116 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index c830b20d54f0..ef1f42236575 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -49,10 +49,18 @@
 #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
 #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
 
+/* to store different SoC variants of Samsung */
+enum samsung_pcie_variants {
+	EXYNOS_5433,
+};
+
 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;
+	unsigned int				soc_variant;
+	enum dw_pcie_device_mode		device_mode;
 };
 
 struct exynos_pcie {
@@ -61,7 +69,14 @@ struct exynos_pcie {
 	const struct samsung_pcie_pdata	*pdata;
 	struct clk_bulk_data		*clks;
 	struct phy			*phy;
-	struct regulator_bulk_data	supplies[2];
+	struct regulator_bulk_data	*supplies;
+	int				supplies_cnt;
+};
+
+struct samsung_res_ops {
+	int (*init_regulator)(struct exynos_pcie *ep);
+	irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
+	void (*set_device_mode)(struct exynos_pcie *ep);
 };
 
 static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
@@ -74,6 +89,31 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
 	return readl(base + reg);
 }
 
+static int samsung_regulator_enable(struct exynos_pcie *ep)
+{
+	int ret;
+
+	if (ep->supplies_cnt == 0)
+		return 0;
+
+	ret = regulator_bulk_enable(ep->supplies_cnt, ep->supplies);
+
+	return ret;
+}
+
+static void samsung_regulator_disable(struct exynos_pcie *ep)
+{
+	struct device *dev = ep->pci.dev;
+	int ret;
+
+	if (ep->supplies_cnt == 0)
+		return;
+
+	ret = regulator_bulk_disable(ep->supplies_cnt, ep->supplies);
+	if (ret)
+		dev_warn(dev, "failed to disable regulators: %d\n", ret);
+}
+
 static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
 {
 	u32 val;
@@ -244,7 +284,26 @@ static const struct dw_pcie_host_ops exynos_pcie_host_ops = {
 	.init = exynos_pcie_host_init,
 };
 
-static int exynos_add_pcie_port(struct exynos_pcie *ep,
+static int exynos_init_regulator(struct exynos_pcie *ep)
+{
+	struct device *dev = ep->pci.dev;
+	int ret = 0;
+
+	ep->supplies_cnt = 2;
+
+	ep->supplies = devm_kcalloc(dev, ep->supplies_cnt, sizeof(*ep->supplies), GFP_KERNEL);
+	if (!ep->supplies)
+		return -ENOMEM;
+
+	ep->supplies[0].supply = "vdd18";
+	ep->supplies[1].supply = "vdd10";
+
+	ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
+
+	return ret;
+}
+
+static int samsung_irq_init(struct exynos_pcie *ep,
 				       struct platform_device *pdev)
 {
 	struct dw_pcie *pci = &ep->pci;
@@ -256,22 +315,15 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
 	if (pp->irq < 0)
 		return pp->irq;
 
-	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
+	ret = devm_request_irq(dev, pp->irq, ep->pdata->res_ops->pcie_irq_handler,
 			       IRQF_SHARED, "exynos-pcie", ep);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
 		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;
 }
 
@@ -282,6 +334,11 @@ 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 = {
+	.init_regulator		= exynos_init_regulator,
+	.pcie_irq_handler	= exynos_pcie_irq_handler,
+};
+
 static int exynos_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -313,28 +370,46 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	ep->supplies[0].supply = "vdd18";
-	ep->supplies[1].supply = "vdd10";
-	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ep->supplies),
-				      ep->supplies);
-	if (ret)
-		return ret;
+	if (pdata->res_ops && pdata->res_ops->init_regulator) {
+		ret = ep->pdata->res_ops->init_regulator(ep);
+		if (ret)
+			return ret;
+	}
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
+	ret = samsung_regulator_enable(ep);
 	if (ret)
 		return ret;
 
 	platform_set_drvdata(pdev, ep);
 
-	ret = exynos_add_pcie_port(ep, pdev);
-	if (ret < 0)
-		goto fail_probe;
+	if (pdata->res_ops && pdata->res_ops->set_device_mode)
+		pdata->res_ops->set_device_mode(ep);
+
+	switch (ep->pdata->device_mode) {
+	case DW_PCIE_RC_TYPE:
+		ret = samsung_irq_init(ep, pdev);
+		if (ret)
+			goto fail_regulator;
+
+		ep->pci.pp.ops = pdata->host_ops;
+
+		ret = dw_pcie_host_init(&ep->pci.pp);
+		if (ret < 0)
+			goto fail_phy_init;
+
+		break;
+	default:
+		dev_err(dev, "invalid device type\n");
+		ret = -EINVAL;
+		goto fail_regulator;
+	}
 
 	return 0;
 
-fail_probe:
+fail_phy_init:
 	phy_exit(ep->phy);
-	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+fail_regulator:
+	samsung_regulator_disable(ep);
 
 	return ret;
 }
@@ -343,21 +418,29 @@ static void exynos_pcie_remove(struct platform_device *pdev)
 {
 	struct exynos_pcie *ep = platform_get_drvdata(pdev);
 
+	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+		return;
 	dw_pcie_host_deinit(&ep->pci.pp);
-	exynos_pcie_assert_core_reset(ep);
+	if (ep->pdata->soc_variant == EXYNOS_5433)
+		exynos_pcie_assert_core_reset(ep);
 	phy_power_off(ep->phy);
 	phy_exit(ep->phy);
-	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+	samsung_regulator_disable(ep);
 }
 
 static int exynos_pcie_suspend_noirq(struct device *dev)
 {
 	struct exynos_pcie *ep = dev_get_drvdata(dev);
+	struct dw_pcie *pci = &ep->pci;
 
-	exynos_pcie_assert_core_reset(ep);
+	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+		return 0;
+
+	if (ep->pdata->soc_variant == EXYNOS_5433)
+		exynos_pcie_assert_core_reset(ep);
 	phy_power_off(ep->phy);
 	phy_exit(ep->phy);
-	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
+	samsung_regulator_disable(ep);
 
 	return 0;
 }
@@ -369,7 +452,10 @@ static int exynos_pcie_resume_noirq(struct device *dev)
 	struct dw_pcie_rp *pp = &pci->pp;
 	int ret;
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
+	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
+		return 0;
+
+	ret = samsung_regulator_enable(ep);
 	if (ret)
 		return ret;
 
@@ -389,6 +475,9 @@ 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,
+	.soc_variant		= EXYNOS_5433,
+	.device_mode		= DW_PCIE_RC_TYPE,
 };
 
 static const struct of_device_id exynos_pcie_of_match[] = {
-- 
2.49.0

Re: [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode

[Bjorn Helgaas]

On Mon, Aug 11, 2025 at 09:16:31PM +0530, Shradha Todi wrote:
> Some resources might differ based on platforms and we need platform
> specific functions to initialize or alter them. For better code
> re-usability, making a separate res_ops which will hold all such
> function pointers or other resource specific data. Include ops like
>  - init_regulator (initialize the regulator data)
>  - pcie_irq_handler (interrupt handler for PCIe)
>  - set_device_mode (set device mode to EP or RC)
> 
> Some operations maybe specific to certain SoCs and not applicable
> to others. For such use cases, adding an SoC variant data field
> which can be used to distinguish between the variants.
> 
> Some SoCs may have dual-role PCIe controller which can work as
> RC or EP. Add device_mode to store the role and take decisions
> accordingly.
> 
> Make enable/disable of regulator and initialization of IRQ as
> common functions to be used by all Samsung SoCs.

As hinted above, this patch ends up being a mixture of several things
that makes this kind of hard to review.  Separating these into their
own patches would make it easier.

> Suggested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/pci-exynos.c | 143 +++++++++++++++++++-----
>  1 file changed, 116 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index c830b20d54f0..ef1f42236575 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -49,10 +49,18 @@
>  #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
>  #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
>  
> +/* to store different SoC variants of Samsung */
> +enum samsung_pcie_variants {
> +	EXYNOS_5433,
> +};
> +
>  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;
> +	unsigned int				soc_variant;
> +	enum dw_pcie_device_mode		device_mode;
>  };
>  
>  struct exynos_pcie {
> @@ -61,7 +69,14 @@ struct exynos_pcie {
>  	const struct samsung_pcie_pdata	*pdata;
>  	struct clk_bulk_data		*clks;
>  	struct phy			*phy;
> -	struct regulator_bulk_data	supplies[2];
> +	struct regulator_bulk_data	*supplies;
> +	int				supplies_cnt;
> +};
> +
> +struct samsung_res_ops {
> +	int (*init_regulator)(struct exynos_pcie *ep);
> +	irqreturn_t (*pcie_irq_handler)(int irq, void *arg);
> +	void (*set_device_mode)(struct exynos_pcie *ep);
>  };
>  
>  static void exynos_pcie_writel(void __iomem *base, u32 val, u32 reg)
> @@ -74,6 +89,31 @@ static u32 exynos_pcie_readl(void __iomem *base, u32 reg)
>  	return readl(base + reg);
>  }
>  
> +static int samsung_regulator_enable(struct exynos_pcie *ep)
> +{
> +	int ret;
> +
> +	if (ep->supplies_cnt == 0)
> +		return 0;
> +
> +	ret = regulator_bulk_enable(ep->supplies_cnt, ep->supplies);
> +
> +	return ret;

Unless you want a warning here, there's no need for "ret":

  return regulator_bulk_enable(ep->supplies_cnt, ep->supplies);

> +}
> +
> +static void samsung_regulator_disable(struct exynos_pcie *ep)
> +{
> +	struct device *dev = ep->pci.dev;
> +	int ret;
> +
> +	if (ep->supplies_cnt == 0)
> +		return;
> +
> +	ret = regulator_bulk_disable(ep->supplies_cnt, ep->supplies);
> +	if (ret)
> +		dev_warn(dev, "failed to disable regulators: %d\n", ret);
> +}
> +
>  static void exynos_pcie_sideband_dbi_w_mode(struct exynos_pcie *ep, bool on)
>  {
>  	u32 val;
> @@ -244,7 +284,26 @@ static const struct dw_pcie_host_ops exynos_pcie_host_ops = {
>  	.init = exynos_pcie_host_init,
>  };
>  
> -static int exynos_add_pcie_port(struct exynos_pcie *ep,
> +static int exynos_init_regulator(struct exynos_pcie *ep)
> +{
> +	struct device *dev = ep->pci.dev;
> +	int ret = 0;
> +
> +	ep->supplies_cnt = 2;
> +
> +	ep->supplies = devm_kcalloc(dev, ep->supplies_cnt, sizeof(*ep->supplies), GFP_KERNEL);
> +	if (!ep->supplies)
> +		return -ENOMEM;
> +
> +	ep->supplies[0].supply = "vdd18";
> +	ep->supplies[1].supply = "vdd10";
> +
> +	ret = devm_regulator_bulk_get(dev, ep->supplies_cnt, ep->supplies);
> +
> +	return ret;

No need for ret.

> +}
> +
> +static int samsung_irq_init(struct exynos_pcie *ep,
>  				       struct platform_device *pdev)
>  {
>  	struct dw_pcie *pci = &ep->pci;
> @@ -256,22 +315,15 @@ static int exynos_add_pcie_port(struct exynos_pcie *ep,
>  	if (pp->irq < 0)
>  		return pp->irq;
>  
> -	ret = devm_request_irq(dev, pp->irq, exynos_pcie_irq_handler,
> +	ret = devm_request_irq(dev, pp->irq, ep->pdata->res_ops->pcie_irq_handler,
>  			       IRQF_SHARED, "exynos-pcie", ep);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
>  		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;
>  }
>  
> @@ -282,6 +334,11 @@ 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 = {
> +	.init_regulator		= exynos_init_regulator,
> +	.pcie_irq_handler	= exynos_pcie_irq_handler,
> +};
> +
>  static int exynos_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -313,28 +370,46 @@ static int exynos_pcie_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	ep->supplies[0].supply = "vdd18";
> -	ep->supplies[1].supply = "vdd10";
> -	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ep->supplies),
> -				      ep->supplies);
> -	if (ret)
> -		return ret;
> +	if (pdata->res_ops && pdata->res_ops->init_regulator) {
> +		ret = ep->pdata->res_ops->init_regulator(ep);
> +		if (ret)
> +			return ret;
> +	}
>  
> -	ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +	ret = samsung_regulator_enable(ep);
>  	if (ret)
>  		return ret;
>  
>  	platform_set_drvdata(pdev, ep);
>  
> -	ret = exynos_add_pcie_port(ep, pdev);
> -	if (ret < 0)
> -		goto fail_probe;
> +	if (pdata->res_ops && pdata->res_ops->set_device_mode)
> +		pdata->res_ops->set_device_mode(ep);
> +
> +	switch (ep->pdata->device_mode) {
> +	case DW_PCIE_RC_TYPE:
> +		ret = samsung_irq_init(ep, pdev);
> +		if (ret)
> +			goto fail_regulator;
> +
> +		ep->pci.pp.ops = pdata->host_ops;
> +
> +		ret = dw_pcie_host_init(&ep->pci.pp);
> +		if (ret < 0)
> +			goto fail_phy_init;
> +
> +		break;
> +	default:
> +		dev_err(dev, "invalid device type\n");
> +		ret = -EINVAL;
> +		goto fail_regulator;
> +	}
>  
>  	return 0;
>  
> -fail_probe:
> +fail_phy_init:
>  	phy_exit(ep->phy);
> -	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +fail_regulator:
> +	samsung_regulator_disable(ep);
>  
>  	return ret;
>  }
> @@ -343,21 +418,29 @@ static void exynos_pcie_remove(struct platform_device *pdev)
>  {
>  	struct exynos_pcie *ep = platform_get_drvdata(pdev);
>  
> +	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> +		return;
>  	dw_pcie_host_deinit(&ep->pci.pp);
> -	exynos_pcie_assert_core_reset(ep);
> +	if (ep->pdata->soc_variant == EXYNOS_5433)
> +		exynos_pcie_assert_core_reset(ep);

I think it's a mistake to test for specific soc_variants.  Better to
test a *property* (or use a function pointer than can be SoC-specific)
because it's likely that several SoCs will share the same property.

>  	phy_power_off(ep->phy);
>  	phy_exit(ep->phy);
> -	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +	samsung_regulator_disable(ep);
>  }
>  
>  static int exynos_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct exynos_pcie *ep = dev_get_drvdata(dev);
> +	struct dw_pcie *pci = &ep->pci;
>  
> -	exynos_pcie_assert_core_reset(ep);
> +	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> +		return 0;
> +
> +	if (ep->pdata->soc_variant == EXYNOS_5433)
> +		exynos_pcie_assert_core_reset(ep);
>  	phy_power_off(ep->phy);
>  	phy_exit(ep->phy);
> -	regulator_bulk_disable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +	samsung_regulator_disable(ep);
>  
>  	return 0;
>  }
> @@ -369,7 +452,10 @@ static int exynos_pcie_resume_noirq(struct device *dev)
>  	struct dw_pcie_rp *pp = &pci->pp;
>  	int ret;
>  
> -	ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
> +	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
> +		return 0;
> +
> +	ret = samsung_regulator_enable(ep);
>  	if (ret)
>  		return ret;
>  
> @@ -389,6 +475,9 @@ 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,
> +	.soc_variant		= EXYNOS_5433,
> +	.device_mode		= DW_PCIE_RC_TYPE,
>  };
>  
>  static const struct of_device_id exynos_pcie_of_match[] = {
> -- 
> 2.49.0
> 

RE: [PATCH v3 05/12] PCI: exynos: Add resource ops, soc variant and device mode

[Shradha Todi]

> > Some resources might differ based on platforms and we need platform
> > specific functions to initialize or alter them. For better code
> > re-usability, making a separate res_ops which will hold all such
> > function pointers or other resource specific data. Include ops like
> >  - init_regulator (initialize the regulator data)
> >  - pcie_irq_handler (interrupt handler for PCIe)
> >  - set_device_mode (set device mode to EP or RC)
> >
> > Some operations maybe specific to certain SoCs and not applicable
> > to others. For such use cases, adding an SoC variant data field
> > which can be used to distinguish between the variants.
> >
> > Some SoCs may have dual-role PCIe controller which can work as
> > RC or EP. Add device_mode to store the role and take decisions
> > accordingly.
> >
> > Make enable/disable of regulator and initialization of IRQ as
> > common functions to be used by all Samsung SoCs.
> 
> As hinted above, this patch ends up being a mixture of several things
> that makes this kind of hard to review.  Separating these into their
> own patches would make it easier.
> 

Will split into multiple patches

[PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files

[Shradha Todi]

The current Exynos PCIe yaml binding file is hard to reuse by
other Samsung SoCs. Refactoring it by:
 - Moving common Samsung PCIe properties into samsung,exynos-pcie.yaml
 - Creating a dedicated samsung,exynos5433-pcie.yaml file for properties
   and constraints specific to the Exynos5433 SoC

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 .../bindings/pci/samsung,exynos-pcie.yaml     | 70 +--------------
 .../bindings/pci/samsung,exynos5433-pcie.yaml | 89 +++++++++++++++++++
 2 files changed, 91 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
index f20ed7e709f7..fd0b97b30821 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
@@ -11,7 +11,7 @@ maintainers:
   - Jaehoon Chung <jh80.chung@samsung.com>
 
 description: |+
-  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
+  Samsung SoCs PCIe host controller is based on the Synopsys DesignWare
   PCIe IP and thus inherits all the common properties defined in
   snps,dw-pcie.yaml.
 
@@ -19,9 +19,6 @@ allOf:
   - $ref: /schemas/pci/snps,dw-pcie.yaml#
 
 properties:
-  compatible:
-    const: samsung,exynos5433-pcie
-
   reg:
     items:
       - description: Data Bus Interface (DBI) registers.
@@ -37,83 +34,20 @@ properties:
   interrupts:
     maxItems: 1
 
-  clocks:
-    items:
-      - description: PCIe bridge clock
-      - description: PCIe bus clock
-
-  clock-names:
-    items:
-      - const: pcie
-      - const: pcie_bus
-
   phys:
     maxItems: 1
 
-  vdd10-supply:
-    description:
-      Phandle to a regulator that provides 1.0V power to the PCIe block.
-
-  vdd18-supply:
-    description:
-      Phandle to a regulator that provides 1.8V power to the PCIe block.
-
-  num-lanes:
-    const: 1
-
-  num-viewport:
-    const: 3
-
 required:
   - reg
   - reg-names
   - interrupts
   - "#address-cells"
   - "#size-cells"
-  - "#interrupt-cells"
-  - interrupt-map
-  - interrupt-map-mask
   - ranges
-  - bus-range
   - device_type
   - num-lanes
-  - num-viewport
   - clocks
   - clock-names
   - phys
-  - vdd10-supply
-  - vdd18-supply
-
-unevaluatedProperties: false
-
-examples:
-  - |
-    #include <dt-bindings/interrupt-controller/irq.h>
-    #include <dt-bindings/interrupt-controller/arm-gic.h>
-    #include <dt-bindings/clock/exynos5433.h>
 
-    pcie: pcie@15700000 {
-        compatible = "samsung,exynos5433-pcie";
-        reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, <0x0c000000 0x1000>;
-        reg-names = "dbi", "elbi", "config";
-        #address-cells = <3>;
-        #size-cells = <2>;
-        #interrupt-cells = <1>;
-        device_type = "pci";
-        interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
-        clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
-        clock-names = "pcie", "pcie_bus";
-        phys = <&pcie_phy>;
-        pinctrl-0 = <&pcie_bus &pcie_wlanen>;
-        pinctrl-names = "default";
-        num-lanes = <1>;
-        num-viewport = <3>;
-        bus-range = <0x00 0xff>;
-        ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000>,
-                 <0x82000000 0 0x0c011000 0x0c011000 0 0x03feefff>;
-        vdd10-supply = <&ldo6_reg>;
-        vdd18-supply = <&ldo7_reg>;
-        interrupt-map-mask = <0 0 0 0>;
-        interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
-    };
-...
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
new file mode 100644
index 000000000000..1fb2c32899c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/samsung,exynos5433-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos5433 SoC PCIe Host Controller
+
+maintainers:
+  - Marek Szyprowski <m.szyprowski@samsung.com>
+  - Jaehoon Chung <jh80.chung@samsung.com>
+
+description:
+  Exynos5433 SoCs PCIe host controller inherits all the
+  common properties defined in samsung,exynos-pcie.yaml
+
+allOf:
+  - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
+
+properties:
+  compatible:
+    const: samsung,exynos5433-pcie
+
+  clocks:
+    items:
+      - description: PCIe bridge clock
+      - description: PCIe bus clock
+
+  clock-names:
+    items:
+      - const: pcie
+      - const: pcie_bus
+
+  num-lanes:
+    const: 1
+
+  num-viewport:
+    const: 3
+
+  vdd10-supply:
+    description:
+      phandle to a regulator that provides 1.0v power to the pcie block
+
+  vdd18-supply:
+    description:
+      phandle to a regulator that provides 1.8v power to the pcie block
+
+required:
+  - "#interrupt-cells"
+  - interrupt-map
+  - interrupt-map-mask
+  - bus-range
+  - num-viewport
+  - vdd10-supply
+  - vdd18-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/exynos5433.h>
+
+    pcie: pcie@15700000 {
+        compatible = "samsung,exynos5433-pcie";
+        reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, <0x0c000000 0x1000>;
+        reg-names = "dbi", "elbi", "config";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        #interrupt-cells = <1>;
+        device_type = "pci";
+        interrupts = <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cmu_fsys CLK_PCIE>, <&cmu_fsys CLK_PCLK_PCIE_PHY>;
+        clock-names = "pcie", "pcie_bus";
+        phys = <&pcie_phy>;
+        pinctrl-0 = <&pcie_bus &pcie_wlanen>;
+        pinctrl-names = "default";
+        num-lanes = <1>;
+        num-viewport = <3>;
+        bus-range = <0x00 0xff>;
+        ranges = <0x81000000 0 0	  0x0c001000 0 0x00010000>,
+                 <0x82000000 0 0x0c011000 0x0c011000 0 0x03feefff>;
+        vdd10-supply = <&ldo6_reg>;
+        vdd18-supply = <&ldo7_reg>;
+        interrupt-map-mask = <0 0 0 0>;
+        interrupt-map = <0 0 0 0 &gic GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+    };
+...
-- 
2.49.0

Re: [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files

[Krzysztof Kozlowski]

On 11/08/2025 17:46, Shradha Todi wrote:
> The current Exynos PCIe yaml binding file is hard to reuse by
> other Samsung SoCs. Refactoring it by:
>  - Moving common Samsung PCIe properties into samsung,exynos-pcie.yaml
>  - Creating a dedicated samsung,exynos5433-pcie.yaml file for properties
>    and constraints specific to the Exynos5433 SoC
> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  .../bindings/pci/samsung,exynos-pcie.yaml     | 70 +--------------
>  .../bindings/pci/samsung,exynos5433-pcie.yaml | 89 +++++++++++++++++++
>  2 files changed, 91 insertions(+), 68 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5433-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> index f20ed7e709f7..fd0b97b30821 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
> @@ -11,7 +11,7 @@ maintainers:
>    - Jaehoon Chung <jh80.chung@samsung.com>
>  
>  description: |+
> -  Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
> +  Samsung SoCs PCIe host controller is based on the Synopsys DesignWare
>    PCIe IP and thus inherits all the common properties defined in
>    snps,dw-pcie.yaml.
>  
> @@ -19,9 +19,6 @@ allOf:
>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
>  
>  properties:
> -  compatible:
> -    const: samsung,exynos5433-pcie
> -
>    reg:
>      items:
>        - description: Data Bus Interface (DBI) registers.


So the only common part left here is reg and phy? I don't think such
common file brings any value.


Best regards,
Krzysztof

RE: [PATCH v3 06/12] dt-bindings: PCI: Split exynos host into two files

[Shradha Todi]

> >
> > @@ -19,9 +19,6 @@ allOf:
> >    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> >
> >  properties:
> > -  compatible:
> > -    const: samsung,exynos5433-pcie
> > -
> >    reg:
> >      items:
> >        - description: Data Bus Interface (DBI) registers.
> 
> 
> So the only common part left here is reg and phy? I don't think such
> common file brings any value.
> 
> 

Okay, will keep two separate files. 

> Best regards,
> Krzysztof

[PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC

[Shradha Todi]

Add Tesla FSD SoC support for both RC and EP.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 .../bindings/pci/tesla,fsd-pcie-ep.yaml       | 91 +++++++++++++++++++
 .../bindings/pci/tesla,fsd-pcie.yaml          | 77 ++++++++++++++++
 2 files changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
new file mode 100644
index 000000000000..8dfe0720e6ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tesla FSD SoC series PCIe Endpoint Controller
+
+maintainers:
+  - Shradha Todi <shradha.t@samsung.com>
+
+description:
+  Tesla FSD SoCs PCIe endpoint controller is based on the Synopsys DesignWare
+  PCIe IP and thus inherits all the common properties defined in
+  snps,dw-pcie-ep.yaml
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
+
+properties:
+  compatible:
+    const: tesla,fsd-pcie-ep
+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: elbi
+      - const: dbi
+      - const: dbi2
+      - const: addr_space
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: aux
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  num-lanes:
+    maximum: 4
+
+  phys:
+    maxItems: 1
+
+  samsung,syscon-pcie:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: phandle for system control registers, used to
+                 control signals at system level
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - num-lanes
+  - phys
+  - samsung,syscon-pcie
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/fsd-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pcieep0: pcie-ep@16a00000 {
+            compatible = "tesla,fsd-pcie-ep";
+            reg = <0x0 0x168b0000 0x0 0x1000>,
+                  <0x0 0x16a00000 0x0 0x2000>,
+                  <0x0 0x16a01000 0x0 0x80>,
+                  <0x0 0x17000000 0x0 0xff0000>;
+            reg-names = "elbi", "dbi", "dbi2", "addr_space";
+            clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+            clock-names = "aux", "dbi", "mstr", "slv";
+            num-lanes = <4>;
+            phys = <&pciephy1>;
+            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
new file mode 100644
index 000000000000..533870ab1d73
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tesla FSD SoC series PCIe Host Controller
+
+maintainers:
+  - Shradha Todi <shradha.t@samsung.com>
+
+description:
+  Tesla FSD SoCs PCIe host controller inherits all the common
+  properties defined in samsung,exynos-pcie.yaml
+
+allOf:
+  - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
+
+properties:
+  compatible:
+    const: tesla,fsd-pcie
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: aux
+      - const: dbi
+      - const: mstr
+      - const: slv
+
+  num-lanes:
+    maximum: 4
+
+  samsung,syscon-pcie:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: phandle for system control registers, used to
+                 control signals at system level
+
+required:
+  - samsung,syscon-pcie
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/fsd-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcierc1: pcie@16b00000 {
+            compatible = "tesla,fsd-pcie";
+            reg = <0x0 0x16b00000 0x0 0x2000>,
+                  <0x0 0x168c0000 0x0 0x1000>,
+                  <0x0 0x18000000 0x0 0x1000>;
+            reg-names = "dbi", "elbi", "config";
+            ranges =  <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
+            clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+            clock-names = "aux", "dbi", "mstr", "slv";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            dma-coherent;
+            device_type = "pci";
+            interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
+            num-lanes = <4>;
+            phys = <&pciephy1>;
+            samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+        };
+    };
+...
-- 
2.49.0

Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC

[Krzysztof Kozlowski]

On 11/08/2025 17:46, Shradha Todi wrote:
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: aux
> +      - const: dbi
> +      - const: mstr
> +      - const: slv
> +
> +  num-lanes:
> +    maximum: 4
> +
> +  phys:
> +    maxItems: 1
> +
> +  samsung,syscon-pcie:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: phandle for system control registers, used to
> +                 control signals at system level

What is "system level"? and what are these "signals" being controlled?


> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - num-lanes
> +  - phys
> +  - samsung,syscon-pcie
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/fsd-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        pcieep0: pcie-ep@16a00000 {
> +            compatible = "tesla,fsd-pcie-ep";
> +            reg = <0x0 0x168b0000 0x0 0x1000>,
> +                  <0x0 0x16a00000 0x0 0x2000>,
> +                  <0x0 0x16a01000 0x0 0x80>,
> +                  <0x0 0x17000000 0x0 0xff0000>;
> +            reg-names = "elbi", "dbi", "dbi2", "addr_space";
> +            clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
> +            clock-names = "aux", "dbi", "mstr", "slv";
> +            num-lanes = <4>;
> +            phys = <&pciephy1>;
> +            samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
> new file mode 100644
> index 000000000000..533870ab1d73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/tesla,fsd-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tesla FSD SoC series PCIe Host Controller
> +
> +maintainers:
> +  - Shradha Todi <shradha.t@samsung.com>
> +
> +description:
> +  Tesla FSD SoCs PCIe host controller inherits all the common
> +  properties defined in samsung,exynos-pcie.yaml
> +
> +allOf:
> +  - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    const: tesla,fsd-pcie
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: aux
> +      - const: dbi
> +      - const: mstr
> +      - const: slv
> +
> +  num-lanes:
> +    maximum: 4
> +
> +  samsung,syscon-pcie:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: phandle for system control registers, used to
> +                 control signals at system level
> +
> +required:
> +  - samsung,syscon-pcie

clocks are required, compatible as well.

Missing supplies, both as properties and required. PCI devices do not
work without power.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/fsd-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcierc1: pcie@16b00000 {
> +            compatible = "tesla,fsd-pcie";
> +            reg = <0x0 0x16b00000 0x0 0x2000>,
> +                  <0x0 0x168c0000 0x0 0x1000>,
> +                  <0x0 0x18000000 0x0 0x1000>;
> +            reg-names = "dbi", "elbi", "config";
> +            ranges =  <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;

Misaligned. Follow closely DTS coding style.

> +            clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
> +            clock-names = "aux", "dbi", "mstr", "slv";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            dma-coherent;
> +            device_type = "pci";
> +            interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
> +            num-lanes = <4>;
> +            phys = <&pciephy1>;
> +            samsung,syscon-pcie = <&sysreg_fsys1 0x510>;

Incomplete, missing supplies.

> +        };
> +    };
> +...


Best regards,
Krzysztof

RE: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC

[Shradha Todi]

> > +
> > +  phys:
> > +    maxItems: 1
> > +
> > +  samsung,syscon-pcie:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: phandle for system control registers, used to
> > +                 control signals at system level
> 
> What is "system level"? and what are these "signals" being controlled?
> 

I will add a more detailed description for why the syscon is being used

> 
> > +title: Tesla FSD SoC series PCIe Host Controller
> > +
> > +maintainers:
> > +  - Shradha Todi <shradha.t@samsung.com>
> > +
> > +description:
> > +  Tesla FSD SoCs PCIe host controller inherits all the common
> > +  properties defined in samsung,exynos-pcie.yaml
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: tesla,fsd-pcie
> > +
> > +  clocks:
> > +    maxItems: 4
> > +
> > +  clock-names:
> > +    items:
> > +      - const: aux
> > +      - const: dbi
> > +      - const: mstr
> > +      - const: slv
> > +
> > +  num-lanes:
> > +    maximum: 4
> > +
> > +  samsung,syscon-pcie:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: phandle for system control registers, used to
> > +                 control signals at system level
> > +
> > +required:
> > +  - samsung,syscon-pcie
> 
> clocks are required, compatible as well.
> 

Since this was inheriting the common exynos yaml file and that had these properties
under required, I did not mention again. Will take care in next version.

> Missing supplies, both as properties and required. PCI devices do not
> work without power.
> 

According to the HW design of FSD SoC, the control to manage PCIe power is given to
a separate CPU where custom firmware runs. Therefore, the Linux side does not control
the PCIe power supplies directly and are hence not included in the device tree.

> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/fsd-clk.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pcierc1: pcie@16b00000 {
> > +            compatible = "tesla,fsd-pcie";
> > +            reg = <0x0 0x16b00000 0x0 0x2000>,
> > +                  <0x0 0x168c0000 0x0 0x1000>,
> > +                  <0x0 0x18000000 0x0 0x1000>;
> > +            reg-names = "dbi", "elbi", "config";
> > +            ranges =  <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
> 
> Misaligned. Follow closely DTS coding style.
> 

Will take care.

Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC

[Manivannan Sadhasivam]

On Mon, Aug 18, 2025 at 02:16:16PM GMT, Shradha Todi wrote:
> > > +
> > > +  phys:
> > > +    maxItems: 1
> > > +
> > > +  samsung,syscon-pcie:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > +    description: phandle for system control registers, used to
> > > +                 control signals at system level
> > 
> > What is "system level"? and what are these "signals" being controlled?
> > 
> 
> I will add a more detailed description for why the syscon is being used
> 
> > 
> > > +title: Tesla FSD SoC series PCIe Host Controller
> > > +
> > > +maintainers:
> > > +  - Shradha Todi <shradha.t@samsung.com>
> > > +
> > > +description:
> > > +  Tesla FSD SoCs PCIe host controller inherits all the common
> > > +  properties defined in samsung,exynos-pcie.yaml
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/pci/samsung,exynos-pcie.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: tesla,fsd-pcie
> > > +
> > > +  clocks:
> > > +    maxItems: 4
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: aux
> > > +      - const: dbi
> > > +      - const: mstr
> > > +      - const: slv
> > > +
> > > +  num-lanes:
> > > +    maximum: 4
> > > +
> > > +  samsung,syscon-pcie:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > +    description: phandle for system control registers, used to
> > > +                 control signals at system level
> > > +
> > > +required:
> > > +  - samsung,syscon-pcie
> > 
> > clocks are required, compatible as well.
> > 
> 
> Since this was inheriting the common exynos yaml file and that had these properties
> under required, I did not mention again. Will take care in next version.
> 

dma-coherent needs to be a required property as well since this binding is
supporting only one controller, that seem to have cache coherent DMA.

> > Missing supplies, both as properties and required. PCI devices do not
> > work without power.
> > 
> 
> According to the HW design of FSD SoC, the control to manage PCIe power is given to
> a separate CPU where custom firmware runs. Therefore, the Linux side does not control
> the PCIe power supplies directly and are hence not included in the device tree.

What do you mean by 'PCIe power'? Supply to the PCIe controller/bus or the
devices connected to the bus?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 07/12] dt-bindings: PCI: Add support for Tesla FSD SoC

[Manivannan Sadhasivam]

On Mon, Aug 11, 2025 at 09:16:33PM GMT, Shradha Todi wrote:
> Add Tesla FSD SoC support for both RC and EP.

Add some info about the PCIe controller here. Like the data rate supported,
lanes, interrupts, any quirks etc...

> 
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  .../bindings/pci/tesla,fsd-pcie-ep.yaml       | 91 +++++++++++++++++++
>  .../bindings/pci/tesla,fsd-pcie.yaml          | 77 ++++++++++++++++
>  2 files changed, 168 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie-ep.yaml
>  create mode 100644 Documentation/devicetree/bindings/pci/tesla,fsd-pcie.yaml
> 

[...]

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/fsd-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcierc1: pcie@16b00000 {
> +            compatible = "tesla,fsd-pcie";
> +            reg = <0x0 0x16b00000 0x0 0x2000>,
> +                  <0x0 0x168c0000 0x0 0x1000>,
> +                  <0x0 0x18000000 0x0 0x1000>;
> +            reg-names = "dbi", "elbi", "config";
> +            ranges =  <0x82000000 0x0 0x18001000 0x0 0x18001000 0x0 0xffefff>;
> +            clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
> +                     <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
> +            clock-names = "aux", "dbi", "mstr", "slv";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            dma-coherent;
> +            device_type = "pci";
> +            interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;

Only one SPI interrupt? What about INTx? Don't you have any external/internal
MSI controller?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for FSD SoC

[Shradha Todi]

Since Tesla FSD SoC uses Samsung PCIe PHY, add support in
exynos PCIe PHY bindings.

In Tesla FSD SoC, the two PHY instances, although having identical
hardware design and register maps, are placed in different locations
(Placement and routing) inside the SoC and have distinct
PHY-to-Controller topologies. (One instance is connected to two PCIe
controllers, while the other is connected to only one). As a result,
they experience different analog environments, including varying
channel losses and noise profiles.

Since these PHYs lack internal adaptation mechanisms and f/w based
tuning, manual register programming is required for analog tuning.
To ensure optimal signal integrity, it is essential to use different
register values for each PHY instance, despite their identical hardware
design. This is because the same register values may not be suitable
for both instances due to their differing environments and topologies.

Due to this, we are using two PHY compatibles for different PHY
instances.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 .../bindings/phy/samsung,exynos-pcie-phy.yaml | 27 +++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
index 41df8bb08ff7..6295472696db 100644
--- a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
@@ -15,10 +15,14 @@ properties:
     const: 0
 
   compatible:
-    const: samsung,exynos5433-pcie-phy
+    enum:
+      - samsung,exynos5433-pcie-phy
+      - tesla,fsd-pcie-phy0
+      - tesla,fsd-pcie-phy1
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   samsung,pmu-syscon:
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -30,6 +34,25 @@ properties:
     description: phandle for FSYS sysreg interface, used to control
                  sysreg registers bits for PCIe PHY
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - tesla,fsd-pcie-phy0
+              - tesla,fsd-pcie-phy1
+    then:
+      properties:
+        reg:
+          items:
+            - description: PHY
+            - description: PCS
+    else:
+      properties:
+        reg:
+          maxItems: 1
+
 required:
   - "#phy-cells"
   - compatible
-- 
2.49.0

Re: [PATCH v3 08/12] dt-bindings: phy: Add PCIe PHY support for FSD SoC

[Krzysztof Kozlowski]

On Mon, Aug 11, 2025 at 09:16:34PM +0530, Shradha Todi wrote:
> Since Tesla FSD SoC uses Samsung PCIe PHY, add support in
> exynos PCIe PHY bindings.
> 
> In Tesla FSD SoC, the two PHY instances, although having identical
> hardware design and register maps, are placed in different locations
> (Placement and routing) inside the SoC and have distinct
> PHY-to-Controller topologies. (One instance is connected to two PCIe
> controllers, while the other is connected to only one). As a result,
> they experience different analog environments, including varying
> channel losses and noise profiles.
> 
> Since these PHYs lack internal adaptation mechanisms and f/w based
> tuning, manual register programming is required for analog tuning.
> To ensure optimal signal integrity, it is essential to use different
> register values for each PHY instance, despite their identical hardware
> design. This is because the same register values may not be suitable
> for both instances due to their differing environments and topologies.

Would be nice if above (or most of it) would be reflected in binding
description. Please do so and:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v3 09/12] phy: exynos: Add platform device private data

[Shradha Todi]

Currently, the exynos PCIe PHY driver supports only single
phy ops making it unusable for other platforms. Add the phy_ops
as platform specific device data so as to extend this driver
to support all platforms using Samsung PHY.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/phy/samsung/phy-exynos-pcie.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
index 53c9230c2907..022222a0212a 100644
--- a/drivers/phy/samsung/phy-exynos-pcie.c
+++ b/drivers/phy/samsung/phy-exynos-pcie.c
@@ -136,6 +136,7 @@ static const struct phy_ops exynos5433_phy_ops = {
 static const struct of_device_id exynos_pcie_phy_match[] = {
 	{
 		.compatible = "samsung,exynos5433-pcie-phy",
+		.data = &exynos5433_phy_ops,
 	},
 	{},
 };
@@ -146,6 +147,11 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
 	struct exynos_pcie_phy *exynos_phy;
 	struct phy *generic_phy;
 	struct phy_provider *phy_provider;
+	const struct phy_ops *drv_data;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -ENODEV;
 
 	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
 	if (!exynos_phy)
@@ -169,7 +175,7 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(exynos_phy->fsysreg);
 	}
 
-	generic_phy = devm_phy_create(dev, dev->of_node, &exynos5433_phy_ops);
+	generic_phy = devm_phy_create(dev, dev->of_node, drv_data);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "failed to create PHY\n");
 		return PTR_ERR(generic_phy);
-- 
2.49.0

[PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC

[Shradha Todi]

Add PCIe PHY support for Tesla FSD SoC.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/phy/samsung/phy-exynos-pcie.c | 287 ++++++++++++++++++++++++++
 1 file changed, 287 insertions(+)

diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c
index 022222a0212a..5a55a22f9661 100644
--- a/drivers/phy/samsung/phy-exynos-pcie.c
+++ b/drivers/phy/samsung/phy-exynos-pcie.c
@@ -34,9 +34,105 @@
 /* PMU PCIE PHY isolation control */
 #define EXYNOS5433_PMU_PCIE_PHY_OFFSET		0x730
 
+/* FSD: PCIe PHY common registers */
+#define FSD_PCIE_PHY_TRSV_CMN_REG03	0x000c
+#define FSD_PCIE_PHY_TRSV_CMN_REG01E	0x0078
+#define FSD_PCIE_PHY_TRSV_CMN_REG02D	0x00b4
+#define FSD_PCIE_PHY_TRSV_CMN_REG031	0x00c4
+#define FSD_PCIE_PHY_TRSV_CMN_REG036	0x00d8
+#define FSD_PCIE_PHY_TRSV_CMN_REG05F	0x017c
+#define FSD_PCIE_PHY_TRSV_CMN_REG060	0x0180
+#define FSD_PCIE_PHY_TRSV_CMN_REG062	0x0188
+#define FSD_PCIE_PHY_TRSV_CMN_REG061	0x0184
+#define FSD_PCIE_PHY_AGG_BIF_RESET	0x0200
+#define FSD_PCIE_PHY_AGG_BIF_CLOCK	0x0208
+#define FSD_PCIE_PHY_CMN_RESET		0x0228
+
+/* FSD: PCIe PHY lane registers */
+#define FSD_PCIE_LANE_OFFSET		0x0400
+#define FSD_PCIE_NUM_LANES		0x4
+
+#define FSD_PCIE_PHY_TRSV_REG001_LN_N	0x0404
+#define FSD_PCIE_PHY_TRSV_REG002_LN_N	0x0408
+#define FSD_PCIE_PHY_TRSV_REG005_LN_N	0x0414
+#define FSD_PCIE_PHY_TRSV_REG006_LN_N	0x0418
+#define FSD_PCIE_PHY_TRSV_REG007_LN_N	0x041c
+#define FSD_PCIE_PHY_TRSV_REG009_LN_N	0x0424
+#define FSD_PCIE_PHY_TRSV_REG00A_LN_N	0x0428
+#define FSD_PCIE_PHY_TRSV_REG00C_LN_N	0x0430
+#define FSD_PCIE_PHY_TRSV_REG012_LN_N	0x0448
+#define FSD_PCIE_PHY_TRSV_REG013_LN_N	0x044c
+#define FSD_PCIE_PHY_TRSV_REG014_LN_N	0x0450
+#define FSD_PCIE_PHY_TRSV_REG015_LN_N	0x0454
+#define FSD_PCIE_PHY_TRSV_REG016_LN_N	0x0458
+#define FSD_PCIE_PHY_TRSV_REG018_LN_N	0x0460
+#define FSD_PCIE_PHY_TRSV_REG020_LN_N	0x0480
+#define FSD_PCIE_PHY_TRSV_REG026_LN_N	0x0498
+#define FSD_PCIE_PHY_TRSV_REG029_LN_N	0x04a4
+#define FSD_PCIE_PHY_TRSV_REG031_LN_N	0x04c4
+#define FSD_PCIE_PHY_TRSV_REG036_LN_N	0x04d8
+#define FSD_PCIE_PHY_TRSV_REG039_LN_N	0x04e4
+#define FSD_PCIE_PHY_TRSV_REG03B_LN_N	0x04ec
+#define FSD_PCIE_PHY_TRSV_REG03C_LN_N	0x04f0
+#define FSD_PCIE_PHY_TRSV_REG03E_LN_N	0x04f8
+#define FSD_PCIE_PHY_TRSV_REG03F_LN_N	0x04fc
+#define FSD_PCIE_PHY_TRSV_REG043_LN_N	0x050c
+#define FSD_PCIE_PHY_TRSV_REG044_LN_N	0x0510
+#define FSD_PCIE_PHY_TRSV_REG046_LN_N	0x0518
+#define FSD_PCIE_PHY_TRSV_REG048_LN_N	0x0520
+#define FSD_PCIE_PHY_TRSV_REG049_LN_N	0x0524
+#define FSD_PCIE_PHY_TRSV_REG04E_LN_N	0x0538
+#define FSD_PCIE_PHY_TRSV_REG052_LN_N	0x0548
+#define FSD_PCIE_PHY_TRSV_REG068_LN_N	0x05a0
+#define FSD_PCIE_PHY_TRSV_REG069_LN_N	0x05a4
+#define FSD_PCIE_PHY_TRSV_REG06A_LN_N	0x05a8
+#define FSD_PCIE_PHY_TRSV_REG06B_LN_N	0x05ac
+#define FSD_PCIE_PHY_TRSV_REG07B_LN_N	0x05ec
+#define FSD_PCIE_PHY_TRSV_REG083_LN_N	0x060c
+#define FSD_PCIE_PHY_TRSV_REG084_LN_N	0x0610
+#define FSD_PCIE_PHY_TRSV_REG086_LN_N	0x0618
+#define FSD_PCIE_PHY_TRSV_REG087_LN_N	0x061c
+#define FSD_PCIE_PHY_TRSV_REG08B_LN_N	0x062c
+#define FSD_PCIE_PHY_TRSV_REG09C_LN_N	0x0670
+#define FSD_PCIE_PHY_TRSV_REG09D_LN_N	0x0674
+#define FSD_PCIE_PHY_TRSV_REG09E_LN_N	0x0678
+#define FSD_PCIE_PHY_TRSV_REG09F_LN_N	0x067c
+#define FSD_PCIE_PHY_TRSV_REG0A2_LN_N	0x0688
+#define FSD_PCIE_PHY_TRSV_REG0A4_LN_N	0x0690
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_N	0x0738
+#define FSD_PCIE_PHY_TRSV_REG0FC_LN_N	0x07f0
+#define FSD_PCIE_PHY_TRSV_REG0FD_LN_N	0x07f4
+#define FSD_PCIE_PHY_TRSV_REG0FE_LN_N	0x07f8
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_1	0x0b38
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_2	0x0f38
+#define FSD_PCIE_PHY_TRSV_REG0CE_LN_3	0x1338
+
+/* FSD: PCIe PCS registers */
+#define FSD_PCIE_PCS_BRF_0		0x0004
+#define FSD_PCIE_PCS_BRF_1		0x0804
+#define FSD_PCIE_PCS_CLK		0x0180
+
+/* FSD: PCIe SYSREG registers */
+#define FSD_PCIE_SYSREG_PHY_0_CON			0x042c
+#define FSD_PCIE_SYSREG_PHY_0_CON_MASK			0x03ff
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)
+#define FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK		0x3
+#define FSD_PCIE_SYSREG_PHY_0_AUX_EN			BIT(4)
+#define FSD_PCIE_SYSREG_PHY_0_CMN_RSTN			BIT(8)
+#define FSD_PCIE_SYSREG_PHY_0_INIT_RSTN			BIT(9)
+
+#define FSD_PCIE_SYSREG_PHY_1_CON			0x0500
+#define FSD_PCIE_SYSREG_PHY_1_CON_MASK			0x01ff
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL			(0x2 << 4)
+#define FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK		0x30
+#define FSD_PCIE_SYSREG_PHY_1_AUX_EN			BIT(0)
+#define FSD_PCIE_SYSREG_PHY_1_CMN_RSTN			BIT(1)
+#define FSD_PCIE_SYSREG_PHY_1_INIT_RSTN			BIT(3)
+
 /* For Exynos pcie phy */
 struct exynos_pcie_phy {
 	void __iomem *base;
+	void __iomem *pcs_base;
 	struct regmap *pmureg;
 	struct regmap *fsysreg;
 };
@@ -133,11 +229,200 @@ static const struct phy_ops exynos5433_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static void fsd_pcie_phy_writel(struct exynos_pcie_phy *phy_ctrl, u32 offset, u32 val)
+{
+	void __iomem *phy_base = phy_ctrl->base;
+	u32 i;
+
+	for (i = 0; i < FSD_PCIE_NUM_LANES; i++)
+		writel(val, phy_base + (offset + i * FSD_PCIE_LANE_OFFSET));
+}
+
+static int fsd_pcie_phy0_reset(struct phy *phy)
+{
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+
+	writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
+
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+			FSD_PCIE_SYSREG_PHY_0_CON_MASK, 0x0);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+		FSD_PCIE_SYSREG_PHY_0_AUX_EN, FSD_PCIE_SYSREG_PHY_0_AUX_EN);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+		FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_0_REF_SEL);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+		FSD_PCIE_SYSREG_PHY_0_INIT_RSTN, FSD_PCIE_SYSREG_PHY_0_INIT_RSTN);
+
+	return 0;
+}
+
+static int fsd_pcie_phy1_reset(struct phy *phy)
+{
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+
+	writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
+
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+			FSD_PCIE_SYSREG_PHY_1_CON_MASK, 0x0);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+		FSD_PCIE_SYSREG_PHY_1_AUX_EN, FSD_PCIE_SYSREG_PHY_1_AUX_EN);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+		FSD_PCIE_SYSREG_PHY_1_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_1_REF_SEL);
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+		FSD_PCIE_SYSREG_PHY_1_INIT_RSTN, FSD_PCIE_SYSREG_PHY_1_INIT_RSTN);
+
+	return 0;
+}
+
+static int fsd_pcie_phy0_init(struct phy *phy)
+{
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+	void __iomem *pbase = phy_ctrl->base;
+
+	fsd_pcie_phy0_reset(phy);
+
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
+	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
+	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
+
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
+	writel(0x11, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
+	writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
+	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
+	writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0CE_LN_N, 0x8);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0xf6);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x04);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG049_LN_N, 0x6f);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x18);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG086_LN_N, 0xff);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x3c);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09D_LN_N, 0x7c);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09F_LN_N, 0x33);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG002_LN_N, 0x1c);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG006_LN_N, 0x3);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG007_LN_N, 0x0c);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG009_LN_N, 0x10);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG00A_LN_N, 0x1);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG00C_LN_N, 0x93);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG012_LN_N, 0x1);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG013_LN_N, 0x0);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG014_LN_N, 0x70);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG015_LN_N, 0x0);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG016_LN_N, 0x70);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FC_LN_N, 0x80);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FD_LN_N, 0x0);
+
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
+		FSD_PCIE_SYSREG_PHY_0_CMN_RSTN, FSD_PCIE_SYSREG_PHY_0_CMN_RSTN);
+
+	return 0;
+}
+
+static int fsd_pcie_phy1_init(struct phy *phy)
+{
+	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
+	void __iomem *pbase = phy_ctrl->base;
+
+	fsd_pcie_phy1_reset(phy);
+
+	writel(0x2, pbase + FSD_PCIE_PHY_CMN_RESET);
+
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
+	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
+	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
+	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
+
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
+	writel(0xaa, pbase + FSD_PCIE_PHY_TRSV_CMN_REG01E);
+	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG02D);
+	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG031);
+	writel(0x21, pbase + FSD_PCIE_PHY_TRSV_CMN_REG036);
+	writel(0x12, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
+	writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
+	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG061);
+	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
+	writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG039_LN_N, 0xf);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03B_LN_N, 0x13);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03C_LN_N, 0x66);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG044_LN_N, 0x57);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03E_LN_N, 0x10);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG03F_LN_N, 0x44);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG043_LN_N, 0x11);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG046_LN_N, 0xef);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG048_LN_N, 0x06);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG049_LN_N, 0xaf);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG04E_LN_N, 0x28);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG068_LN_N, 0x1f);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG069_LN_N, 0xc);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06A_LN_N, 0x8);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG06B_LN_N, 0x78);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG083_LN_N, 0xa);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG084_LN_N, 0x80);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG087_LN_N, 0x30);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG08B_LN_N, 0xa0);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09C_LN_N, 0xf7);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG09E_LN_N, 0x33);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0A2_LN_N, 0xfa);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0A4_LN_N, 0xf2);
+	writel(0x8, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_N);
+	writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_1);
+	writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_2);
+	writel(0x9, pbase + FSD_PCIE_PHY_TRSV_REG0CE_LN_3);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG0FE_LN_N, 0x33);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG001_LN_N, 0x3f);
+	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG005_LN_N, 0x2b);
+
+	writel(0x3, pbase + FSD_PCIE_PHY_CMN_RESET);
+
+	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_1_CON,
+		FSD_PCIE_SYSREG_PHY_1_CMN_RSTN, FSD_PCIE_SYSREG_PHY_1_CMN_RSTN);
+
+	return 0;
+}
+
+static const struct phy_ops fsd_phy0_ops = {
+	.init		= fsd_pcie_phy0_init,
+	.reset		= fsd_pcie_phy0_reset,
+	.owner		= THIS_MODULE,
+};
+
+static const struct phy_ops fsd_phy1_ops = {
+	.init		= fsd_pcie_phy1_init,
+	.reset		= fsd_pcie_phy1_reset,
+	.owner		= THIS_MODULE,
+};
+
 static const struct of_device_id exynos_pcie_phy_match[] = {
 	{
 		.compatible = "samsung,exynos5433-pcie-phy",
 		.data = &exynos5433_phy_ops,
 	},
+	{
+		.compatible = "tesla,fsd-pcie-phy0",
+		.data = &fsd_phy0_ops,
+	},
+	{
+		.compatible = "tesla,fsd-pcie-phy1",
+		.data = &fsd_phy1_ops,
+	},
 	{},
 };
 
@@ -181,6 +466,8 @@ static int exynos_pcie_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(generic_phy);
 	}
 
+	exynos_phy->pcs_base = devm_platform_ioremap_resource(pdev, 1);
+
 	phy_set_drvdata(generic_phy, exynos_phy);
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
-- 
2.49.0

Re: [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC

[Vinod Koul]

On 11-08-25, 21:16, Shradha Todi wrote:
> Add PCIe PHY support for Tesla FSD SoC.

Can you pls add a bit more description of what you are adding, helps to
understand the change

> +/* FSD: PCIe PCS registers */
> +#define FSD_PCIE_PCS_BRF_0		0x0004
> +#define FSD_PCIE_PCS_BRF_1		0x0804
> +#define FSD_PCIE_PCS_CLK		0x0180
> +
> +/* FSD: PCIe SYSREG registers */
> +#define FSD_PCIE_SYSREG_PHY_0_CON			0x042c
> +#define FSD_PCIE_SYSREG_PHY_0_CON_MASK			0x03ff
> +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)

Use GENMASK() please here and elsewhere

> +static int fsd_pcie_phy0_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
> +
> +	writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
> +
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +			FSD_PCIE_SYSREG_PHY_0_CON_MASK, 0x0);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_AUX_EN, FSD_PCIE_SYSREG_PHY_0_AUX_EN);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_0_REF_SEL);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_INIT_RSTN, FSD_PCIE_SYSREG_PHY_0_INIT_RSTN);

pls conform to coding style for these

> +
> +	return 0;

why return a value when this wont ever return anything else than 0?

> +
> +	writel(0x2, pbase + FSD_PCIE_PHY_CMN_RESET);
> +
> +	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
> +	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
> +	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
> +	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
> +
> +	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
> +	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
> +	writel(0xaa, pbase + FSD_PCIE_PHY_TRSV_CMN_REG01E);
> +	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG02D);
> +	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG031);
> +	writel(0x21, pbase + FSD_PCIE_PHY_TRSV_CMN_REG036);
> +	writel(0x12, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
> +	writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
> +	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG061);
> +	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
> +	writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);

Magic numbers?
-- 
~Vinod

[PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

Add host and endpoint controller driver support for FSD SoC.

Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
 1 file changed, 278 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index ef1f42236575..9aabfecdc147 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -18,6 +18,8 @@
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 
@@ -49,15 +51,35 @@
 #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
 #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
 
+#define FSD_IRQ2_STS				0x008
+#define FSD_IRQ_MSI_ENABLE			BIT(17)
+#define FSD_IRQ2_EN				0x018
+#define FSD_PCIE_APP_LTSSM_ENABLE		0x054
+#define FSD_PCIE_LTSSM_ENABLE			0x1
+#define FSD_PCIE_DEVICE_TYPE			0x080
+#define FSD_DEVICE_TYPE_RC			0x4
+#define FSD_DEVICE_TYPE_EP			0x0
+#define FSD_PCIE_CXPL_DEBUG_00_31		0x2c8
+
 /* to store different SoC variants of Samsung */
 enum samsung_pcie_variants {
+	FSD,
 	EXYNOS_5433,
 };
 
+/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
+enum fsd_pcie_addr_type {
+	ADDR_TYPE_DBI = 0x0,
+	ADDR_TYPE_DBI2 = 0x12,
+	ADDR_TYPE_ATU = 0x36,
+	ADDR_TYPE_DMA = 0x3f,
+};
+
 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 dw_pcie_ep_ops		*ep_ops;
 	const struct samsung_res_ops		*res_ops;
 	unsigned int				soc_variant;
 	enum dw_pcie_device_mode		device_mode;
@@ -67,6 +89,8 @@ struct exynos_pcie {
 	struct dw_pcie			pci;
 	void __iomem			*elbi_base;
 	const struct samsung_pcie_pdata	*pdata;
+	struct regmap			*sysreg;
+	unsigned int			sysreg_offset;
 	struct clk_bulk_data		*clks;
 	struct phy			*phy;
 	struct regulator_bulk_data	*supplies;
@@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
 	.start_link = exynos_pcie_start_link,
 };
 
+static void fsd_pcie_stop_link(struct dw_pcie *pci)
+{
+	u32 val;
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+	val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+	val &= ~FSD_PCIE_LTSSM_ENABLE;
+	writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+}
+
+static int fsd_pcie_start_link(struct dw_pcie *pci)
+{
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+	struct dw_pcie_ep *dw_ep = &pci->ep;
+
+	if (dw_pcie_link_up(pci))
+		return 0;
+
+	writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
+
+	/* no need to wait for link in case of host as core files take care */
+	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
+		return 0;
+
+	/* check if the link is up or not in case of EP */
+	if (!dw_pcie_wait_for_link(pci)) {
+		dw_pcie_ep_linkup(dw_ep);
+		return 0;
+	}
+
+	return -ETIMEDOUT;
+}
+
+static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
+{
+	u32 val;
+	struct exynos_pcie *ep = arg;
+	struct dw_pcie *pci = &ep->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+
+	val = readl(ep->elbi_base + FSD_IRQ2_STS);
+	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
+		val &= FSD_IRQ_MSI_ENABLE;
+		writel(val, ep->elbi_base + FSD_IRQ2_STS);
+		dw_handle_msi_irq(pp);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void fsd_pcie_msi_init(struct exynos_pcie *ep)
+{
+	int val;
+
+	val = readl(ep->elbi_base + FSD_IRQ2_EN);
+	val |= FSD_IRQ_MSI_ENABLE;
+	writel(val, ep->elbi_base + FSD_IRQ2_EN);
+}
+
+static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
+{
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+	if (base >= pci->atu_base) {
+		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
+		return (base - DEFAULT_DBI_ATU_OFFSET);
+	} else if (base == pci->dbi_base) {
+		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
+	} else if (base == pci->dbi_base2) {
+		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
+	}
+
+	return base;
+}
+
+static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
+				u32 reg, size_t size)
+{
+	void __iomem *addr;
+	u32 val;
+
+	addr = fsd_atu_setting(pci, base);
+	dw_pcie_read(addr + reg, size, &val);
+
+	return val;
+}
+
+static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
+				u32 reg, size_t size, u32 val)
+{
+	void __iomem *addr;
+
+	addr = fsd_atu_setting(pci, base);
+	dw_pcie_write(addr + reg, size, val);
+}
+
+static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
+				u32 reg, size_t size, u32 val)
+{
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+	fsd_atu_setting(pci, base);
+	dw_pcie_write(pci->dbi_base + reg, size, val);
+	regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
+}
+
+static bool fsd_pcie_link_up(struct dw_pcie *pci)
+{
+	u32 val;
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+	val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
+	val &= PORT_LOGIC_LTSSM_STATE_MASK;
+
+	return (val == PORT_LOGIC_LTSSM_STATE_L0);
+}
+
+static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct exynos_pcie *ep = to_exynos_pcie(pci);
+
+	phy_init(ep->phy);
+	fsd_pcie_msi_init(ep);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
+	.init = fsd_pcie_host_init,
+};
+
+static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				 unsigned int type, u16 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_IRQ_INTX:
+		return dw_pcie_ep_raise_intx_irq(ep, func_no);
+	case PCI_IRQ_MSIX:
+		dev_err(pci->dev, "EP does not support MSI-X\n");
+		return -EINVAL;
+	case PCI_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	default:
+		dev_err(pci->dev, "UNKNOWN IRQ type\n");
+	}
+
+	return 0;
+}
+
+static const struct pci_epc_features fsd_pcie_epc_features = {
+	.linkup_notifier = false,
+	.msi_capable = true,
+	.msix_capable = false,
+};
+
+static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
+{
+	return &fsd_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops fsd_ep_ops = {
+	.raise_irq	= fsd_pcie_raise_irq,
+	.get_features	= fsd_pcie_get_features,
+};
+
+static void fsd_set_device_mode(struct exynos_pcie *ep)
+{
+	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
+		writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
+	else
+		writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
+}
+
+static const struct dw_pcie_ops fsd_dw_pcie_ops = {
+	.read_dbi	= fsd_pcie_read_dbi,
+	.write_dbi	= fsd_pcie_write_dbi,
+	.write_dbi2	= fsd_pcie_write_dbi2,
+	.start_link	= fsd_pcie_start_link,
+	.stop_link	= fsd_pcie_stop_link,
+	.link_up	= fsd_pcie_link_up,
+};
+
 static const struct samsung_res_ops exynos_res_ops_data = {
 	.init_regulator		= exynos_init_regulator,
 	.pcie_irq_handler	= exynos_pcie_irq_handler,
 };
 
+static const struct samsung_res_ops fsd_res_ops_data = {
+	.pcie_irq_handler	= fsd_pcie_irq_handler,
+	.set_device_mode	= fsd_set_device_mode,
+};
+
 static int exynos_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(ep->phy))
 		return PTR_ERR(ep->phy);
 
+	if (ep->pdata->soc_variant == FSD) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
+		if (ret)
+			return ret;
+
+		ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+				"samsung,syscon-pcie");
+		if (IS_ERR(ep->sysreg)) {
+			dev_err(dev, "sysreg regmap lookup failed.\n");
+			return PTR_ERR(ep->sysreg);
+		}
+
+		ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
+						 &ep->sysreg_offset);
+		if (ret) {
+			dev_err(dev, "couldn't get the register offset for syscon!\n");
+			return ret;
+		}
+	}
+
 	/* External Local Bus interface (ELBI) registers */
 	ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
 	if (IS_ERR(ep->elbi_base))
@@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto fail_phy_init;
 
+		break;
+	case DW_PCIE_EP_TYPE:
+		phy_init(ep->phy);
+
+		ep->pci.ep.ops = pdata->ep_ops;
+
+		ret = dw_pcie_ep_init(&ep->pci.ep);
+		if (ret < 0)
+			goto fail_phy_init;
+
+		ret = dw_pcie_ep_init_registers(&ep->pci.ep);
+		if (ret)
+			goto fail_phy_init;
+
+		pci_epc_init_notify(ep->pci.ep.epc);
+
 		break;
 	default:
 		dev_err(dev, "invalid device type\n");
@@ -436,6 +686,9 @@ static int exynos_pcie_suspend_noirq(struct device *dev)
 	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
 		return 0;
 
+	if (ep->pdata->dwc_ops->stop_link)
+		ep->pdata->dwc_ops->stop_link(pci);
+
 	if (ep->pdata->soc_variant == EXYNOS_5433)
 		exynos_pcie_assert_core_reset(ep);
 	phy_power_off(ep->phy);
@@ -471,6 +724,23 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
 				  exynos_pcie_resume_noirq)
 };
 
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_rc_pdata = {
+	.dwc_ops		= &fsd_dw_pcie_ops,
+	.host_ops		= &fsd_pcie_host_ops,
+	.res_ops		= &fsd_res_ops_data,
+	.soc_variant		= FSD,
+	.device_mode		= DW_PCIE_RC_TYPE,
+};
+
+static const struct samsung_pcie_pdata fsd_hw3_pcie_ep_pdata = {
+	.dwc_ops		= &fsd_dw_pcie_ops,
+	.ep_ops			= &fsd_ep_ops,
+	.res_ops		= &fsd_res_ops_data,
+	.soc_variant		= FSD,
+	.device_mode		= DW_PCIE_EP_TYPE,
+};
+
 static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
 	.dwc_ops		= &exynos_dw_pcie_ops,
 	.pci_ops		= &exynos_pci_ops,
@@ -485,6 +755,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
 		.compatible = "samsung,exynos5433-pcie",
 		.data = (void *) &exynos_5433_pcie_rc_pdata,
 	},
+	{
+		.compatible = "tesla,fsd-pcie",
+		.data = (void *) &fsd_hw3_pcie_rc_pdata,
+	},
+	{
+		.compatible = "tesla,fsd-pcie-ep",
+		.data = (void *) &fsd_hw3_pcie_ep_pdata,
+	},
 	{ },
 };
 
-- 
2.49.0

Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Bjorn Helgaas]

On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.

I think this might be easier if you added host mode first, then added
endpoint mode with a separate patch.

It's kind of unfortunate that the driver uses "ep" everywhere for
struct exynos_pcie pointers.  It's going to be confusing because "ep"
is also commonly used for endpoint-related things, e.g., struct
dw_pcie_ep pointers.  Maybe it's not worth changing; I dunno.

> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
>  1 file changed, 278 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index ef1f42236575..9aabfecdc147 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -18,6 +18,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/phy/phy.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  
> @@ -49,15 +51,35 @@
>  #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
>  #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
>  
> +#define FSD_IRQ2_STS				0x008
> +#define FSD_IRQ_MSI_ENABLE			BIT(17)
> +#define FSD_IRQ2_EN				0x018
> +#define FSD_PCIE_APP_LTSSM_ENABLE		0x054
> +#define FSD_PCIE_LTSSM_ENABLE			0x1
> +#define FSD_PCIE_DEVICE_TYPE			0x080
> +#define FSD_DEVICE_TYPE_RC			0x4
> +#define FSD_DEVICE_TYPE_EP			0x0
> +#define FSD_PCIE_CXPL_DEBUG_00_31		0x2c8
> +
>  /* to store different SoC variants of Samsung */
>  enum samsung_pcie_variants {
> +	FSD,
>  	EXYNOS_5433,
>  };
>  
> +/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
> +enum fsd_pcie_addr_type {
> +	ADDR_TYPE_DBI = 0x0,
> +	ADDR_TYPE_DBI2 = 0x12,
> +	ADDR_TYPE_ATU = 0x36,
> +	ADDR_TYPE_DMA = 0x3f,
> +};
> +
>  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 dw_pcie_ep_ops		*ep_ops;
>  	const struct samsung_res_ops		*res_ops;
>  	unsigned int				soc_variant;
>  	enum dw_pcie_device_mode		device_mode;
> @@ -67,6 +89,8 @@ struct exynos_pcie {
>  	struct dw_pcie			pci;
>  	void __iomem			*elbi_base;
>  	const struct samsung_pcie_pdata	*pdata;
> +	struct regmap			*sysreg;
> +	unsigned int			sysreg_offset;
>  	struct clk_bulk_data		*clks;
>  	struct phy			*phy;
>  	struct regulator_bulk_data	*supplies;
> @@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
>  	.start_link = exynos_pcie_start_link,
>  };
>  
> +static void fsd_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +	val &= ~FSD_PCIE_LTSSM_ENABLE;
> +	writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int fsd_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +	struct dw_pcie_ep *dw_ep = &pci->ep;
> +
> +	if (dw_pcie_link_up(pci))
> +		return 0;
> +
> +	writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +
> +	/* no need to wait for link in case of host as core files take care */
> +	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> +		return 0;
> +
> +	/* check if the link is up or not in case of EP */
> +	if (!dw_pcie_wait_for_link(pci)) {
> +		dw_pcie_ep_linkup(dw_ep);
> +		return 0;
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = arg;
> +	struct dw_pcie *pci = &ep->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
> +
> +	val = readl(ep->elbi_base + FSD_IRQ2_STS);
> +	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> +		val &= FSD_IRQ_MSI_ENABLE;
> +		writel(val, ep->elbi_base + FSD_IRQ2_STS);

This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
bit, but here you're treating it as a *status* bit.

As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
will never be set again.  That seems wrong; am I missing something?

> +		dw_handle_msi_irq(pp);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> +{
> +	int val;
> +
> +	val = readl(ep->elbi_base + FSD_IRQ2_EN);
> +	val |= FSD_IRQ_MSI_ENABLE;
> +	writel(val, ep->elbi_base + FSD_IRQ2_EN);
> +}
> +
> +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)

The "setting" name suggests that this merely returns an address
without side effects, but in fact it actively *sets* the view.

In this case there's no locking around:

  addr = fsd_atu_setting(pci, base);
  dw_pcie_read(addr + reg, size, &val);

even though concurrent calls would cause issues, but I think that's OK
because we only get there via the driver, and I assume multiple DBI or
DBI2 accesses never happen because they're not used in asynchronous
paths like interrupt handlers.

But I think a name that hints at the fact that this does have side
effects would be helpful as a reminder in the callers that they must
not be used concurrently.

> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	if (base >= pci->atu_base) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
> +		return (base - DEFAULT_DBI_ATU_OFFSET);
> +	} else if (base == pci->dbi_base) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +	} else if (base == pci->dbi_base2) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
> +	}
> +
> +	return base;
> +}
> +
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size)
> +{
> +	void __iomem *addr;
> +	u32 val;
> +
> +	addr = fsd_atu_setting(pci, base);
> +	dw_pcie_read(addr + reg, size, &val);
> +
> +	return val;
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	void __iomem *addr;
> +
> +	addr = fsd_atu_setting(pci, base);
> +	dw_pcie_write(addr + reg, size, val);
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	fsd_atu_setting(pci, base);
> +	dw_pcie_write(pci->dbi_base + reg, size, val);
> +	regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +
> +static bool fsd_pcie_link_up(struct dw_pcie *pci)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
> +	val &= PORT_LOGIC_LTSSM_STATE_MASK;
> +
> +	return (val == PORT_LOGIC_LTSSM_STATE_L0);
> +}
> +
> +static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	phy_init(ep->phy);
> +	fsd_pcie_msi_init(ep);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
> +	.init = fsd_pcie_host_init,
> +};
> +
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +				 unsigned int type, u16 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> +	switch (type) {
> +	case PCI_IRQ_INTX:
> +		return dw_pcie_ep_raise_intx_irq(ep, func_no);
> +	case PCI_IRQ_MSIX:
> +		dev_err(pci->dev, "EP does not support MSI-X\n");
> +		return -EINVAL;
> +	case PCI_IRQ_MSI:
> +		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> +	default:
> +		dev_err(pci->dev, "UNKNOWN IRQ type\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pci_epc_features fsd_pcie_epc_features = {
> +	.linkup_notifier = false,
> +	.msi_capable = true,
> +	.msix_capable = false,

I think we should omit features we do *not* support instead of calling
them out explicitly, e.g., we don't need .linkup_notifier or
.msix_capable.

We've added them in the past, but they're unnecessary and they lead to
either pervasive changes (adding ".new_feature = false" to all
existing drivers when adding the feature) or inconsistency (new
drivers include ".new_feature = false" but existing drivers do not).

> +};
> +
> +static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
> +{
> +	return &fsd_pcie_epc_features;
> +}
> +
> +static const struct dw_pcie_ep_ops fsd_ep_ops = {
> +	.raise_irq	= fsd_pcie_raise_irq,
> +	.get_features	= fsd_pcie_get_features,
> +};
> +
> +static void fsd_set_device_mode(struct exynos_pcie *ep)
> +{
> +	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> +		writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +	else
> +		writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +}
> +
> +static const struct dw_pcie_ops fsd_dw_pcie_ops = {
> +	.read_dbi	= fsd_pcie_read_dbi,
> +	.write_dbi	= fsd_pcie_write_dbi,
> +	.write_dbi2	= fsd_pcie_write_dbi2,
> +	.start_link	= fsd_pcie_start_link,
> +	.stop_link	= fsd_pcie_stop_link,
> +	.link_up	= fsd_pcie_link_up,
> +};
> +
>  static const struct samsung_res_ops exynos_res_ops_data = {
>  	.init_regulator		= exynos_init_regulator,
>  	.pcie_irq_handler	= exynos_pcie_irq_handler,
>  };
>  
> +static const struct samsung_res_ops fsd_res_ops_data = {
> +	.pcie_irq_handler	= fsd_pcie_irq_handler,
> +	.set_device_mode	= fsd_set_device_mode,
> +};
> +
>  static int exynos_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
>  	if (IS_ERR(ep->phy))
>  		return PTR_ERR(ep->phy);
>  
> +	if (ep->pdata->soc_variant == FSD) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> +		if (ret)
> +			return ret;
> +
> +		ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> +				"samsung,syscon-pcie");
> +		if (IS_ERR(ep->sysreg)) {
> +			dev_err(dev, "sysreg regmap lookup failed.\n");
> +			return PTR_ERR(ep->sysreg);
> +		}
> +
> +		ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> +						 &ep->sysreg_offset);
> +		if (ret) {
> +			dev_err(dev, "couldn't get the register offset for syscon!\n");
> +			return ret;
> +		}
> +	}

This is a good example of a complicated set of things where I think
you should either add a SoC-specific function pointer to do this or
test a property, e.g., "DMA width", instead of testing for a specific
SoC.

>  	/* External Local Bus interface (ELBI) registers */
>  	ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
>  	if (IS_ERR(ep->elbi_base))
> @@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto fail_phy_init;
>  
> +		break;
> +	case DW_PCIE_EP_TYPE:
> +		phy_init(ep->phy);
> +
> +		ep->pci.ep.ops = pdata->ep_ops;
> +
> +		ret = dw_pcie_ep_init(&ep->pci.ep);
> +		if (ret < 0)
> +			goto fail_phy_init;
> +
> +		ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> +		if (ret)
> +			goto fail_phy_init;
> +
> +		pci_epc_init_notify(ep->pci.ep.epc);
> +
>  		break;
>  	default:
>  		dev_err(dev, "invalid device type\n");
> @@ -436,6 +686,9 @@ static int exynos_pcie_suspend_noirq(struct device *dev)
>  	if (ep->pdata->device_mode == DW_PCIE_EP_TYPE)
>  		return 0;
>  
> +	if (ep->pdata->dwc_ops->stop_link)
> +		ep->pdata->dwc_ops->stop_link(pci);
> +
>  	if (ep->pdata->soc_variant == EXYNOS_5433)
>  		exynos_pcie_assert_core_reset(ep);
>  	phy_power_off(ep->phy);
> @@ -471,6 +724,23 @@ static const struct dev_pm_ops exynos_pcie_pm_ops = {
>  				  exynos_pcie_resume_noirq)
>  };
>  
> +
> +static const struct samsung_pcie_pdata fsd_hw3_pcie_rc_pdata = {
> +	.dwc_ops		= &fsd_dw_pcie_ops,
> +	.host_ops		= &fsd_pcie_host_ops,
> +	.res_ops		= &fsd_res_ops_data,
> +	.soc_variant		= FSD,
> +	.device_mode		= DW_PCIE_RC_TYPE,
> +};
> +
> +static const struct samsung_pcie_pdata fsd_hw3_pcie_ep_pdata = {
> +	.dwc_ops		= &fsd_dw_pcie_ops,
> +	.ep_ops			= &fsd_ep_ops,
> +	.res_ops		= &fsd_res_ops_data,
> +	.soc_variant		= FSD,
> +	.device_mode		= DW_PCIE_EP_TYPE,
> +};
> +
>  static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
>  	.dwc_ops		= &exynos_dw_pcie_ops,
>  	.pci_ops		= &exynos_pci_ops,
> @@ -485,6 +755,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
>  		.compatible = "samsung,exynos5433-pcie",
>  		.data = (void *) &exynos_5433_pcie_rc_pdata,
>  	},
> +	{
> +		.compatible = "tesla,fsd-pcie",
> +		.data = (void *) &fsd_hw3_pcie_rc_pdata,
> +	},
> +	{
> +		.compatible = "tesla,fsd-pcie-ep",
> +		.data = (void *) &fsd_hw3_pcie_ep_pdata,
> +	},
>  	{ },
>  };
>  
> -- 
> 2.49.0
> 

RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> > Add host and endpoint controller driver support for FSD SoC.
> 
> I think this might be easier if you added host mode first, then added
> endpoint mode with a separate patch.
> 

Will do.

> It's kind of unfortunate that the driver uses "ep" everywhere for
> struct exynos_pcie pointers.  It's going to be confusing because "ep"
> is also commonly used for endpoint-related things, e.g., struct
> dw_pcie_ep pointers.  Maybe it's not worth changing; I dunno.
> 

I did try to rename the structure and the pointers 
(https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
But the intention was different back then and so the idea was rejected.
I could add a patch to only rename the pointers to something less
confusing like "exy_pci"

> > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > +{
> > +	u32 val;
> > +	struct exynos_pcie *ep = arg;
> > +	struct dw_pcie *pci = &ep->pci;
> > +	struct dw_pcie_rp *pp = &pci->pp;
> > +
> > +	val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > +	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > +		val &= FSD_IRQ_MSI_ENABLE;
> > +		writel(val, ep->elbi_base + FSD_IRQ2_STS);
> 
> This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> bit, but here you're treating it as a *status* bit.
> 
> As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> will never be set again.  That seems wrong; am I missing something?
> 

Actually the status IRQ and enable IRQ registers are different offsets
but the bit position for MSI remains same in both cases so I just reused
the macro. But I understand that it's confusing so I will add another
macro for FSD_IRQ_MSI_STATUS or just rename the macro to
FSD_IRQ_MSI to re-use.

> > +		dw_handle_msi_irq(pp);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> > +{
> > +	int val;
> > +
> > +	val = readl(ep->elbi_base + FSD_IRQ2_EN);
> > +	val |= FSD_IRQ_MSI_ENABLE;
> > +	writel(val, ep->elbi_base + FSD_IRQ2_EN);
> > +}
> > +
> > +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
> 
> The "setting" name suggests that this merely returns an address
> without side effects, but in fact it actively *sets* the view.
> 
> In this case there's no locking around:
> 
>   addr = fsd_atu_setting(pci, base);
>   dw_pcie_read(addr + reg, size, &val);
> 
> even though concurrent calls would cause issues, but I think that's OK
> because we only get there via the driver, and I assume multiple DBI or
> DBI2 accesses never happen because they're not used in asynchronous
> paths like interrupt handlers.
> 

Yes, there is no concurrent access to this function and hence I have
not added locking mechanism.

> But I think a name that hints at the fact that this does have side
> effects would be helpful as a reminder in the callers that they must
> not be used concurrently.
> 

Sure, I will change the name and also add comment as a reminder.

> > +static const struct pci_epc_features fsd_pcie_epc_features = {
> > +	.linkup_notifier = false,
> > +	.msi_capable = true,
> > +	.msix_capable = false,
> 
> I think we should omit features we do *not* support instead of calling
> them out explicitly, e.g., we don't need .linkup_notifier or
> .msix_capable.
> 
> We've added them in the past, but they're unnecessary and they lead to
> either pervasive changes (adding ".new_feature = false" to all
> existing drivers when adding the feature) or inconsistency (new
> drivers include ".new_feature = false" but existing drivers do not).
> 

Will remove

> > +	if (ep->pdata->soc_variant == FSD) {
> > +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> > +		if (ret)
> > +			return ret;
> > +
> > +		ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> > +				"samsung,syscon-pcie");
> > +		if (IS_ERR(ep->sysreg)) {
> > +			dev_err(dev, "sysreg regmap lookup failed.\n");
> > +			return PTR_ERR(ep->sysreg);
> > +		}
> > +
> > +		ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> > +						 &ep->sysreg_offset);
> > +		if (ret) {
> > +			dev_err(dev, "couldn't get the register offset for syscon!\n");
> > +			return ret;
> > +		}
> > +	}
> 
> This is a good example of a complicated set of things where I think
> you should either add a SoC-specific function pointer to do this or
> test a property, e.g., "DMA width", instead of testing for a specific
> SoC.
> 

Got your point and it makes sense. In future, other drivers could also
want to set DMA width, etc. Will make properties to replace soc_variant:
 - DMA_width
 - has_syscon
 - function pointer to assert_core_reset and deassert_core_reset

Any suggestions or is this approach okay?

-Shradha

Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Bjorn Helgaas]

[+to Krzysztof]

On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
> > On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> > > Add host and endpoint controller driver support for FSD SoC.

> > It's kind of unfortunate that the driver uses "ep" everywhere for
> > struct exynos_pcie pointers.  It's going to be confusing because "ep"
> > is also commonly used for endpoint-related things, e.g., struct
> > dw_pcie_ep pointers.  Maybe it's not worth changing; I dunno.
> 
> I did try to rename the structure and the pointers 
> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
> But the intention was different back then and so the idea was rejected.
> I could add a patch to only rename the pointers to something less
> confusing like "exy_pci"

The patch you mention did several renames:

  s/to_exynos_pcie/to_samsung_pcie/
  s/struct exynos_pcie/struct samsung_pcie/
  s/struct exynos_pcie *ep/struct samsung_pcie *sp/

I'm only concerned about the confusion of "ep" being used both for
"struct exynos_pcie *" and for "struct dw_pcie_ep *".

It would still be sort of an annoying patch to do something like this:

  s/struct exynos_pcie *ep/struct exynos_pcie *pcie/

But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
using "pcie" in this way is quite common, so maybe it would be worth
doing.

What do you think, Krzysztof?

> > > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > > +{
> > > +	u32 val;
> > > +	struct exynos_pcie *ep = arg;
> > > +	struct dw_pcie *pci = &ep->pci;
> > > +	struct dw_pcie_rp *pp = &pci->pp;
> > > +
> > > +	val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > > +	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > > +		val &= FSD_IRQ_MSI_ENABLE;
> > > +		writel(val, ep->elbi_base + FSD_IRQ2_STS);
> > 
> > This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> > bit, but here you're treating it as a *status* bit.
> > 
> > As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> > fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> > will never be set again.  That seems wrong; am I missing something?
> 
> Actually the status IRQ and enable IRQ registers are different offsets
> but the bit position for MSI remains same in both cases so I just reused
> the macro.

Ah, that's what I missed, thanks!  At probe-time, fsd_pcie_msi_init()
enables it in FSD_IRQ2_EN.  Here you clear it in FSD_IRQ2_STS.

> But I understand that it's confusing so I will add another
> macro for FSD_IRQ_MSI_STATUS or just rename the macro to
> FSD_IRQ_MSI to re-use.

Using the same name just because a similar bit happens to be at the
same position in two different registers is definitely confusing.  I
think it will be better to have two macros, one for FSD_IRQ2_STS and
another for FSD_IRQ2_EN, e.g.,

  #define FSD_IRQ2_STS                         0x008
  #define   FSD_IRQ2_STS_MSI                   BIT(17)
  #define FSD_IRQ2_EN                          0x018
  #define   FSD_IRQ2_EN_MSI                    BIT(17)

Another question about the test:

  if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {

This assumes there are no other bits in FSD_IRQ2_STS that could be
set.  I would have expected a test like this:

  if (val & FSD_IRQ_MSI_ENABLE) {

Is there a reason to restrict it to the case when *only*
FSD_IRQ_MSI_ENABLE is set?

Bjorn

Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Krzysztof Kozlowski]

On 18/08/2025 20:25, Bjorn Helgaas wrote:
> [+to Krzysztof]
> 
> On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
>>> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
>>>> Add host and endpoint controller driver support for FSD SoC.
> 
>>> It's kind of unfortunate that the driver uses "ep" everywhere for
>>> struct exynos_pcie pointers.  It's going to be confusing because "ep"
>>> is also commonly used for endpoint-related things, e.g., struct
>>> dw_pcie_ep pointers.  Maybe it's not worth changing; I dunno.
>>
>> I did try to rename the structure and the pointers 
>> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
>> But the intention was different back then and so the idea was rejected.
>> I could add a patch to only rename the pointers to something less
>> confusing like "exy_pci"
> 
> The patch you mention did several renames:
> 
>   s/to_exynos_pcie/to_samsung_pcie/
>   s/struct exynos_pcie/struct samsung_pcie/
>   s/struct exynos_pcie *ep/struct samsung_pcie *sp/
> 
> I'm only concerned about the confusion of "ep" being used both for
> "struct exynos_pcie *" and for "struct dw_pcie_ep *".
> 
> It would still be sort of an annoying patch to do something like this:
> 
>   s/struct exynos_pcie *ep/struct exynos_pcie *pcie/
> 
> But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
> using "pcie" in this way is quite common, so maybe it would be worth
> doing.
> 
> What do you think, Krzysztof?

I think you want other Krzysztof, but nevertheless, the reasoning there
"Changing it to samsung_pcie for making it
generic."
is wrong. The naming of these structures do not matter, they are not
less generic. This is rather churn, which will affect backporting for
ZERO readability increase. Why zero? Because calling all this "exynos"
is the same as calling all this "samsung". It just does not matter.

However s/ep/pcie/ in variable name makes sense if that's more common.


Best regards,
Krzysztof

RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 August 2025 12:04
> To: Bjorn Helgaas <helgaas@kernel.org>; Shradha Todi <shradha.t@samsung.com>; Krzysztof
> Kozlowski <krzysztof.kozlowski@linaro.org>
> 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; linux-phy@lists.infradead.org;
> mani@kernel.org; lpieralisi@kernel.org; kwilczynski@kernel.org; robh@kernel.org;
> bhelgaas@google.com; jingoohan1@gmail.com; krzk+dt@kernel.org; conor+dt@kernel.org;
> alim.akhtar@samsung.com; vkoul@kernel.org; kishon@kernel.org; arnd@arndb.de;
> m.szyprowski@samsung.com; jh80.chung@samsung.com; pankaj.dubey@samsung.com
> Subject: Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC
> 
> On 18/08/2025 20:25, Bjorn Helgaas wrote:
> > [+to Krzysztof]
> >
> > On Mon, Aug 18, 2025 at 03:00:00PM +0530, Shradha Todi wrote:
> >>> On Mon, Aug 11, 2025 at 09:16:37PM +0530, Shradha Todi wrote:
> >>>> Add host and endpoint controller driver support for FSD SoC.
> >
> >>> It's kind of unfortunate that the driver uses "ep" everywhere for
> >>> struct exynos_pcie pointers.  It's going to be confusing because "ep"
> >>> is also commonly used for endpoint-related things, e.g., struct
> >>> dw_pcie_ep pointers.  Maybe it's not worth changing; I dunno.
> >>
> >> I did try to rename the structure and the pointers
> >> (https://lore.kernel.org/all/20230214121333.1837-9-shradha.t@samsung.com/)
> >> But the intention was different back then and so the idea was rejected.
> >> I could add a patch to only rename the pointers to something less
> >> confusing like "exy_pci"
> >
> > The patch you mention did several renames:
> >
> >   s/to_exynos_pcie/to_samsung_pcie/
> >   s/struct exynos_pcie/struct samsung_pcie/
> >   s/struct exynos_pcie *ep/struct samsung_pcie *sp/
> >
> > I'm only concerned about the confusion of "ep" being used both for
> > "struct exynos_pcie *" and for "struct dw_pcie_ep *".
> >
> > It would still be sort of an annoying patch to do something like this:
> >
> >   s/struct exynos_pcie *ep/struct exynos_pcie *pcie/
> >
> > But 'git grep "struct .*_pcie \*.*=" drivers/pci/controller/' says
> > using "pcie" in this way is quite common, so maybe it would be worth
> > doing.
> >
> > What do you think, Krzysztof?
> 
> I think you want other Krzysztof, but nevertheless, the reasoning there
> "Changing it to samsung_pcie for making it
> generic."
> is wrong. The naming of these structures do not matter, they are not
> less generic. This is rather churn, which will affect backporting for
> ZERO readability increase. Why zero? Because calling all this "exynos"
> is the same as calling all this "samsung". It just does not matter.
> 
> However s/ep/pcie/ in variable name makes sense if that's more common.
> 

I will add a patch in the series to do that.

> 
> Best regards,
> Krzysztof

RE: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Shradha Todi]

> > > > +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> > > > +{
> > > > +	u32 val;
> > > > +	struct exynos_pcie *ep = arg;
> > > > +	struct dw_pcie *pci = &ep->pci;
> > > > +	struct dw_pcie_rp *pp = &pci->pp;
> > > > +
> > > > +	val = readl(ep->elbi_base + FSD_IRQ2_STS);
> > > > +	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > > > +		val &= FSD_IRQ_MSI_ENABLE;
> > > > +		writel(val, ep->elbi_base + FSD_IRQ2_STS);
> > >
> > > This looks weird because FSD_IRQ_MSI_ENABLE sounds like an *enable*
> > > bit, but here you're treating it as a *status* bit.
> > >
> > > As far as I can tell, you set FSD_IRQ_MSI_ENABLE once at probe-time in
> > > fsd_pcie_msi_init(), then you clear it here in an IRQ handler, and it
> > > will never be set again.  That seems wrong; am I missing something?
> >
> > Actually the status IRQ and enable IRQ registers are different offsets
> > but the bit position for MSI remains same in both cases so I just reused
> > the macro.
> 
> Ah, that's what I missed, thanks!  At probe-time, fsd_pcie_msi_init()
> enables it in FSD_IRQ2_EN.  Here you clear it in FSD_IRQ2_STS.
> 
> > But I understand that it's confusing so I will add another
> > macro for FSD_IRQ_MSI_STATUS or just rename the macro to
> > FSD_IRQ_MSI to re-use.
> 
> Using the same name just because a similar bit happens to be at the
> same position in two different registers is definitely confusing.  I
> think it will be better to have two macros, one for FSD_IRQ2_STS and
> another for FSD_IRQ2_EN, e.g.,
> 
>   #define FSD_IRQ2_STS                         0x008
>   #define   FSD_IRQ2_STS_MSI                   BIT(17)
>   #define FSD_IRQ2_EN                          0x018
>   #define   FSD_IRQ2_EN_MSI                    BIT(17)
> 
> Another question about the test:
> 
>   if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> 
> This assumes there are no other bits in FSD_IRQ2_STS that could be
> set.  I would have expected a test like this:
> 
>   if (val & FSD_IRQ_MSI_ENABLE) {
> 

Thanks for pointing this out. FSD_IRQ_MSI_ENABLE is a single-bit, so there
is no functional difference in the two statements. I didn't have a specific
reason for using "== FSD_IRQ_MSI_ENABLE".
But I see that "val & FSD_IRQ_MSI_ENABLE" would have been the more
standard way to write this. I will update this for clarity.

> Is there a reason to restrict it to the case when *only*
> FSD_IRQ_MSI_ENABLE is set?
> 
> Bjorn

Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Bjorn Helgaas]

On Tue, Aug 19, 2025 at 05:09:34PM +0530, Shradha Todi wrote:
> ...

> > Another question about the test:
> > 
> >   if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> > 
> > This assumes there are no other bits in FSD_IRQ2_STS that could be
> > set.  I would have expected a test like this:
> > 
> >   if (val & FSD_IRQ_MSI_ENABLE) {
> 
> Thanks for pointing this out. FSD_IRQ_MSI_ENABLE is a single-bit, so there
> is no functional difference in the two statements. I didn't have a specific
> reason for using "== FSD_IRQ_MSI_ENABLE".
> But I see that "val & FSD_IRQ_MSI_ENABLE" would have been the more
> standard way to write this. I will update this for clarity.

Oof, sorry, I don't know what I was thinking.  You're right, it's OK
as is.  But "val & FSD_IRQ_MSI_ENABLE" *is* shorter and more
idiomatic, so I think preferable anyway.

Bjorn

Re: [PATCH v3 11/12] PCI: exynos: Add support for Tesla FSD SoC

[Manivannan Sadhasivam]

On Mon, Aug 11, 2025 at 09:16:37PM GMT, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
> 

Again, more info about the controller required.

> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/pci-exynos.c | 278 ++++++++++++++++++++++++
>  1 file changed, 278 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
> index ef1f42236575..9aabfecdc147 100644
> --- a/drivers/pci/controller/dwc/pci-exynos.c
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -18,6 +18,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/phy/phy.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  
> @@ -49,15 +51,35 @@
>  #define EXYNOS_PCIE_ELBI_SLV_ARMISC		0x120
>  #define EXYNOS_PCIE_ELBI_SLV_DBI_ENABLE		BIT(21)
>  
> +#define FSD_IRQ2_STS				0x008
> +#define FSD_IRQ_MSI_ENABLE			BIT(17)
> +#define FSD_IRQ2_EN				0x018
> +#define FSD_PCIE_APP_LTSSM_ENABLE		0x054
> +#define FSD_PCIE_LTSSM_ENABLE			0x1
> +#define FSD_PCIE_DEVICE_TYPE			0x080
> +#define FSD_DEVICE_TYPE_RC			0x4
> +#define FSD_DEVICE_TYPE_EP			0x0
> +#define FSD_PCIE_CXPL_DEBUG_00_31		0x2c8
> +
>  /* to store different SoC variants of Samsung */
>  enum samsung_pcie_variants {
> +	FSD,
>  	EXYNOS_5433,

I hope you are going to drop these variants and use the properties for specific
functionality as suggested by Bjorn.

>  };
>  
> +/* Values to be written to SYSREG to view DBI space as CDM/DBI2/IATU/DMA */
> +enum fsd_pcie_addr_type {
> +	ADDR_TYPE_DBI = 0x0,
> +	ADDR_TYPE_DBI2 = 0x12,
> +	ADDR_TYPE_ATU = 0x36,
> +	ADDR_TYPE_DMA = 0x3f,
> +};
> +
>  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 dw_pcie_ep_ops		*ep_ops;
>  	const struct samsung_res_ops		*res_ops;
>  	unsigned int				soc_variant;
>  	enum dw_pcie_device_mode		device_mode;
> @@ -67,6 +89,8 @@ struct exynos_pcie {
>  	struct dw_pcie			pci;
>  	void __iomem			*elbi_base;
>  	const struct samsung_pcie_pdata	*pdata;
> +	struct regmap			*sysreg;
> +	unsigned int			sysreg_offset;
>  	struct clk_bulk_data		*clks;
>  	struct phy			*phy;
>  	struct regulator_bulk_data	*supplies;
> @@ -334,11 +358,201 @@ static const struct dw_pcie_ops exynos_dw_pcie_ops = {
>  	.start_link = exynos_pcie_start_link,
>  };
>  
> +static void fsd_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);

Please use reverse Xmas order for local variables.

> +
> +	val = readl(ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +	val &= ~FSD_PCIE_LTSSM_ENABLE;
> +	writel(val, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int fsd_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +	struct dw_pcie_ep *dw_ep = &pci->ep;
> +
> +	if (dw_pcie_link_up(pci))
> +		return 0;
> +
> +	writel(FSD_PCIE_LTSSM_ENABLE, ep->elbi_base + FSD_PCIE_APP_LTSSM_ENABLE);
> +
> +	/* no need to wait for link in case of host as core files take care */

What do you mean by 'core files'? DWC core driver?

> +	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> +		return 0;
> +
> +	/* check if the link is up or not in case of EP */
> +	if (!dw_pcie_wait_for_link(pci)) {

I presume there is no way to detect link up using the SPI interrupt?

> +		dw_pcie_ep_linkup(dw_ep);
> +		return 0;
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static irqreturn_t fsd_pcie_irq_handler(int irq, void *arg)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = arg;
> +	struct dw_pcie *pci = &ep->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
> +
> +	val = readl(ep->elbi_base + FSD_IRQ2_STS);
> +	if ((val & FSD_IRQ_MSI_ENABLE) == FSD_IRQ_MSI_ENABLE) {
> +		val &= FSD_IRQ_MSI_ENABLE;
> +		writel(val, ep->elbi_base + FSD_IRQ2_STS);
> +		dw_handle_msi_irq(pp);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void fsd_pcie_msi_init(struct exynos_pcie *ep)
> +{
> +	int val;
> +
> +	val = readl(ep->elbi_base + FSD_IRQ2_EN);
> +	val |= FSD_IRQ_MSI_ENABLE;
> +	writel(val, ep->elbi_base + FSD_IRQ2_EN);

Could you please explain how MSI handling works? I don't understand it.

> +}
> +
> +static void __iomem *fsd_atu_setting(struct dw_pcie *pci, void __iomem *base)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	if (base >= pci->atu_base) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_ATU);
> +		return (base - DEFAULT_DBI_ATU_OFFSET);
> +	} else if (base == pci->dbi_base) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +	} else if (base == pci->dbi_base2) {
> +		regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI2);
> +	}
> +
> +	return base;
> +}
> +
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size)
> +{
> +	void __iomem *addr;
> +	u32 val;
> +
> +	addr = fsd_atu_setting(pci, base);

The returned 'addr' seems to be static. Why can't you just store it during probe
and use the 'addr' directly?

> +	dw_pcie_read(addr + reg, size, &val);
> +
> +	return val;
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	void __iomem *addr;
> +
> +	addr = fsd_atu_setting(pci, base);
> +	dw_pcie_write(addr + reg, size, val);
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	fsd_atu_setting(pci, base);
> +	dw_pcie_write(pci->dbi_base + reg, size, val);
> +	regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +
> +static bool fsd_pcie_link_up(struct dw_pcie *pci)
> +{
> +	u32 val;
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	val = readl(ep->elbi_base + FSD_PCIE_CXPL_DEBUG_00_31);
> +	val &= PORT_LOGIC_LTSSM_STATE_MASK;
> +
> +	return (val == PORT_LOGIC_LTSSM_STATE_L0);
> +}
> +
> +static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> +	phy_init(ep->phy);
> +	fsd_pcie_msi_init(ep);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fsd_pcie_host_ops = {
> +	.init = fsd_pcie_host_init,
> +};
> +
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +				 unsigned int type, u16 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> +	switch (type) {
> +	case PCI_IRQ_INTX:
> +		return dw_pcie_ep_raise_intx_irq(ep, func_no);
> +	case PCI_IRQ_MSIX:
> +		dev_err(pci->dev, "EP does not support MSI-X\n");
> +		return -EINVAL;
> +	case PCI_IRQ_MSI:
> +		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> +	default:
> +		dev_err(pci->dev, "UNKNOWN IRQ type\n");

Do you want to return success below even for this error?

> +	}
> +
> +	return 0;

this should be -EOPNOTSUPP.

> +}
> +
> +static const struct pci_epc_features fsd_pcie_epc_features = {
> +	.linkup_notifier = false,

Default value is 'false'. So no need to initialize it agian.

> +	.msi_capable = true,
> +	.msix_capable = false,

Here also.

> +};
> +
> +static const struct pci_epc_features *fsd_pcie_get_features(struct dw_pcie_ep *ep)
> +{
> +	return &fsd_pcie_epc_features;
> +}
> +
> +static const struct dw_pcie_ep_ops fsd_ep_ops = {
> +	.raise_irq	= fsd_pcie_raise_irq,
> +	.get_features	= fsd_pcie_get_features,
> +};
> +
> +static void fsd_set_device_mode(struct exynos_pcie *ep)
> +{
> +	if (ep->pdata->device_mode == DW_PCIE_RC_TYPE)
> +		writel(FSD_DEVICE_TYPE_RC, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +	else
> +		writel(FSD_DEVICE_TYPE_EP, ep->elbi_base + FSD_PCIE_DEVICE_TYPE);
> +}
> +
> +static const struct dw_pcie_ops fsd_dw_pcie_ops = {
> +	.read_dbi	= fsd_pcie_read_dbi,
> +	.write_dbi	= fsd_pcie_write_dbi,
> +	.write_dbi2	= fsd_pcie_write_dbi2,
> +	.start_link	= fsd_pcie_start_link,
> +	.stop_link	= fsd_pcie_stop_link,
> +	.link_up	= fsd_pcie_link_up,
> +};
> +
>  static const struct samsung_res_ops exynos_res_ops_data = {
>  	.init_regulator		= exynos_init_regulator,
>  	.pcie_irq_handler	= exynos_pcie_irq_handler,
>  };
>  
> +static const struct samsung_res_ops fsd_res_ops_data = {
> +	.pcie_irq_handler	= fsd_pcie_irq_handler,
> +	.set_device_mode	= fsd_set_device_mode,
> +};
> +
>  static int exynos_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -361,6 +575,26 @@ static int exynos_pcie_probe(struct platform_device *pdev)
>  	if (IS_ERR(ep->phy))
>  		return PTR_ERR(ep->phy);
>  
> +	if (ep->pdata->soc_variant == FSD) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> +		if (ret)
> +			return ret;
> +
> +		ep->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> +				"samsung,syscon-pcie");
> +		if (IS_ERR(ep->sysreg)) {
> +			dev_err(dev, "sysreg regmap lookup failed.\n");

Use dev_err_probe() here and below.

> +			return PTR_ERR(ep->sysreg);
> +		}
> +
> +		ret = of_property_read_u32_index(dev->of_node, "samsung,syscon-pcie", 1,
> +						 &ep->sysreg_offset);
> +		if (ret) {
> +			dev_err(dev, "couldn't get the register offset for syscon!\n");
> +			return ret;
> +		}
> +	}
> +
>  	/* External Local Bus interface (ELBI) registers */
>  	ep->elbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
>  	if (IS_ERR(ep->elbi_base))
> @@ -397,6 +631,22 @@ static int exynos_pcie_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto fail_phy_init;
>  
> +		break;
> +	case DW_PCIE_EP_TYPE:
> +		phy_init(ep->phy);
> +
> +		ep->pci.ep.ops = pdata->ep_ops;
> +
> +		ret = dw_pcie_ep_init(&ep->pci.ep);
> +		if (ret < 0)
> +			goto fail_phy_init;
> +
> +		ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> +		if (ret)
> +			goto fail_phy_init;
> +
> +		pci_epc_init_notify(ep->pci.ep.epc);

So the platform doesn't support PERST# and generates its own refclk?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC

[Shradha Todi]

Add the support for PCIe controller driver and phy driver for Tesla FSD.
It includes support for both RC and EP.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Shradha Todi <shradha.t@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd-evb.dts      |  34 +++++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi |  65 +++++++++
 arch/arm64/boot/dts/tesla/fsd.dtsi         | 147 +++++++++++++++++++++
 3 files changed, 246 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 9ff22e1c8723..1b63c5d72d19 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -130,3 +130,37 @@ &serial_0 {
 &ufs {
 	status = "okay";
 };
+
+&pcierc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
+			<&pcie0_slot1>;
+};
+
+&pcieep2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
+			<&pcie0_slot1>;
+};
+
+&pcierc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
+			 <&pcie0_slot0>;
+};
+
+&pcieep0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
+			 <&pcie0_slot0>;
+};
+
+&pcierc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
+};
+
+&pcieep1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
+};
diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
index 6f4658f57453..fa99aa4b9906 100644
--- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
@@ -120,6 +120,27 @@ eth0_mdio: eth0-mdio-pins {
 		samsung,pin-pud = <FSD_PIN_PULL_NONE>;
 		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
 	};
+
+	pcie1_clkreq: pcie1-clkreq-pins {
+		samsung,pins = "gpf6-0";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie1_wake: pcie1-wake-pins {
+		samsung,pins = "gpf6-1";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie1_preset: pcie1-preset-pins {
+		samsung,pins = "gpf6-2";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
 };
 
 &pinctrl_peric {
@@ -493,6 +514,50 @@ eth1_mdio: eth1-mdio-pins {
 		samsung,pin-pud = <FSD_PIN_PULL_UP>;
 		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
 	};
+
+	pcie0_clkreq: pcie0-clkreq-pins {
+		samsung,pins = "gpc8-0";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie0_wake0: pcie0-wake0-pins {
+		samsung,pins = "gpc8-1";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie0_preset0: pcie0-preset0-pins {
+		samsung,pins = "gpc8-2";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie0_wake1: pcie0-wake1-pins {
+		samsung,pins = "gpc8-3";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	pcie0_slot0: pcie0-gpio22-pins {
+		samsung,pins = "gpg2-6";
+		samsung,pin-function = <FSD_PIN_FUNC_OUTPUT>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+		samsung,pin-val = <1>;
+	};
+
+	pcie0_slot1: pcie0-gpio23-pins {
+		samsung,pins = "gpg2-7";
+		samsung,pin-function = <FSD_PIN_FUNC_OUTPUT>;
+		samsung,pin-pud = <FSD_PIN_PULL_UP>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+		samsung,pin-val = <1>;
+	};
 };
 
 &pinctrl_pmu {
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index a5ebb3f9b18f..8ed8d2131855 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -1009,6 +1009,16 @@ ethernet1: ethernet@14300000 {
 			status = "disabled";
 		};
 
+		pciephy0: pcie-phy@15080000 {
+			compatible = "tesla,fsd-pcie-phy0";
+			reg = <0x0 0x15080000 0x0 0x2000>,
+			      <0x0 0x150a0000 0x0 0x1000>;
+			#phy-cells = <0>;
+			samsung,pmu-syscon = <&pmu_system_controller>;
+			samsung,fsys-sysreg = <&sysreg_fsys0>;
+			status = "disabled";
+		};
+
 		ufs: ufs@15120000 {
 			compatible = "tesla,fsd-ufs";
 			reg = <0x0 0x15120000 0x0 0x200>,  /* 0: HCI standard */
@@ -1057,6 +1067,143 @@ ethernet0: ethernet@15300000 {
 			iommus = <&smmu_fsys0 0x0 0x1>;
 			status = "disabled";
 		};
+
+		pcierc2: pcie@15400000 {
+			compatible = "tesla,fsd-pcie";
+			reg = <0x0 0x15400000 0x0 0x2000>,
+			      <0x0 0x15090000 0x0 0x1000>,
+			      <0x0 0x15800000 0x0 0x1000>;
+			reg-names = "dbi", "elbi", "config";
+			ranges =  <0x82000000 0 0x15801000 0 0x15801000 0 0xffefff>;
+			clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			dma-coherent;
+			device_type = "pci";
+			interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
+			num-lanes = <4>;
+			phys = <&pciephy0>;
+			iommu-map = <0x0 &smmu_fsys0 0x4 0x10000>;
+			iommu-map-mask = <0x0>;
+			samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
+			status = "disabled";
+		};
+
+		pcieep2: pcie-ep@15400000 {
+			compatible = "tesla,fsd-pcie-ep";
+			reg = <0x0 0x15090000 0x0 0x1000>,
+			      <0x0 0x15400000 0x0 0x2000>,
+			      <0x0 0x15402000 0x0 0x80>,
+			      <0x0 0x15800000 0x0 0xff0000>;
+			reg-names = "elbi", "dbi", "dbi2", "addr_space";
+			clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
+				 <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			num-lanes = <4>;
+			phys = <&pciephy0>;
+			samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
+			status = "disabled";
+		};
+
+		pciephy1: pcie-phy@16880000 {
+			compatible = "tesla,fsd-pcie-phy1";
+			reg = <0x0 0x16880000 0x0 0x2000>,
+			      <0x0 0x16860000 0x0 0x1000>;
+			#phy-cells = <0>;
+			samsung,pmu-syscon = <&pmu_system_controller>;
+			samsung,fsys-sysreg = <&sysreg_fsys1>;
+			status = "disabled";
+		};
+
+		pcierc0: pcie@16a00000 {
+			compatible = "tesla,fsd-pcie";
+			reg = <0x0 0x16a00000 0x0 0x2000>,
+			      <0x0 0x168b0000 0x0 0x1000>,
+			      <0x0 0x17000000 0x0 0x1000>;
+			reg-names = "dbi", "elbi", "config";
+			ranges =  <0x82000000 0 0x17001000 0 0x17001000 0 0xffefff>;
+			clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			dma-coherent;
+			device_type = "pci";
+			interrupts = <GIC_SPI 115 IRQ_TYPE_EDGE_RISING>;
+			num-lanes = <4>;
+			phys = <&pciephy1>;
+			iommu-map = <0x0 &smmu_imem 0x0 0x10000>;
+			iommu-map-mask = <0x0>;
+			samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+			status = "disabled";
+		};
+
+		pcieep0: pcie-ep@16a00000 {
+			compatible = "tesla,fsd-pcie-ep";
+			reg = <0x0 0x168b0000 0x0 0x1000>,
+			      <0x0 0x16a00000 0x0 0x2000>,
+			      <0x0 0x16a02000 0x0 0x80>,
+			      <0x0 0x17000000 0x0 0xff0000>;
+			reg-names = "elbi", "dbi", "dbi2", "addr_space";
+			clocks = <&clock_fsys1 PCIE_LINK0_IPCLKPORT_AUX_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_DBI_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_MSTR_ACLK>,
+				 <&clock_fsys1 PCIE_LINK0_IPCLKPORT_SLV_ACLK>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			num-lanes = <4>;
+			phys = <&pciephy1>;
+			samsung,syscon-pcie = <&sysreg_fsys1 0x50c>;
+			status = "disabled";
+		};
+
+		pcierc1: pcie@16b00000 {
+			compatible = "tesla,fsd-pcie";
+			reg = <0x0 0x16b00000 0x0 0x2000>,
+			      <0x0 0x168c0000 0x0 0x1000>,
+			      <0x0 0x18000000 0x0 0x1000>;
+			reg-names = "dbi", "elbi", "config";
+			ranges =  <0x82000000 0 0x18001000 0 0x18001000 0 0xffefff>;
+			clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			dma-coherent;
+			device_type = "pci";
+			interrupts = <GIC_SPI 117 IRQ_TYPE_EDGE_RISING>;
+			num-lanes = <4>;
+			phys = <&pciephy1>;
+			samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+			status = "disabled";
+		};
+
+		pcieep1: pcie-ep@16b00000 {
+			compatible = "tesla,fsd-pcie-ep";
+			reg = <0x0 0x168c0000 0x0 0x1000>,
+			      <0x0 0x16b00000 0x0 0x2000>,
+			      <0x0 0x16b02000 0x0 0x80>,
+			      <0x0 0x18000000 0x0 0xff0000>;
+			reg-names = "elbi", "dbi", "dbi2", "addr_space";
+			clocks = <&clock_fsys1 PCIE_LINK1_IPCLKPORT_AUX_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_DBI_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_MSTR_ACLK>,
+				 <&clock_fsys1 PCIE_LINK1_IPCLKPORT_SLV_ACLK>;
+			clock-names = "aux", "dbi", "mstr", "slv";
+			num-lanes = <4>;
+			phys = <&pciephy1>;
+			samsung,syscon-pcie = <&sysreg_fsys1 0x510>;
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.49.0

Re: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC

[Krzysztof Kozlowski]

On 11/08/2025 17:46, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver for Tesla FSD.
> It includes support for both RC and EP.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts      |  34 +++++
>  arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi |  65 +++++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi         | 147 +++++++++++++++++++++
>  3 files changed, 246 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 9ff22e1c8723..1b63c5d72d19 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -130,3 +130,37 @@ &serial_0 {
>  &ufs {
>  	status = "okay";
>  };
> +
> +&pcierc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;
> +};
> +
> +&pcieep2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;
> +};
> +
> +&pcierc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +};
> +
> +&pcieep0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +};
> +
> +&pcierc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};
> +
> +&pcieep1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;


All these are pointless, because the node is disabled. The board level
should be complete, so also supplies and enabling the device.

Best regards,
Krzysztof

RE: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC

[Shradha Todi]

> > +&pcieep2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> > +			<&pcie0_slot1>;
> > +};
> > +
> > +&pcierc0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> > +			 <&pcie0_slot0>;
> > +};
> > +
> > +&pcieep0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> > +			 <&pcie0_slot0>;
> > +};
> > +
> > +&pcierc1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> > +};
> > +
> > +&pcieep1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> 
> 
> All these are pointless, because the node is disabled. The board level
> should be complete, so also supplies and enabling the device.
> 

I will enable required nodes. Had enabled while testing but missed to
add in patch. Though all nodes will not be enabled as it is a dual-mode
controller and cannot run as both RC and EP at the same time.

> Best regards,
> Krzysztof

Re: [PATCH v3 12/12] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC

[Manivannan Sadhasivam]

On Mon, Aug 11, 2025 at 09:16:38PM GMT, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver for Tesla FSD.
> It includes support for both RC and EP.
> 
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts      |  34 +++++
>  arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi |  65 +++++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi         | 147 +++++++++++++++++++++
>  3 files changed, 246 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 9ff22e1c8723..1b63c5d72d19 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -130,3 +130,37 @@ &serial_0 {
>  &ufs {
>  	status = "okay";
>  };
> +
> +&pcierc2 {

It'd be good to use underscore to differentiate RC and EP modes:

	pcie_rc1
	pcie_ep1

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;

Could you please explain what these 'preset' and 'slot' pins are?

> +};
> +
> +&pcieep2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;
> +};
> +
> +&pcierc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +};
> +
> +&pcieep0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +};
> +
> +&pcierc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};
> +
> +&pcieep1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +};

[...]

> +		pcieep2: pcie-ep@15400000 {
> +			compatible = "tesla,fsd-pcie-ep";
> +			reg = <0x0 0x15090000 0x0 0x1000>,
> +			      <0x0 0x15400000 0x0 0x2000>,
> +			      <0x0 0x15402000 0x0 0x80>,
> +			      <0x0 0x15800000 0x0 0xff0000>;
> +			reg-names = "elbi", "dbi", "dbi2", "addr_space";
> +			clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
> +			clock-names = "aux", "dbi", "mstr", "slv";
> +			num-lanes = <4>;
> +			phys = <&pciephy0>;
> +			samsung,syscon-pcie = <&sysreg_fsys0 0x434>;
> +			status = "disabled";

So only host mode DMA is cache coherent and not endpoint? Weird.

- Mani

-- 
மணிவண்ணன் சதாசிவம்